Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if first is not empty #3333

Merged
merged 1 commit into from
Jun 30, 2021
Merged

Conversation

FlexW
Copy link

@FlexW FlexW commented May 18, 2021

If it's not checked that first is empty, crashes may happen.

Signed-off-by: Felix Weilbach felix.weilbach@nextcloud.com

@@ -177,7 +177,7 @@ void ProcessDirectoryJob::process()
// For windows, the hidden state is also discovered within the vio
// local stat function.
// Recall file shall not be ignored (#4420)
bool isHidden = e.localEntry.isHidden || (f.first[0] == '.' && f.first != QLatin1String(".sys.admin#recall#"));
bool isHidden = e.localEntry.isHidden || (f.first.size() != 0 && f.first[0] == '.' && f.first != QLatin1String(".sys.admin#recall#"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FlexW Wouldn't f.isEmpty() do the same as f.first.size() != 0 but look shorter?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FlexW Also, I guess you forgot to add an assert and qcWarning. Just in case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes thats better:)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allexzander I'm not sure if we should add a warning. The entry that caused trouble came from e2e. Maybe it is expected that some names are empty?

@FlexW FlexW force-pushed the bugfix/check-if-first-is-empty branch from d81e8f1 to 9bfeaaa Compare May 18, 2021 09:09
@mgallien
Copy link
Collaborator

/rebase

@github-actions github-actions bot force-pushed the bugfix/check-if-first-is-empty branch from 9bfeaaa to 39072b0 Compare June 22, 2021 08:58
@FlexW
Copy link
Author

FlexW commented Jun 22, 2021

/rebase

1 similar comment
@mgallien
Copy link
Collaborator

/rebase

If it's not checked that first is empty, crashes may happen.

Signed-off-by: Felix Weilbach <felix.weilbach@nextcloud.com>
@github-actions github-actions bot force-pushed the bugfix/check-if-first-is-empty branch from 39072b0 to c9715db Compare June 30, 2021 06:57
@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-3333-c9715db7202ec4bc69dd3c673597aee74ecf2b07-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien merged commit 2a0b6f3 into master Jun 30, 2021
@mgallien mgallien deleted the bugfix/check-if-first-is-empty branch June 30, 2021 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants