Touffy / client-zip

A client-side streaming ZIP generator
MIT License
351 stars 22 forks source link

Add the unicode extra field when needed #22

Closed gigagg closed 3 years ago

gigagg commented 3 years ago

If non ascii char are detected in the name :

Touffy commented 3 years ago

Thank you for this.

Would you mind adding a test with a non-ASCII filename to ensure your code works as intended ?

Also, I know the original spec requires the extra field, but modern Zip programs that I've tried are fine with UTF8 in the default filename field. Did you run into one that isn't ? Just curious.

gigagg commented 3 years ago

I have one (slightly outdated) zip program that does not handle UTF-8 names correctly (file-roller on an old ubuntu).

I've done the corrections and I've added a test for the unicodePathExtraField function.

Touffy commented 3 years ago

I've tried running the code after merging, and unfortunately this breaks non-ASCII filenames with unzip and 7zip on macOS. Maybe the extra field must be added to the file header too, or maybe setting the EFS flag (bit 11) would be enough for file-roller ? I don't have more time to experiment today, could you check ?

gigagg commented 3 years ago

You are right, this version does not work properly. I maid it work, by adding the extra field in the file header. (Apparently the EFS flag was already on)

To be honest, I don't think it's worth the trouble :

I will still push a fixed version of my code, but I think you can just revert the changes if you wish ;)

Touffy commented 3 years ago

OK, yeah, I suspected it might be a little too situational. I'll keep the fix somewhere, maybe later when there is a plug-in mechanism we can add the Unicode Path extra field that way, without burdening the core library.

Touffy commented 3 years ago

I've added a ("won't fix" for now) issue to keep track of the problem — in case anyone else runs into it.