gistfile1.txt

# PR #1: Complete Rock Ridge Extension Support

## Overview

Rock Ridge (RRIP/IEEE P1282) was ~80% implemented in go-diskfs. Read support worked, write
infrastructure existed, and most entry types (PX, NM, TF, SL, CL, PL, RE) serialized correctly.
This PR completes write support, fixes 12 bugs (4 originally planned + 8 discovered via RRIP spec
review and integration testing), improves read-back fidelity, and adds comprehensive tests
including xorriso integration validation.

**Scope**: Bug fixes + write-path completion + round-trip tests + xorriso integration tests.

**Normative Reference**: IEEE P1282 / RRIP (Rock Ridge Interchange Protocol), retrieved from
`http://cdrtools.sourceforge.net/private/RRIP/rrip.ps`. All field definitions and flag values
cited below are drawn from this specification.

---

## RRIP Specification Summary (Key Fields)

The following spec details were critical for identifying and fixing bugs in this PR. Each
System Use Entry type has a defined binary layout per IEEE P1282.

### PX ΓÇö POSIX File Attributes (Section 4.1.1)

```
Bytes 1-2:  "PX" signature
Byte  3:    Length (36 for RRIP 1.10, 44 for RRIP 1.12)
Byte  4:    Version (1)
Bytes 5-12: File Mode (both-endian uint32) ΓÇö POSIX st_mode
Bytes 13-20: Number of Links (both-endian uint32) ΓÇö POSIX st_nlink
Bytes 21-28: User ID (both-endian uint32) ΓÇö POSIX st_uid
Bytes 29-36: Group ID (both-endian uint32) ΓÇö POSIX st_gid
Bytes 37-44: Serial Number (both-endian uint32) ΓÇö RRIP 1.12 only, POSIX st_ino
```

**POSIX st_mode field encoding** (per POSIX.1 / `<sys/stat.h>`):

| Bits      | Mask       | Meaning           |
|-----------|------------|-------------------|
| 15-12     | `0o170000` | File type (IFMT)  |
| 11        | `0o4000`   | Set-user-ID       |
| 10        | `0o2000`   | Set-group-ID      |
| 9         | `0o1000`   | Sticky bit         |
| 8-0       | `0o0777`   | Permission bits    |

**POSIX file type values** (upper 4 bits of st_mode):

| Value      | Type           |
|------------|----------------|
| `0o140000` | Socket         |
| `0o120000` | Symbolic link  |
| `0o100000` | Regular file   |
| `0o060000` | Block device   |
| `0o040000` | Directory      |
| `0o020000` | Character device |
| `0o010000` | FIFO/pipe      |

**Critical insight**: Go's `os.ModeSetuid`, `os.ModeSetgid`, and `os.ModeSticky` use entirely
different bit positions (bits 23, 22, 19 respectively) than POSIX (`0o4000`, `0o2000`, `0o1000`).
Direct bitwise OR between Go mode bits and POSIX mode bits produces garbage. Explicit translation
is required.

### SL ΓÇö Symbolic Link (Section 4.1.3)

```
Bytes 1-2:  "SL" signature
Byte  3:    Length
Byte  4:    Version (1)
Byte  5:    Flags (bit 0 = CONTINUE to next SL entry)
Bytes 6+:   Component Records
```

Each **Component Record**:
```
Byte  0:    Component Flags
Byte  1:    Component Length (of content following these 2 bytes)
Bytes 2+:   Component content (if length > 0)
```

**Component Flag bits** (per RRIP Table 9):

| Bit | Mask   | Meaning                              |
|-----|--------|--------------------------------------|
| 0   | `0x01` | CONTINUE ΓÇö component continues in next CR |
| 1   | `0x02` | CURRENT ΓÇö represents `.`             |
| 2   | `0x04` | PARENT ΓÇö represents `..`             |
| 3   | `0x08` | ROOT ΓÇö represents `/`                |

### NM ΓÇö Alternate Name (Section 4.1.4)

```
Bytes 1-2:  "NM" signature
Byte  3:    Length
Byte  4:    Version (1)
Byte  5:    Flags (bit 0 = CONTINUE to next NM entry)
Bytes 6+:   Name content
```

Maximum name content per NM entry is 249 bytes (255 max SUSP entry - 5 header - 1 flags).
Names longer than 249 bytes require multiple NM entries with the CONTINUE flag set.

### CL ΓÇö Child Link (Section 4.1.5.1)

```
Bytes 1-2:  "CL" signature
Byte  3:    Length (12)
Byte  4:    Version (1)
Bytes 5-12: Location of child directory (both-endian uint32)
```

Used in the **original parent** to point to a relocated directory's actual location.
The CL entry replaces the original directory entry with a non-directory file entry.

### PL ΓÇö Parent Link (Section 4.1.5.2)

```
Bytes 1-2:  "PL" signature
Byte  3:    Length (12)
Byte  4:    Version (1)
Bytes 5-12: Location of true parent directory (both-endian uint32)
```

Used in the **relocated directory** itself. The `..` entry of a relocated directory uses PL
to point back to the original (true) parent instead of the relocation directory.

### RE ΓÇö Relocated Directory (Section 4.1.5.3)

```
Bytes 1-2:  "RE" signature
Byte  3:    Length (4)
Byte  4:    Version (1)
```

Marks a directory entry in `RR_MOVED` as a relocated directory. Rock Ridge-aware readers
should hide entries with RE from directory listings and instead follow CL pointers.

### TF ΓÇö Timestamps (Section 4.1.6)

```
Bytes 1-2:  "TF" signature
Byte  3:    Length
Byte  4:    Version (1)
Byte  5:    Flags (bit 7 = long form; bits 0-6 select which timestamps)
Bytes 6+:   Timestamp data (7 bytes short form, 17 bytes long form, per timestamp)
```

---

## 1. Bug Fixes

### 1.1 RE (Relocated Directory) Length Mismatch

**File**: `rockridge.go` | **Status**: Fixed in commit 92a6f9d

`rockRidgeRelocatedDirectory.Length()` returned `8`, but per RRIP Section 4.1.5.3, RE is a
4-byte entry: just the SUSP header (signature + length + version) with no data payload.

**Fix**: Changed `Length()` to return `4` and `Bytes()` to produce only 4 bytes.

### 1.2 Relocate() Bug ΓÇö Wrong Variable Appended

**File**: `rockridge.go` | **Status**: Fixed in commit 92a6f9d

When building replacement children for the true parent, the code appended `e` (the relocated
entry) instead of `c` (the current child). This discarded all non-relocated siblings.

**Fix**: Changed `children = append(children, e)` to `children = append(children, c)`.

### 1.3 Symlink Parsing ΓÇö ALL Component Flags Wrong

**File**: `rockridge.go` | **Status**: Fixed in commit 92a6f9d

All three special component flag values in `parseSymlink()` were wrong per RRIP Table 9:

| Component | Code had | Spec says | Status |
|-----------|----------|-----------|--------|
| `.`       | `0x01`   | `0x02`    | Wrong  |
| `..`      | `0x02`   | `0x04`    | Wrong  |
| `/`       | `0x03`   | `0x08`    | Wrong + unreachable dead code |

The write path was already correct. Only the read path was fixed.

### 1.4 Mode() on directoryEntry Ignores PX Permissions

**File**: `directoryentry.go` | **Status**: Fixed in commit 92a6f9d

`Mode()` returned `0o755 | os.ModeSymlink` for symlink entries (detected via
`rockRidgeSymlink` extension) and hardcoded `0o755` for everything else, ignoring the POSIX
mode stored in PX extensions entirely.

**Fix**: `Mode()` now reads the actual POSIX mode from the PX extension when present. Falls
back to `os.ModeDir | 0o755` for directories, `os.ModeSymlink | 0o777` for symlinks without
PX, and `0o755` for regular files without PX.

Also added `Sys()` returning `*RockRidgeInfo` (UID, GID, Nlink, Mode, Symlink target).

---

### Spec-Driven Bug Fixes (Discovered via RRIP Specification Review)

The following 6 bugs were discovered by comparing the implementation against the actual
IEEE P1282 / RRIP specification. They were not visible from code review alone because they
involved bit-level field definitions that require the spec as ground truth.

### 1.5 PX Setuid/Setgid/Sticky Bit Mapping (Write + Read)

**File**: `rockridge.go` | **Spec**: PX entry, POSIX st_mode field

Go's `os.ModeSetuid` (bit 23), `os.ModeSetgid` (bit 22), and `os.ModeSticky` (bit 19) use
entirely different bit positions than POSIX `0o4000`, `0o2000`, `0o1000`. The code was doing
direct bitwise OR, producing corrupt mode values.

**Write path fix** (PX `Data()` method):
```go
// Before (WRONG): modes |= uint32(m & os.ModeSetuid)
// After (CORRECT):
if m&os.ModeSetuid != 0 { modes |= 0o4000 }
if m&os.ModeSetgid != 0 { modes |= 0o2000 }
if m&os.ModeSticky != 0 { modes |= 0o1000 }
```

**Read path fix** (`parsePosixAttributes`):
```go
// Before (WRONG): m |= (modes & uint32(os.ModeSetuid))
// After (CORRECT):
if modes&0o4000 != 0 { m |= uint32(os.ModeSetuid) }
if modes&0o2000 != 0 { m |= uint32(os.ModeSetgid) }
if modes&0o1000 != 0 { m |= uint32(os.ModeSticky) }
```

Also removed the obsolete `saveSwapText` field from `rockRidgePosixAttributes` struct,
replacing it with proper `os.ModeSticky` handling.

### 1.6 PX File Type Mask ΓÇö Overlapping Bitmask Checks

**File**: `rockridge.go` | **Spec**: POSIX IFMT = `0o170000`

The read path used overlapping bitmask checks like `modes&0o60000 == 0o60000` instead of
masking with the POSIX file type field (`modes & 0o170000`). This caused block devices
(`0o060000`) to be misidentified as character devices because `0o060000 & 0o020000 != 0`.

**Fix**: Changed to proper POSIX file type extraction:
```go
switch modes & 0o170000 {
case 0o140000: m |= uint32(os.ModeSocket)
case 0o120000: m |= uint32(os.ModeSymlink)
case 0o060000: m |= uint32(os.ModeDevice)
case 0o020000: m |= uint32(os.ModeCharDevice | os.ModeDevice)
case 0o040000: m |= uint32(os.ModeDir)
case 0o010000: m |= uint32(os.ModeNamedPipe)
}
```

### 1.7 NM Continuation ΓÇö nameBytes Not Advanced

**File**: `rockridge.go` | **Spec**: NM entry, CONTINUE flag (Section 4.1.4)

In `rockRidgeName.Bytes()`, the `nameBytes` slice was never advanced after writing each
NM entry's portion, causing the first 249 bytes to be duplicated in every continuation record.

**Fix**: Added `nameBytes = nameBytes[len(copyBytes):]` after each iteration.

### 1.8 Symlink Parsing ΓÇö Missing Separators Before `.` and `..`

**File**: `rockridge.go` | **Spec**: SL Component Records

`parseSymlink()` concatenated `.` and `..` components without path separators, turning
`../../foo` into `....foo`. Named components were unconditionally prepended with `/`
(`name += "/" + string(...)`) which produced paths like `//a/b` for absolute symlinks.

**Fix**: Added separator logic that only inserts `/` between components when there's already
content that doesn't end with `/`:
```go
case flags&0x04 != 0:
    if name != "" && name != "/" {
        name += "/"
    }
    name += ".."
case flags&0x02 != 0:
    if name != "" && name != "/" {
        name += "/"
    }
    name += "."
case size > 0:
    if name != "" && name != "/" {
        name += "/"
    }
    name += string(b2[2 : 2+size])
```

The `name != ""` check prevents relative symlinks from getting a leading `/` (e.g., `../foo`
starts with `..`, not `/../foo`).

### 1.9 Relocate() ΓÇö CL Placeholder Gets Spurious RE Extension

**File**: `rockridge.go`

When `Relocate()` creates a CL placeholder in the original parent, `*replacer = *c` copies
the `trueParent` field from the (already-modified) entry. During finalization, the presence
of `trueParent` causes an RE extension to be emitted on the placeholder. Rock Ridge-aware
readers then hide the CL placeholder, making the relocated directory invisible.

**Fix**: Added `replacer.trueParent = nil` after the copy.

### 1.10 Relocate() ΓÇö Relocated Directory Not Added to Relocation Directory's Children

**File**: `rockridge.go`

The relocated directory entry was never appended to `relocationDir.children`. During
finalization, this meant the relocated directory never received a proper sector allocation,
so the CL extension pointed to an uninitialized (zero) location, causing read-back to fail.

**Fix**: Added `relocationDir.children = append(relocationDir.children, e)`.

### 1.11 getLocation() ΓÇö CL Resolution Only for Non-Terminal Path Components

**File**: `directoryentry.go`

The CL extension resolution (following the child link to the relocated directory's actual
location) was inside the `if len(parts) > 1` block. This meant a `ReadDir("a/b/c/d/e/f/g")`
could resolve intermediate CL entries, but a final path component that was a CL placeholder
would not be followed.

**Fix**: Moved CL resolution outside the `len(parts) > 1` conditional so it applies regardless
of whether the entry is terminal or intermediate in the path.

---

### Bug Fix Discovered via Integration Testing

### 1.12 Symlinks Treated as Regular Files During Finalize

**File**: `finalize.go`

`walkTree()` added symlinks to `fileList` like regular files, and `collapseAndSortChildren()`
collected them into the `files` list used by the copy loop. During the copy phase,
`os.Open()` follows symlinks: dangling symlinks (e.g., pointing to `/a/b/c`) caused
"no such file or directory" errors, and valid symlinks caused size mismatches (the target
file's content length differs from the symlink's own size).

In ISO 9660 with Rock Ridge, symlinks store their target in SL SUSP entries and have no
data extent.

**Fix**: Excluded symlinks from both `fileList` in `walkTree()` and from the files list in
`collapseAndSortChildren()` via `e.mode&os.ModeSymlink == 0` checks.

---

## 2. Enhancements

### 2.1 RR_MOVED Directory Hidden from Listings

When reading a Rock Ridge ISO with relocated directories, entries in `RR_MOVED` that have
RE markers are filtered from directory listings via `Relocated()`. This works correctly
after the Relocate() bug fixes.

### 2.2 UID/GID/Nlink Exposed Through FileInfo.Sys()

Added `RockRidgeInfo` struct and `Sys()` method on `directoryEntry`:

```go
type RockRidgeInfo struct {
    UID     uint32
    GID     uint32
    Nlink   uint32
    Mode    os.FileMode
    Symlink string
}
```

### 2.3 Mode() Reads PX Permissions

`directoryEntry.Mode()` now reads the actual POSIX mode from the PX extension instead of
returning hardcoded values. Falls back to sensible defaults when PX is absent.

---

## 3. Tests Implemented

### 3.1 Unit Tests (`rockridge_internal_test.go`)

| Test Function | What It Validates |
|---------------|-------------------|
| `TestRockRidgeSymlinkConsecutiveDotDot` | `../../foo`, `../../../bar`, `../..`, `./../foo` ΓÇö separators between consecutive dot-dot components |
| `TestRockRidgePosixAttributesSerial` | Serial number field round-trips correctly in RRIP 1.12 format |
| `TestRockRidgePosixAttributesFileTypes` | All 7 POSIX file types: regular, directory, symlink, pipe, socket, char device, block device |
| `TestRockRidgePosixAttributesSpecialBits` | Setuid, setgid, sticky, and combinations ΓÇö verifies POSIX<>Go bit translation |
| `TestRockRidgePosixAttributesRRIP110` | 36-byte PX format (RRIP 1.10) compatibility |
| `TestRockRidgeNameRoundTrip` | Short name, exactly 249 bytes, 500 bytes with NM continuation |
| `TestRockRidgeTimestampsRoundTrip` | Short/long form, various timestamp type combinations |

### 3.2 Round-Trip Integration Tests (`finalize_test.go`)

All Rock Ridge round-trip tests are organized under `TestFinalizeRockRidgeRoundTrips` using
`t.Run` subtests, with a shared `createRockRidgeISO` helper that extracts the common
boilerplate (create workspace, populate via setup function, finalize with Rock Ridge, read
back, return entries).

| Subtest | What It Validates |
|---------|-------------------|
| `permissions` | File permissions 0644, 0755, 0600 survive finalize -> read round-trip via PX extensions |
| `symlinks` | Absolute (`/a/b/c`) and relative (`../foo`) symlinks: ModeSymlink flag, SL target via `Sys().(*RockRidgeInfo)` |
| `long filenames` | Names > 31 chars preserved via NM extension |
| `deep directories` | Depth-9 directory tree: CL/PL/RE relocation, file content at original deep path, RR_MOVED hidden from root |
| `with El Torito` | Both Rock Ridge and El Torito active simultaneously ΓÇö boot image + regular file round-trip |

### 3.3 xorriso Integration Tests (`rockridge_xorriso_test.go`)

These tests validate Rock Ridge output against xorriso, the reference ISO 9660
implementation. Gated by `TEST_IMAGE` environment variable (Docker required, run via
`make test`).

| Test Function | What It Validates |
|---------------|-------------------|
| `TestRockRidgeXorrisoReadGoOutput` | Create ISO with go-diskfs, validate with `isoinfo -R`: permissions (0600/0644/0755), long filenames, symlinks, deep directory relocation |
| `TestRockRidgeGoReadXorrisoOutput` | Create ISO with xorriso (`-as mkisofs -R`), read with go-diskfs: filenames, permissions, subdirectory traversal, deep directory content, symlink target via `RockRidgeInfo` |
| `TestRockRidgeSUSPEntryComparison` | Byte-level comparison of raw PX and NM SUSP entries between go-diskfs and xorriso output for identical input files |

### 3.4 Docker Test Image Changes (`testhelper/docker/Dockerfile`)

Updated the test image to include both xorriso and cdrkit's `isoinfo`, which conflict on
`/usr/bin/mkisofs`. The Dockerfile installs cdrkit first, preserves `isoinfo`, removes
cdrkit's conflicting binaries, then installs xorriso. Also adds `e2fsprogs` for ext4 tests.

---

## 4. Files Changed

| File | Changes |
|------|---------|
| `filesystem/iso9660/rockridge.go` | Fix RE length, fix Relocate() variable bug, fix SL flags, fix SL separators, fix PX setuid/setgid/sticky, fix PX file type mask, fix NM continuation, fix Relocate() trueParent, fix Relocate() missing children, remove obsolete saveSwapText |
| `filesystem/iso9660/directoryentry.go` | Fix Mode() to read PX, add Sys() for RockRidgeInfo, fix getLocation() CL resolution scope |
| `filesystem/iso9660/finalize.go` | Exclude symlinks from fileList and collapseAndSortChildren() ΓÇö symlinks have no data extent |
| `filesystem/iso9660/rockridge_internal_test.go` | 7 new unit tests covering symlinks, PX attributes, NM continuation, timestamps |
| `filesystem/iso9660/finalize_test.go` | Rock Ridge round-trip subtests (permissions, symlinks, long filenames, deep dirs, El Torito) with shared helper |
| `filesystem/iso9660/rockridge_xorriso_test.go` | 3 xorriso integration tests: go->xorriso validation, xorriso->go reading, byte-level SUSP comparison |
| `testhelper/docker/Dockerfile` | Add xorriso + e2fsprogs, resolve cdrkit/xorriso mkisofs conflict |

**Total diff**: ~1545 insertions, ~70 deletions across 7 files.

---

## 5. Known Limitations

### Multi-Level Deep Directory Relocation

Directories deeper than depth 9 (requiring more than one level of relocation) cause a panic
during finalization due to directory sizing issues. The relocation logic handles the first
level correctly, but cascaded relocations (e.g., depth 11 requiring relocation at both depth 9
and depth 10) fail. This is a pre-existing limitation and is NOT a regression from this PR.

### Pre-Existing Test Failures (Not Caused by This PR)

- `TestIso9660Finalize` ΓÇö "copied 15 bytes, expected 0" (OpenFile+Write tracking bug)
- `TestIso9660OpenFile/Write/workspace` ΓÇö APPEND|RDONLY mode issue
- `TestGetExtensions` ΓÇö Windows SID cannot be parsed as integer UID

---

## 6. Compatibility Notes

- The write path uses **IEEE P1282 (Rock Ridge 1.12)** by default, which includes the serial
  number field in PX entries (44 bytes instead of 36).
- The read path handles both 1.10 and 1.12 formats via `pxLength` on the extension handler.
- ISOs generated by go-diskfs should be readable by Linux `mount -t iso9660`, macOS Finder,
  7-Zip, xorriso, and isoinfo.

---

## 7. Why Were the Spec-Driven Bugs Missed Initially?

The original PR1 plan (bugs 1.1-1.4) was based on **code review alone**. The additional bugs
(1.5-1.12) were invisible without the RRIP specification and integration testing because:

1. **Bit-level field definitions** ΓÇö The PX setuid/setgid bug (1.5) requires knowing that POSIX
   uses `0o4000`/`0o2000`/`0o1000` while Go uses bits 23/22/19. This mapping is not documented
   in the code and is only apparent from the POSIX st_mode specification referenced by RRIP.

2. **File type mask semantics** ΓÇö The overlapping bitmask bug (1.6) requires knowing that POSIX
   defines a file type *field* (`0o170000`) not independent flags. `0o060000 & 0o020000` being
   nonzero is only a problem if you know the correct extraction method.

3. **NM continuation protocol** ΓÇö The nameBytes advancement bug (1.7) only manifests with names
   longer than 249 bytes, which no existing test exercised. The spec defines the continuation
   protocol that made this testable.

4. **Symlink component separators** ΓÇö The `.`/`..` separator bug (1.8) only manifests with
   consecutive special components (e.g., `../../foo`), a pattern not previously tested.

5. **Relocation cascading bugs** (1.9-1.11) ΓÇö These only manifest during deep directory round-trip
   testing, which was enabled by fixing the earlier bugs. Each fix exposed the next issue.

6. **Symlink finalize bug** (1.12) ΓÇö Only manifests when symlinks point to non-existent targets
   (dangling symlinks) or when the target file's content length differs from the symlink's own
   size. Discovered via integration tests creating symlinks to `/a/b/c` and `../foo`.

---

## 8. Spec References

- **RRIP (Rock Ridge)**: IEEE P1282 "Rock Ridge Interchange Protocol" ΓÇö defines PX, PN, SL,
  NM, CL, PL, RE, TF, SF entry types. Retrieved from
  `http://cdrtools.sourceforge.net/private/RRIP/rrip.ps`
- **SUSP**: IEEE P1281 "System Use Sharing Protocol" ΓÇö defines SP, CE, PD, ST, ER, ES entries
- **ECMA-119 / ISO 9660**: Base filesystem spec ΓÇö directory records, path tables, volume
  descriptors
- **POSIX.1 / IEEE 1003.1**: Defines `st_mode` field encoding used by PX entries