Properly fetch shares for files with spaces/other special characters in filenames #5884
Conversation
Signed-off-by: Claudio Cambra <[email protected]>
…eManager Signed-off-by: Claudio Cambra <[email protected]>
…tEncoding Signed-off-by: Claudio Cambra <[email protected]>
|
AppImage file: nextcloud-PR-5884-48f1dfaef5afbce2dba6fcf99e56e5895ae77d1c-x86_64.AppImage |
Signed-off-by: Claudio Cambra <[email protected]>
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
@claucambra besides one small comment, I've tested and it seems there is some issue with the path:
| newShare = parseLinkShare(data); | ||
| } else if (Share::isShareTypeUserGroupEmailRoomOrRemote(static_cast <Share::ShareType>(shareType))) { | ||
| newShare = parseUserGroupShare(data); | ||
| shares.append(parseLinkShare(data)); |
There was a problem hiding this comment.
@claucambra I don't see a good reason for this refactoring, besides removing space after static_cast you are basically doing the same but introducing more code for review
There was a problem hiding this comment.
while fixing the issue @claucambra would it be possible to write a test for the new method you're adding?
There was a problem hiding this comment.
The main reason for this change was to eliminate the likelihood of appending a null newShare to shares, and making the code a little shorter -- it doesn't seem like we need the newShare here.
As for the space I think this was auto-done by clang-format but I can re-add
There was a problem hiding this comment.
Looking into the other points now
There was a problem hiding this comment.
@claucambra No, I am fine with removing the space. I can see your point about newShare being not null, but there is an else statement so it should never be null, that said, you can just add a check if (newShare) prior to adding it if that makes sense
Ensure percent encoding is used. PR also includes some other minor fixes