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

Don't fail, if filenames are too long, shorten them instead #42

Open
wants to merge 4 commits into
base: 2.0-dev
Choose a base branch
from

Conversation

nibra
Copy link
Contributor

@nibra nibra commented Feb 12, 2022

Summary of Changes

The length of filenames is restricted to 250 characters, move_uploaded_files() already fails with length above 247 characters.(as stated in a comment in the PHP manual).

With this PR, filenames (i.e., the basename plus the extension) are shortend to 240 characters, if their length exceed that length with the File::makeSafe() method. The directory and the extension are preserved.

Changes affecting tests only:

  • The HelperTest addresses example.org instead of joomla.org, which seems to be more reliable.

Testing Instructions

The change is covered by unit tests.
Call File::makeSafe() with a filename with more than 240 characters. The result should be a sanitised version of the filename with a maximum length of 240 characters.

Documentation Changes Required

Probably.

Accessing joomla.org does not always work; example.org is better suited for this purpose.
@HLeithner
Copy link
Contributor

I think this should throw an exception and not silently change the input

@Llewellynvdm
Copy link

Llewellynvdm commented Feb 12, 2022

I would tend to agree with @HLeithner since the file name may be stored in the database, and unbeknown to the developer/user the mismatch exist.

Then filenames of that length is an edge-case and not that common, well at-least in my world.

@nibra
Copy link
Contributor Author

nibra commented Feb 13, 2022

Objection sustained!

@nibra nibra closed this Feb 13, 2022
@nibra nibra reopened this Feb 13, 2022
@nibra
Copy link
Contributor Author

nibra commented Feb 13, 2022

Moved shortening to File::makeSafe() which is expected to sanitise the filename properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants