Fix panic when temp dir contains extra delimiters

[s1gnate-sync]

TMPDIR=/tmp// cause iso9660.Create to panic

iso9660>TMPDIR=/tmp////////// go test 
/tmp//////////diskfs_iso3563908867 /tmp/diskfs_iso3563908867
--- FAIL: TestFinalizeElTorito (0.03s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xf8 pc=0x5687cd]

goroutine 58 [running]:
testing.tRunner.func1.2({0x5ed900, 0x7b9460})
	/usr/lib/go/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
	/usr/lib/go/src/testing/testing.go:1635 +0x35e
panic({0x5ed900?, 0x7b9460?})
	/usr/lib/go/src/runtime/panic.go:785 +0x132
github.com/diskfs/go-diskfs/filesystem/iso9660.walkTree.func1({0xc0000206c0, 0x23}, {0x67a9b0, 0xc000036040}, {0x0?, 0x0?})
	/home/user/playground/go-diskfs/filesystem/iso9660/finalize.go:960 +0x54d
path/filepath.walkDir({0xc0000206c0, 0x23}, {0x67a9b0, 0xc000036040}, 0xc0000df730)
	/usr/lib/go/src/path/filepath/path.go:310 +0x50
path/filepath.walkDir({0xc000020330, 0x22}, {0x67aa90, 0xc00002a620}, 0xc0000df730)
	/usr/lib/go/src/path/filepath/path.go:332 +0x285
path/filepath.WalkDir({0xc000020330, 0x22}, 0xc00006d730)
	/usr/lib/go/src/path/filepath/path.go:400 +0x75
github.com/diskfs/go-diskfs/filesystem/iso9660.walkTree({0xc000020330, 0x22})
	/home/user/playground/go-diskfs/filesystem/iso9660/finalize.go:922 +0xc5
github.com/diskfs/go-diskfs/filesystem/iso9660.(*FileSystem).Finalize(0xc00013e120, {0x0?, 0x80?, 0xc00006de50?, {0x0?, 0x4fc720?}})
	/home/user/playground/go-diskfs/filesystem/iso9660/finalize.go:485 +0x2bd
github.com/diskfs/go-diskfs/filesystem/iso9660_test.TestFinalizeElTorito(0xc0000bab60)
	/home/user/playground/go-diskfs/filesystem/iso9660/finalize_test.go:58 +0x468
testing.tRunner(0xc0000bab60, 0x63bde0)
	/usr/lib/go/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 1
	/usr/lib/go/src/testing/testing.go:1743 +0x390
exit status 2
FAIL	github.com/diskfs/go-diskfs/filesystem/iso9660	0.094s

After fix

go-diskfs>TMPDIR=/tmp///// go test
PASS
ok  	github.com/diskfs/go-diskfs	0.056s

Encounter it on MacOS while experimenting with lima, more info here: lima-vm/lima#3150

I also made go test execute under scrambled TMPDIR just in case it would happen someplace else in the codebase

[s1gnate-sync]

@deitch, do you still maintain the library? Could please have a look at my PR as blocks my work.

Btw, thanks for all the work you did for this library! :-)

[deitch]

See the single comment.

[deitch]

I have a few suggestions here:

First, I understand the desire to test this, but I am not sure this should be here vs inside the code itself. We want the base case to be clean.

I think a different approach might be to work with the places where it runs os.MkdirTemp() and do os.SetEnv() in the test files before calling those. Can you see if that works instead?

Second, we call os.MkdirTemp() in one other place, squashfs. Take a look at a search for it.

Third, should we clean it up immediately after calling MkdirTemp()?

[s1gnate-sync]

@deitch that's for your reply! The reason I did it like so I think you guessed right. I just have a very little experience working on/writing tests in go. You're suggestion is absolutely on point! Feels nice not only provide a fix but also learn something new.

Second, we call os.MkdirTemp() in one other place, squashfs. Take a look at a search for it.

That's a good question! It's not a crime to have multiple slashes in path and it is absolutely valid. In ideal world nothing should be affected by it. It's even described in unix specification: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_271

A pathname can optionally contain one or more trailing <slash> characters. Multiple successive <slash> characters are considered to be the same as one <slash>, except for the case of exactly two leading <slash> characters.

So I think we shouldn't and it would be interesting to understand what exactly failing in this particular case. Some of external libraries, perhaps? Or if it's due to bug in this library then it should be definitely addressed as this is more like a workaround, not the ugliest though (as I believe some sort of data validation/normalisation is fine).

At the same time as any programmer I love when things work the same way and originate from the single source of truth. It won't hurt if I just introduce helper/wrapper of MkdirTemp function with extra sanity checks.

Do you lean any particular option? I probably slightly biased towards 2nd option as it looks more natural for me.

[deitch]

So I think we shouldn't and it would be interesting to understand what exactly failing in this particular case. Some of external libraries, perhaps? Or if it's due to bug in this library then it should be definitely addressed as this is more like a workaround, not the ugliest though (as I believe some sort of data validation/normalisation is fine)

You are correct, this is a better approach. I am reasonably confident that os.MkdirTemp() is working fine, and something in our library (or an external library) is failing. Your stack trace indicates that it is failing at iso9660/finalize.go:960. That is an odd place to fail, as the only referenced var that could possibly be nil there is parentDirInfo, defined here, which fills it from dirList. I suspect that the parentDir is slightly different from the key for the entry. Walk through it with dlv or put in some debugging printfs and check?