codedread / bitjs

Binary Tools for JavaScript
MIT License
84 stars 7 forks source link

untar `ustar` format for long filenames missing slash #42

Closed andrewbranch closed 1 year ago

andrewbranch commented 1 year ago

👋 I think I found a bug in your untar implementation by pretty roundabout means.

I received an issue report on my own project at https://github.com/arethetypeswrong/arethetypeswrong.github.io/issues/101. I’m not using bitjs; I’m using my own fork of https://github.com/antimatter15/untar.js, which is itself a port of bitjs. The user there provided a repro, which I’ve built and packed into a tarball here, if you’d like to have a repro for a failing test:

fails-2.0.1-alpha.0.tgz

Unpacking that tarball with bitjs’s untar will show a file name missing a slash compared to what you’d see if you unpacked the tarball onto your filesystem:

+ dist/types/primitives/ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKABCDEFGHIJKLMNOPQRSTUVWXYZA.d.ts
- dist/types/primitives/ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJK/ABCDEFGHIJKLMNOPQRSTUVWXYZA.d.ts
                                                             ^

This name is too long to fit in the archive header, so its name is split between the name and prefix fields and has to be concatenated, as you have here: https://github.com/codedread/bitjs/blob/97cfb3b388404c5ce48dbb96b7ef4280819205ea/archive/untar.js#L89-L93

I found a source to support the empirical evidence that these should be joined with a slash:

The name field (100 chars) an inserted slash ('/') and the prefix field (155 chars) produce the pathname of the file. When recreating the original filename, name and prefix are concatenated, using a slash character in the middle. If a pathname does not fit in the space provided or may not be split at a slash character so that the parts will fit into 100 + 155 chars, the file may not be archived. Linknames longer than 100 chars may not be archived too.

https://linux.die.net/man/1/ustar

I made this change on my fork: https://github.com/andrewbranch/untar.js/commit/095c173ddcc2a41b54bb0753f340b2c7d80ce953

codedread commented 1 year ago

Thanks Andrew!

codedread commented 1 year ago

I'll hopefully get to fixing this with a test this week.

Also, Andrew, I encourage you to submit a PR fo antimatter15's semi-fork :)

andrewbranch commented 1 year ago

I’ve tried to contribute there before but it looks unmaintained, which is why I forked it in the first place 😅 Not sure anyone’s home there.

codedread commented 1 year ago

Re-reading that doc, it sounds like it should be name + '/' + prefix (which makes no sense to me). Anyway, I have made this change the way you have described in commit 96a74d910c917f2b8534b7ba729ce27969c12855 and I'll release it in 1.1.4

andrewbranch commented 1 year ago

I was similarly puzzled by that, but I think it was just careless wording.