antangelo / xdvdfs

Original Xbox DVD Filesystem library and management tool
https://xiso.antangelo.com/
MIT License
73 stars 8 forks source link

xdvdfs-cli erases "invalid" images #73

Closed astarivi closed 8 months ago

astarivi commented 8 months ago

Trying to repack a Redump Xbox image under Windows 11 with the following:

xdvdfs pack D:\xbox\test\redump.iso Error: Not an XDVDFS volume

Overwrites the input image (redump.iso) and renders it to 0 bytes of length, effectively erasing the input. Same with the web version.

Specifying an output image, such as xdvdfs pack redump.iso repacked.iso has the same result.

Test image is a Redump CRC32 verified Morrowind (USA) image, and xdvdfs unpack command works as expected on this image, producing a valid output folder.

Additionally, marking the file as "Read-only" before running xdvdfs pack on it causes Error: Access is denied. (os error 5) output.

EDIT: Whoops, Github created the issue before I was done writing it.

antangelo commented 8 months ago

Generally I think the issue is that the output path resolves to the same path as the input, and the output file is truncated before the input is read (which is what results in the Not an XDVDFS volume error).

In the first case I think this is caused when an absolute path to the input is specified while being in the same directory as the input image. I don't know why this would happen with an explicit output path or with the web tool (unless the image is already zeroed out at the point you ran them), because those don't follow the same auto-resolution path.

It could happen in the web tool case if you select the input and output images to the same path manually (see #36 ).

astarivi commented 8 months ago

It seems there are two issues here:

  1. The one you've mentioned about the output overwriting the input. I applied a quick fix to my fork, and that one seems to be gone. (Which I could PR if you'd like)
  2. An issue when reading from images as a FS; it has to do with the usage of Path and PathBuff at the XDVDFSFilesystem read/write parts due to their platform dependent behavior.

To expand upon the 2nd one, an example of this issue is here.

In Windows, that line will, as an example, produce an /a\b path due to the platform path separator. In Linux, it will produce the XDVDFS correct /a/b path. The "\" path separator contained within this path in Windows will cause an Error: IOError: Entry does not exist error when it attempts to walk down that path.

To solve this, I'd propose either replacing the usage of Path and PathBuf(s) altogether from the write module in favor of str, like the reading code does, or, convert a str containing the right "/" path separators to a PathBuf, thus avoiding the code-base refactor.

antangelo commented 8 months ago

Going to leave this open until I fix the absolute path issue