Skip to content

fix(unzip): keep non UTF-8 entries and drop entries with a bad checksum - #5155

Open
shepilov wants to merge 1 commit into
masterfrom
fix/unzip-entry-names-and-checksum
Open

shepilov wants to merge 1 commit into
masterfrom
fix/unzip-entry-names-and-checksum

Conversation

@shepilov

@shepilov shepilov commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Fixes #5131
Fixes #5134

Problem

Solution

  • The directory check reads the original header (f.Mode().IsDir() before touching the name), and f.Name is no longer mutated.
  • Names that are not valid UTF-8 are decoded from CP437, the usual DOS code page; every byte maps to a rune, so names stay distinct. Valid UTF-8 names are unchanged, including those from archivers that write UTF-8 without setting the flag (macOS). golang.org/x/text is already a direct dependency. CP437 is a heuristic: names written in another code page (Latin-1, CP866, Shift-JIS...) are readable only partly, but they are no longer lost.
  • When io.Copy fails after Close() committed the file, the file is destroyed (fs.DestroyFile) and the error is returned.

Test plan

  • unzip keeps the files with a non UTF-8 name: photos/\xe1, \x82.txt and \x8a.txt are extracted as photos/ß, é.txt and è.txt with their content, and café.txt (valid UTF-8, no UTF-8 flag) stays café.txt
  • unzip does not keep an entry with a bad checksum: the job fails with zip.ErrChecksum and a.txt does not exist
  • go test ./worker/archive/ passes, including the existing path traversal tests

@shepilov
shepilov requested a review from taratatach as a code owner November 3, 2026 14:59
The invalid UTF-8 bytes of an entry name were deleted, and the cleaned
name was then used to know if the entry is a directory. A file named
"photos/\xe1" (ß in CP437, as written by archivers that do not set the
UTF-8 flag) became "photos/", so it was skipped as a directory and its
content silently dropped. Distinct names could also collide ("\x82.txt"
and "\x8a.txt" both became ".txt"). The directory check now reads the
original header, and names that are not valid UTF-8 are decoded from
CP437, the usual DOS code page (#5131).

archive/zip reports a CRC or data descriptor mismatch on the last read,
after every byte has been written, so file.Close() committed a file with
unverified content before the job failed. The file is now destroyed when
the copy fails (#5134).

Fixes #5131
Fixes #5134
@shepilov
shepilov force-pushed the fix/unzip-entry-names-and-checksum branch from 39ffb37 to 1b3acf2 Compare November 3, 2026 15:13

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant