Previously we could create cd9660 images with duplicate short (level 2) names. cd9660_level2_convert_filename used a 30-character limit (for files and directories), not including the '.' separator. cd9660_rename_filename used a 31-character limit, including the '.'. Directory names 31 characters or longer (without '.') were shortened to 30 characters, and if a collision occurred cd9660_rename_filename uniquified them starting with the 31st character. Unfortunately the directory record's name_len was already set, so the unique part of the name was stripped off. Directories are up to 31 d-characters (i.e., A-Z 0-9 and _); there is no provision for a '.' in a directory name. Increase the name length limit to 31 for directories, and exclude '.'s. This code is still fragile and convoluted, and would beenfit from a more holistic improvement. PR: 283238, 283112
Details
Details
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Example from PR283238:
$ mkdir -p test9660/this-is-a-directory-with-a-long-common-prefix-1 test9660/this-is-a-directory-with-a-long-common-prefix-2 $ makefs -t cd9660 -o rockridge test.iso test9660/ $ isoinfo -p -i test.iso Setting input-charset to 'UTF-8' from locale. Path table starts at block 18, size 86 1: 1 14 2: 1 15 THIS_IS_A_DIRECTORY_WITH_A_LON 3: 1 16 THIS_IS_A_DIRECTORY_WITH_A_LON
With this:
$ isoinfo -p -i test.iso Setting input-charset to 'UTF-8' from locale. Path table starts at block 18, size 90 1: 1 14 2: 1 15 THIS_IS_A_DIRECTORY_WITH_A_LON0 3: 1 16 THIS_IS_A_DIRECTORY_WITH_A_LONG
Comment Actions
Any chance you could turn this into a regression test case?
I've been looking at makefs regression tests, in particular as a prerequisite for a more holistic rewrite of makefs's cd9660 pathname logic. The approach taken by the existing tests isn't great for these sorts of tests (it mounts the created image and relies on the kernel's iso9660 support). We could add tests that require cdrtools to be present, but my (longer-term) preference is to add a (probably lua-based) iso9660 dumper.
Anyhow it looks like basic functionality can be verified with the current mount-the-iso approach (with -r to disable use of Rockridge extensions).
Comment Actions
It should be quite straightforward to just add a cdrtools-based test (or even one using pycdlib) in the interim. Unless there's some existing iso9660 parser written in lua, it's going to take a fair bit of work to write one...
Anyhow it looks like basic functionality can be verified with the current mount-the-iso approach (with -r to disable use of Rockridge extensions).
That also seems fine to me.
Comment Actions
Add a comment on uniqueness requirements, and verify that the uniqueness is at the end.
Comment Actions
This looks good. Mark is right about tests, but that's largely orthogonal now that you added a test for this issue.
Comment Actions
Clarify the test with comments, and move check_cd9660_support to the beginning -- no point in creating test inputs if we're not going to run the test.
Comment Actions
As it turns out this breaks some existing tests - we create a directory called .g and the test fails as it now creates _g.