Directory and filename must be no more than 255 characters in length

[lfluvisotto]

Description (*)

According to https://en.wikipedia.org/wiki/Comparison_of_file_systems , I know that wikipedia isn't the best reference to find things related, but most of operation systems the maximum filename length is 255 characters.

https://www.ibm.com/support/knowledgecenter/SSEQVQ_8.1.9/client/c_cmd_filespecsyntax.html

On this documentation above says:

• On AIX, Solaris, and Mac: The maximum number of characters for a file name is 255.
• On Linux: The maximum length for a file name is 255 bytes.
• The maximum number of bytes for a file name and file path when combined is 6255. However, the file name itself cannot exceed 255 bytes

https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-shares--directories--files--and-metadata

On this documentation above from azure says:

Directory and file component names must be no more than 255 characters in length.

So, the path + filename cannot exceed 255 characters in length.

The current logic is incorrect because it's checking only the basename (filename), not the entire path (directory + filename).

So, I created a pull request fixing it:

Related Pull Requests

N/A

Fixed Issues

Fixes #29377

Manual testing scenarios (*)

Upload a file where filename is 255 characters length an exception will be thrown.

Questions or comments

N/A

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @lfluvisotto. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[lfluvisotto]

@ihor-sviziev could you have a look?

[ihor-sviziev]

@magento run all tests

[lfluvisotto]

@ihor-sviziev these failed checks, should I install the composer package b2b to see what's going on?

[igrybkov]

@lfluvisotto, I don't have my opinion about the pull request itself, but I don't think you understood documentation correctly, including Azure and Wikipedia.

While wikipedia says that most filesystems has 255 symbols restriction for the filename, the pathname length is mostly not limited or measured in thousands of symbols.

And here's what azure documentation says about path name:

A path name may be no more than 2,048 characters in length. Individual components in the path can be a maximum of 255 characters in length.

So the statement that combined path + name cannot be more than 255 symbols is not correct. The name could have 255 symbols and each directory could have 255 symbols with max limit for the whole path length of 2048 symbols.

And the part of the documentation that you referred to make this statement just says about limitations of each node of a path that could be either a file or a directory, so these limitations are common for them. It doesn't mean it's the limitation for the pathname:

Directory and file component names must be no more than 255 characters in length.

[lfluvisotto]

@igrybkov

On the table, the column that stores path + filename the data type is varchar 255.

We could define text data type for the column but I don't think it's necessary, we should avoid changes on data type but we should be able to use the 255 characters available.

It would be good we remove the limitation of 90 characters on the basename of file.

The limitation of 90 characters length doesn't make sense, we should keep the maximum length of defined data type can store itself.

[ihor-sviziev]

Hi @lfluvisotto,
Please review my comments and review failing tests, seems like tests should be fixed also. Also please cover your changes with some kind of test in case if failing tests not covering your case

[lfluvisotto]

@magento run all tests

[lfluvisotto]

@magento run all tests

[lfluvisotto]

@magento run all tests

[ihor-sviziev]

@lfluvisotto your changes looks good. could you cover your changes with some automated test?

[ihor-sviziev]

@lfluvisotto your changes looks good. could you cover your changes with some automated test?

[ihor-sviziev]

@magento run all tests

[ihor-sviziev]

@magento run Unit Tests

Strange... unit tests failed, but no failures in reports. Restarting...

[ihor-sviziev]

@lfluvisotto could you check why the unit tests are failing? I don't see anything in the reports, but could you try to run the tests you've changed?

[gabrieldagama]

@lfluvisotto @ihor-sviziev here is the reason we are getting the failure on the Unit Tests:

12:54:10 [Unit-Tests-CE] ..PHP Fatal error:  Class Mock_WriteInterface_3adfa6cf contains 18 abstract methods and must therefore be declared abstract or implement the remaining methods (Magento\Framework\Filesystem\Directory\WriteInterface::create, Magento\Framework\Filesystem\Directory\WriteInterface::delete, Magento\Framework\Filesystem\Directory\WriteInterface::renameFile, ...) in /var/www/html/vendor/phpunit/phpunit/src/Framework/MockObject/MockClass.php(42) : eval()'d code on line 3

That seems to be related to the change of the Writer's implementation to its interface on the test (app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php).

[lfluvisotto]

@lfluvisotto @ihor-sviziev here is the reason we are getting the failure on the Unit Tests:

12:54:10 [Unit-Tests-CE] ..PHP Fatal error:  Class Mock_WriteInterface_3adfa6cf contains 18 abstract methods and must therefore be declared abstract or implement the remaining methods (Magento\Framework\Filesystem\Directory\WriteInterface::create, Magento\Framework\Filesystem\Directory\WriteInterface::delete, Magento\Framework\Filesystem\Directory\WriteInterface::renameFile, ...) in /var/www/html/vendor/phpunit/phpunit/src/Framework/MockObject/MockClass.php(42) : eval()'d code on line 3

That seems to be related to the change of the Writer's implementation to its interface on the test (app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php).

The weird thing is: other php files use the same I did and don't have problems. I'm rechecking it.

[lfluvisotto]

@lfluvisotto @ihor-sviziev here is the reason we are getting the failure on the Unit Tests:

12:54:10 [Unit-Tests-CE] ..PHP Fatal error:  Class Mock_WriteInterface_3adfa6cf contains 18 abstract methods and must therefore be declared abstract or implement the remaining methods (Magento\Framework\Filesystem\Directory\WriteInterface::create, Magento\Framework\Filesystem\Directory\WriteInterface::delete, Magento\Framework\Filesystem\Directory\WriteInterface::renameFile, ...) in /var/www/html/vendor/phpunit/phpunit/src/Framework/MockObject/MockClass.php(42) : eval()'d code on line 3

That seems to be related to the change of the Writer's implementation to its interface on the test (app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php).

now it's supposed to run ok, I changed by \Magento\Framework\Filesystem\Directory\Write

[lfluvisotto]

@magento run all tests

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8262 has been created to process this Pull Request

[engcom-Bravo]

✔️ QA Passed

BEFORE switching to the PR, an error appeared on Importing

AFTER switching to the PR, the Import is successfull

The image is successfully added to Images And Videos section

Manual testing scenario

1. Copy a .jpg image into var/import/images directory
2. Rename the image using long name (you may use the long name from the .csv file attached to the issue Filename is too long exception when importing media images #29377. File name is - 295744_20200804073409_this_is_a_very_long_product_image_name_1004222_16330_chroma_blau_schw_150cm_rl_30.jpg )
3. From Magento Admin, go to System - Import
4. Set the following values:

• Entity type - Product
• Import Behavior - Add/Update

5. Click Choose File and upload the sample_long.csv file attached to the Filename is too long exception when importing media images #29377
6. Click Check Data
   (File is valid! To start import process press "Import" button success message appears)
7. Click Import
   (Import successfully done messauge appears)
8. Go to Catalog - Products and assert that Sample Product has been imported
9. Click Edit an assert that the products Images And Videos contains the previously used image

If the filename is too long, a proper error message appears on Importing

[m2-assistant]

Hi @lfluvisotto, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.