iso9660: Truncate short filenames to 8.3 format for compatibility (#315)

* iso9660: Truncate short filenames to 8.3 format for compatibility

When creating ISO 9660 filesystems with Rock Ridge extensions, diskfs
creates both a Rock Ridge long name and an ISO 9660 short name for
compatibility with tools that don't support Rock Ridge.

Previously, the calculateShortnameExtension() function would uppercase
and sanitize filenames but did not enforce the ISO 9660 Level 1 8.3
format constraints. This resulted in short names with extensions longer
than 3 characters and basenames longer than 8 characters.

Some tools access files using their ISO 9660 short names and expect
proper 8.3 format (8-character basename, 3-character extension). When
diskfs creates ISOs with longer extensions or basenames, these tools
fail to find the files.

This commit updates calculateShortnameExtension() to enforce ISO 9660
Level 1 limits:
- Extensions are truncated to a maximum of 3 characters
- Basenames are truncated to a maximum of 8 characters

This matches the behavior of traditional ISO creation tools like
xorriso (default Level 1 mode) and ensures maximum compatibility.

Comprehensive tests have been added to verify the 8.3 format is
correctly enforced for various filename patterns including long names,
special characters, and edge cases.

Assisted-by: Claude Code

* iso9660: Resolve filename collisions in Level 1 format

When multiple files truncate to the same 8.3 short name, automatically
assign unique numeric suffixes to resolve the collision. Files with
naturally-occurring numeric suffixes are preserved, and the collision
resolution uses the minimum number of digits needed.

For example, when 76 files all truncate to FILENAME, collision
resolution assigns names FILENA00 through FILENA75. This matches the
behaviour of xorriso in Level 1 mode.

Assisted-by: Claude Code

Author: zaneb

filesystem/iso9660/finalize.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"math"
 	"os"
 	"path"
 	"path/filepath"
@@ -52,6 +53,13 @@ type FinalizeOptions struct {
 //	Nlink() uint32         // number of hardlinks, if supported
 //	Uid()   uint32         // uid, if supported
 //	Gid()   uint32         // gid, if supported
+type collisionGroup struct {
+	files     []*finalizeFileInfo
+	parent    *finalizeFileInfo // directory containing these files
+	basename  string            // the truncated basename they all collide on
+	extension string            // the truncated extension (if any)
+}
+
 type finalizeFileInfo struct {
 	path               string
 	target             string
@@ -914,10 +922,11 @@ func createPathTable(fi []*finalizeFileInfo) *pathTable {
 
 func walkTree(workspace string) ([]*finalizeFileInfo, map[string]*finalizeFileInfo, error) {
 	var (
-		dirList  = make(map[string]*finalizeFileInfo)
-		fileList = make([]*finalizeFileInfo, 0)
-		entry    *finalizeFileInfo
-		serial   uint64
+		dirList         = make(map[string]*finalizeFileInfo)
+		fileList        = make([]*finalizeFileInfo, 0)
+		collisionGroups = make(map[string]*collisionGroup)
+		entry           *finalizeFileInfo
+		serial          uint64
 	)
 	err := filepath.WalkDir(workspace, func(actualPath string, d fs.DirEntry, err error) error {
 		if err != nil {
@@ -961,14 +970,120 @@ func walkTree(workspace string) ([]*finalizeFileInfo, map[string]*finalizeFileIn
 			dirList[parentDir] = parentDirInfo
 			fileList = append(fileList, entry)
 		}
+
+		// Add to collision groups (for both files and directories)
+		// Build collision key: parent path + truncated 8.3 name
+		truncated := entry.shortname
+		if entry.extension != "" {
+			truncated += "." + entry.extension
+		}
+		collisionKey := parentDir + "/" + truncated
+		if collisionGroups[collisionKey] == nil {
+			collisionGroups[collisionKey] = &collisionGroup{
+				files:     make([]*finalizeFileInfo, 0),
+				parent:    parentDirInfo,
+				basename:  entry.shortname,
+				extension: entry.extension,
+			}
+		}
+		collisionGroups[collisionKey].files = append(collisionGroups[collisionKey].files, entry)
 		return nil
 	})
 	if err != nil {
 		return nil, nil, err
 	}
+
+	// Resolve filename collisions
+	for _, group := range collisionGroups {
+		if len(group.files) > 1 {
+			if err := resolveCollisionGroup(group); err != nil {
+				return nil, nil, fmt.Errorf("error resolving filename collisions: %v", err)
+			}
+		}
+	}
+
 	return fileList, dirList, nil
 }
 
+// resolveCollisionGroup resolves filename collisions for a group of files
+// using xorriso's algorithm from libisofs/ecma119_tree.c
+func resolveCollisionGroup(group *collisionGroup) error {
+	if len(group.files) <= 1 {
+		return nil
+	}
+
+	// Build hash table of all names in the parent directory
+	nameTable := make(map[string]bool)
+	for _, sibling := range group.parent.children {
+		name := sibling.shortname
+		if sibling.extension != "" {
+			name += "." + sibling.extension
+		}
+		nameTable[name] = true
+	}
+
+	// Calculate minimum digit width needed
+	digits := len(fmt.Sprintf("%d", len(group.files)-1))
+
+	// Try increasing digit widths until all files are resolved (max 7 digits)
+	for digits < 8 {
+		maxBasename := 8 - digits
+
+		// Truncate basename to max length
+		basename := group.basename
+		if len(basename) > maxBasename {
+			basename = basename[:maxBasename]
+		}
+		extension := group.extension
+
+		// Try to assign names to each file in the collision group
+		change := 0
+		maxChange := int(math.Pow10(digits))
+		allResolved := true
+
+		for _, file := range group.files {
+			// Try incrementing change values until we find a unique name
+			found := false
+			for change < maxChange {
+				// Format the candidate name
+				indexStr := fmt.Sprintf("%0*d", digits, change)
+				candidate := basename + indexStr
+				if extension != "" {
+					candidate += "." + extension
+				}
+				change++
+
+				// Check if this name is already taken
+				if !nameTable[candidate] {
+					// Update file shortname in-place
+					file.shortname = basename + indexStr
+					nameTable[candidate] = true
+					found = true
+					break
+				}
+			}
+
+			if !found {
+				allResolved = false
+				break
+			}
+		}
+
+		if allResolved {
+			return nil
+		}
+
+		// Need more digits, try again
+		digits++
+	}
+
+	name := group.basename
+	if group.extension != "" {
+		name += "." + group.extension
+	}
+	return fmt.Errorf("too many filename collisions for: %s", name)
+}
+
 func calculateBlocks(size, blocksize int64) uint32 {
 	blocks := uint32(size / blocksize)
 	// add one for partial
@@ -993,5 +1108,17 @@ func calculateShortnameExtension(name string) (shortname, extension string) {
 	shortname = re.ReplaceAllString(shortname, "_")
 	extension = re.ReplaceAllString(extension, "_")
 
+	// Truncate to ISO 9660 Level 1 (8.3 format) for maximum compatibility
+	// This ensures compatibility with tools that expect traditional short names
+	// Extension: max 3 characters
+	if len(extension) > 3 {
+		extension = extension[:3]
+	}
+
+	// Basename: max 8 characters
+	if len(shortname) > 8 {
+		shortname = shortname[:8]
+	}
+
 	return shortname, extension
 }

filesystem/iso9660/finalize_internal_test.go

@@ -114,6 +114,66 @@ func TestCreatePathTable(t *testing.T) {
 	}
 }
 
+func TestCalculateShortnameExtension(t *testing.T) {
+	tests := []struct {
+		name              string
+		expectedShortname string
+		expectedExtension string
+	}{
+		// Test extension truncation to 3 characters
+		{"kargs.json", "KARGS", "JSO"},
+		{"file.html", "FILE", "HTM"},
+		{"document.markdown", "DOCUMENT", "MAR"},
+		{"archive.tar.gz", "ARCHIVE", "TAR"},
+		// Test extensions that are already 3 characters or less
+		{"test.js", "TEST", "JS"},
+		{"readme.md", "README", "MD"},
+		{"app.css", "APP", "CSS"},
+		{"data.txt", "DATA", "TXT"},
+		// Test files with no extension
+		{"README", "README", ""},
+		{"Makefile", "MAKEFILE", ""},
+		// Test uppercasing
+		{"MyFile.TxT", "MYFILE", "TXT"},
+		{"example.HtMl", "EXAMPLE", "HTM"},
+		// Test illegal character replacement and basename truncation
+		{"my-file.json", "MY_FILE", "JSO"},
+		{"test file.html", "TEST_FIL", "HTM"}, // "test file" = 9 chars after underscore replacement -> truncated to 8
+		{"[email protected]", "DATA_HOM", "XML"},  // "data@home" = 9 chars after underscore replacement -> truncated to 8
+		// Test basename truncation to 8 characters (ISO 9660 Level 1 - 8.3 format)
+		{"verylongfilenamethatexceedsthirtychars.txt", "VERYLONG", "TXT"},
+		{"anextremelylongbasenamethatgoeswaybeyondlimits.json", "ANEXTREM", "JSO"},
+		{"exactlythirtycharsbasenamehere.log", "EXACTLYT", "LOG"},
+		{"exactlyeightchars.js", "EXACTLYE", "JS"},
+		{"eightchr.txt", "EIGHTCHR", "TXT"},
+		// Test basename-only files that exceed 8 characters
+		{"verylongbasenamethatexceedsthirtycharacterswithnoextension", "VERYLONG", ""},
+		{"exactlythirtycharacterslong12", "EXACTLYT", ""},
+		{"thisfilenameisexactlythirtyone", "THISFILE", ""},
+		{"exactlyeightcharacters", "EXACTLYE", ""},
+		{"eightchr", "EIGHTCHR", ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			shortname, extension := calculateShortnameExtension(tt.name)
+			if shortname != tt.expectedShortname {
+				t.Errorf("calculateShortnameExtension(%q) shortname = %q, want %q", tt.name, shortname, tt.expectedShortname)
+			}
+			if extension != tt.expectedExtension {
+				t.Errorf("calculateShortnameExtension(%q) extension = %q, want %q", tt.name, extension, tt.expectedExtension)
+			}
+			// Verify ISO 9660 Level 1 (8.3 format) constraints
+			if len(shortname) > 8 {
+				t.Errorf("calculateShortnameExtension(%q) basename length = %d, exceeds 8 character limit", tt.name, len(shortname))
+			}
+			if len(extension) > 3 {
+				t.Errorf("calculateShortnameExtension(%q) extension length = %d, exceeds 3 character limit", tt.name, len(extension))
+			}
+		})
+	}
+}
+
 func TestCollapseAndSortChildren(t *testing.T) {
 	// we need to build a file tree, and then see that the results are correct and in order
 	// the algorithm uses the following properties of finalizeFileInfo:
@@ -202,3 +262,98 @@ func TestCollapseAndSortChildren(t *testing.T) {
 		t.Log(output)
 	}
 }
+
+func TestCollisionResolution(t *testing.T) {
+	tests := []struct {
+		name     string
+		files    []string          // filenames to create
+		expected map[string]string // original -> expected short name
+	}{
+		{
+			name:  "two file collision",
+			files: []string{"collision1.txt", "collision2.txt"},
+			expected: map[string]string{
+				"collision1.txt": "COLLISI0.TXT",
+				"collision2.txt": "COLLISI1.TXT",
+			},
+		},
+		{
+			name:  "natural name preserved with collision",
+			files: []string{"filenam1", "filenamelong", "filenamereallylong"},
+			expected: map[string]string{
+				"filenam1":           "FILENAM1",
+				"filenamelong":       "FILENAM0",
+				"filenamereallylong": "FILENAM2",
+			},
+		},
+		{
+			name: "10 files needing 1 digit + natural filenam9",
+			files: []string{
+				"filenam9",
+				"filenamereallylong0", "filenamereallylong1", "filenamereallylong2",
+				"filenamereallylong3", "filenamereallylong4", "filenamereallylong5",
+				"filenamereallylong6", "filenamereallylong7", "filenamereallylong8",
+				"filenamereallylong9",
+			},
+			expected: map[string]string{
+				"filenam9":            "FILENAM9",
+				"filenamereallylong0": "FILENA00",
+				"filenamereallylong1": "FILENA01",
+				"filenamereallylong2": "FILENA02",
+				"filenamereallylong3": "FILENA03",
+				"filenamereallylong4": "FILENA04",
+				"filenamereallylong5": "FILENA05",
+				"filenamereallylong6": "FILENA06",
+				"filenamereallylong7": "FILENA07",
+				"filenamereallylong8": "FILENA08",
+				"filenamereallylong9": "FILENA09",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create temp directory
+			tmpDir, err := os.MkdirTemp("", "collision-test-*")
+			if err != nil {
+				t.Fatalf("Failed to create temp dir: %v", err)
+			}
+			defer os.RemoveAll(tmpDir)
+
+			// Create test files
+			for _, filename := range tt.files {
+				path := tmpDir + "/" + filename
+				if err := os.WriteFile(path, []byte("test"), 0o600); err != nil {
+					t.Fatalf("Failed to create file %s: %v", filename, err)
+				}
+			}
+
+			// Run walkTree which also resolves collisions
+			_, dirList, err := walkTree(tmpDir)
+			if err != nil {
+				t.Fatalf("walkTree failed: %v", err)
+			}
+
+			// Add parent properties
+			root := dirList["."]
+			root.addProperties(1)
+
+			// Check results
+			for _, file := range root.children {
+				expectedShort, ok := tt.expected[file.name]
+				if !ok {
+					continue // not in our test map
+				}
+
+				actualShort := file.shortname
+				if file.extension != "" {
+					actualShort += "." + file.extension
+				}
+
+				if actualShort != expectedShort {
+					t.Errorf("File %s: expected short name %s, got %s", file.name, expectedShort, actualShort)
+				}
+			}
+		})
+	}
+}

filesystem/iso9660/finalize_test.go

@@ -196,7 +196,7 @@ func TestFinalize9660(t *testing.T) {
 
 		fooCount := 75
 		for i := 0; i <= fooCount; i++ {
-			filename := fmt.Sprintf("/FOO/FILENAME_%d", i)
+			filename := fmt.Sprintf("/FOO/FILENAME_%02d", i)
 			contents := []byte(fmt.Sprintf("filename_%d\n", i))
 			isofile, err = fs.OpenFile(filename, os.O_CREATE|os.O_RDWR)
 			if err != nil {
@@ -248,9 +248,9 @@ func TestFinalize9660(t *testing.T) {
 
 		// get a few files I expect
 		fileContents := map[string]string{
-			"/README.MD":       "readme\n",
-			"/FOO/FILENAME_50": "filename_50\n",
-			"/FOO/FILENAME_2":  "filename_2\n",
+			"/README.MD":    "readme\n",
+			"/FOO/FILENA50": "filename_50\n",
+			"/FOO/FILENA02": "filename_2\n",
 		}
 
 		for k, v := range fileContents {