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

Handle badly named version files more gracefully and log information #34528

Merged
merged 1 commit into from Nov 21, 2022

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Oct 11, 2022

Fix #34085

Signed-off-by: Côme Chilliet come.chilliet@nextcloud.com

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc added the 3. to review Waiting for reviews label Oct 11, 2022
@come-nc come-nc added this to the Nextcloud 25 milestone Oct 11, 2022
@come-nc come-nc self-assigned this Oct 11, 2022
@come-nc come-nc requested review from a team, PVince81, ArtificialOwl and blizzz and removed request for a team October 11, 2022 08:13
@PVince81
Copy link
Member

@come-nc can you add an example file name ? (ideal would be in unit tests if possible)

any explanation why this is happening at all ? does PHP parse extensions differently in some versions ?

@blizzz blizzz mentioned this pull request Oct 11, 2022
2 tasks
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Oct 11, 2022
@blizzz
Copy link
Member

blizzz commented Oct 11, 2022

master is 26 👀

@come-nc
Copy link
Contributor Author

come-nc commented Oct 11, 2022

@come-nc can you add an example file name ? (ideal would be in unit tests if possible)

any explanation why this is happening at all ? does PHP parse extensions differently in some versions ?

To my knowledge this can only happen if there is a file in files_version with an incorrect name, but it’s better to log that correctly. See the linked issue.

PHP does not parse extension differently, but trying to access $array['undefinedkey'] was a silent error before PHP 8 if I recall correctly.

@come-nc
Copy link
Contributor Author

come-nc commented Oct 11, 2022

/backport to stable25

@come-nc
Copy link
Contributor Author

come-nc commented Oct 11, 2022

/backport to stable24

@come-nc
Copy link
Contributor Author

come-nc commented Oct 11, 2022

/backport to stable23

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@come-nc
Copy link
Contributor Author

come-nc commented Oct 11, 2022

CI failure unrelated

@come-nc
Copy link
Contributor Author

come-nc commented Oct 11, 2022

Tagging this do not merge until I understand the actual source of the problem, might as well fix the real issue.

@come-nc
Copy link
Contributor Author

come-nc commented Oct 20, 2022

Tagging this do not merge until I understand the actual source of the problem, might as well fix the real issue.

Ok, I am reopening this for merging.
It appears the original ticket is caused by bad behavior of groupfolders application.
This should help have a better understanding of the error in such cases.

@come-nc come-nc requested a review from szaimen November 21, 2022 14:14
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM but didnt test

@come-nc come-nc merged commit d0c72cc into master Nov 21, 2022
@come-nc come-nc deleted the fix/files_versions-clean-handling-of-bad-names branch November 21, 2022 15:18
@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable23 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
4 participants