fat32 fs creates dirs and files which are distinct on letter case

[quite]

The fat32 filesystem implementation allows creating directories and files that are distinct on case (upper/lower). I can for example have all these created (as listed by 7z l file):

...
2024-07-19 10:45:28 .....            5          512  EFI/BOOT/BOOTX64.EFI
2024-07-19 10:45:28 .....           17          512  EFI/BOOT/bootx64.efi
2024-07-19 10:45:28 .....           17          512  EFI/boot/bootx64.efi
...

A fat32 filesystem should not allow this.

There is also a TODO here about collision of generated short filenames, no matter letter case.

[deitch]

The fat32 implementation does check and convert, for the shortname and extension, that it is upper-case.

When it asks a directory to createEntry(), is gets the short name and extension fly calling to convertLfnSfn(), which calls uCaseValid() on both shortname and extension.

If you have a reproducible case where it doesn't, can you post it here?

[quite]

Sorry, I think I could not express exactly what I meant.

On FAT32, you cannot store two files in the same directory if their names differ only on letter case. Also, you can access any one file by its name in any case. But go-diskfs allows me to write such files (like above). The OpenFile impl also does not let me open files case-insensitively, but that's just natural consequence of it letting me create files that differ only on case I guess... Try this little script to get a feel for it:

#!/bin/bash
dd if=/dev/zero of=img bs=1024 count=$((1024 * 10))

mkfs.vfat ./img
echo foo | mcopy -i ./img               - ::/longEnoughName
echo bar | mcopy -i ./img -D autorename - ::/longenoughname
mdir -i ./img ::/
mcopy -i ./img ::/LONGENOUGHNAME -

Also in other words from mtools docs: https://www.gnu.org/software/mtools/manual/mtools.html#case-sensitivity

[quite]

There is also the thing that convertLfnSfn creates conflicting shortnames, as noted in TODO here https://github.com/diskfs/go-diskfs/blob/master/filesystem/fat32/directory.go#L43 -- but perhaps that should go in a separate new issue.

[deitch]

On FAT32, you cannot store two files in the same directory if their names differ only on letter case. Also, you can access any one file by its name in any case.

Right, that is mtools doing it correctly. What I don't understand is what is not working in diskfs. What is a use case where it creates a problem?

[quite]

I think I first by mistake ran into an error, creating /directory, and then trying to write file /DIRECTORY/FILE, which failed.

But in general, the fat32 implementation here is unfortunately flawed as it stands. And in such a low-level place that it will create all sorts of issues which I can't even imagine :) Other implementations will def be confused by finding clashing long filenames, since the definition is that they are case-insensitive (and actually even clashing short filenames, letter case aside! (that TODO)). Users will be confused if they can't open a file /FOOBARBAZ by the name /foobarbaz. And so on.

I guess one way to get started with fixing the implementation is to create test cases for all the ways that we currently think the fat32 impl is too lenient.

[deitch]

I guess one way to get started with fixing the implementation is to create test cases for all the ways that we currently think the fat32 impl is too lenient.

That sounds like the best place to start. FAT32 is not so complex that I am afraid of fixing it. Good test cases will define the problem well.

[jsando]

Hello, I encountered the same issue (with /EFI/BOOT as well, lol).

I was able to reproduce the duplicate folders with a test case added to fat32_test.go:

func TestFat32MkdirCases(t *testing.T) {
    f, err := tmpFat32(false, 0, 0)
    if err != nil {
        t.Fatal(err)
    }
    defer os.Remove(f.Name())
    fs, err := fat32.Create(f, 1048576, 0, 512, "")
    if err != nil {
        t.Error(err.Error())
    }
    err = fs.Mkdir("/EFI/BOOT")
    if err != nil {
        t.Error(err.Error())
    }
    // Make the same folders but now lowercase ... I expect it not to create anything new,
    // these folders exist but are named /EFI/BOOT
    err = fs.Mkdir("/efi/boot")
    if err != nil {
        t.Error(err.Error())
    }
    files, err := fs.ReadDir("/")
    if err != nil {
        t.Error(err.Error())
    }
    if len(files) != 1 {
        for _, file := range files {
            fmt.Printf("file: %s\n", file.Name())
        }
        t.Fatalf("expected 1 file, found %d", len(files))
    }
}

Test result:

=== RUN   TestFat32MkdirCases
file: EFI
file: efi
    fat32_test.go:136: expected 1 file, found 2
--- FAIL: TestFat32MkdirCases (0.00s)

FAIL

[deitch]

Now we are talking! This is good, let's get this in and fix it.

[deitch]

The issue appears to be here, where it checks if a conflict exists. The logic is off a bit. As far as I recall from the fat32 spec, you cannot have any case-insensitive conflict, even if the long names differ. So FooBar and foobar and FOOBAR cannot all coexist.