Skip to content

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, ArtificialOwl, PVince81 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 come-nc requested review from CarlSchwan and artonge October 11, 2022 13:41
@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

Development

Successfully merging this pull request may close these issues.

[Bug]: NC 25 Error: Undefined array key "extension" at /var/www/nextcloud/apps/files_versions/lib/Storage.php#489

5 participants