A One-Star Review, and the Eight Bytes That Fixed It

On 2 September a US reader left ComicFlow one star. The review said it doesn’t recognise 90% of CBZ and CBR files.

My first reaction was that 90% is obviously wrong. I convert comics in this app every week. The reader is fine, the converter is fine, and if it failed on nine files out of ten there would be more than one review saying so.

That reaction was the bug. Not a bug in the code, a bug in me: I checked the claim against my own files, which all came from places that name things correctly, and concluded the claim was exaggerated. His files came from somewhere else, and for his library the number was probably about right.

Version 3.3 shipped the following day. The fix reads eight bytes.

What a CBZ actually is

A CBZ is a ZIP archive of numbered page images. A CBR is a RAR archive of the same thing. A CB7 is 7-Zip. The extension exists to tell a reader “open this in comic mode” and carries no other information.

Nothing enforces it. Renaming volume01.cbr to volume01.cbz changes the name and not one byte of the contents. The file is still a RAR.

So the extension is a hint supplied by whoever last touched the file, which on a comic archive is a long list of people: scanlation groups who rename by hand, sites whose export script hardcodes one extension, and readers who rename a file to the other one hoping it will start working. That last one is the cruel case. They are trying to fix it, and they are making it undiagnosable.

The code that was wrong

Both paths through the app made the same decision the same way. The reader picked a page provider, the converter picked an extractor, and both branched on pathExtension:

switch url.pathExtension.lowercased() {
case "cbz", "zip": return ZIPExtractor()
case "pdf":        return PDFRasterizer()
default:           return RARExtractor()
}

Look at the default. Anything unrecognised goes to RAR, which is a reasonable guess in 2019 and is also the line that makes the failure silent. A ZIP named .cbr lands in the RAR extractor. A RAR named .cbz lands in the ZIP extractor, where ZIPArchive(url:) returns nil:

guard let archive = ZIPArchive(url: zipURL) else {
    continuation.yield(.failed(.invalidFile))
    return
}

Both directions surface as invalidFile. The user sees a message that amounts to “this file is broken”, closes the app, opens the same file in any other reader, and it works. From where they are standing, my app is the broken one. They are not wrong.

What the telemetry said once I went looking

Twenty invalidFile reports on the shipped 3.2 build between 8 August and 2 September. Not twenty over the app’s lifetime. Twenty in about three and a half weeks, on one version.

I had been reading that number as a rounding error, because twenty is small next to total conversions and because invalidFile is exactly what you would expect a genuinely corrupt download to produce. It is a plausible error. That is what made it invisible: the bug was hiding behind an error message that was doing its job.

The one-star review is what reframed it. Twenty reports of “your app is broken” from people who each had a working file is not a rounding error, it is a category of failure I had labelled as user error and stopped looking at.

The fix

Read the header. Every archive format starts with a fixed signature, and eight bytes covers all of them:

static func container(forHeader bytes: [UInt8]) -> ArchiveContainer {
    // ZIP: local file header, plus the empty and spanned variants.
    if matches([0x50, 0x4B, 0x03, 0x04]) ||
       matches([0x50, 0x4B, 0x05, 0x06]) ||
       matches([0x50, 0x4B, 0x07, 0x08]) { return .zip }
    // RAR 5:  Rar!\x1A\x07\x01\x00      RAR 4: Rar!\x1A\x07\x00
    if matches([0x52, 0x61, 0x72, 0x21, 0x1A, 0x07, 0x01, 0x00]) ||
       matches([0x52, 0x61, 0x72, 0x21, 0x1A, 0x07, 0x00]) { return .rar }
    if matches([0x25, 0x50, 0x44, 0x46, 0x2D]) { return .pdf }        // %PDF-
    if matches([0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C]) { return .sevenZip }
    return .unknown
}

Then one rule about precedence:

static func resolvedContainer(of url: URL) -> ArchiveContainer {
    let sniffed = container(of: url)
    if sniffed != .unknown { return sniffed }
    return containerFromExtension(of: url)
}

The header wins. The extension is the fallback for a file whose header we cannot read or do not recognise, and that ordering is the whole decision: a mislabelled file is common, an unreadable header is not. Getting it the other way round would have fixed nothing.

The extension fallback also keeps the old default: .rar behaviour intact, deliberately. I did not want a file that happened to work in 3.2 to stop working in 3.3 because I had tidied up an unrelated branch.

The trap I nearly shipped

EPUB is a ZIP. Its header is PK\x03\x04 like any other ZIP, so a sniffer routes an EPUB straight to the CBZ provider, which opens it as a bag of images and shows you the cover art, the publisher logo and whatever illustrations the book contains, in archive order.

That is worse than the bug I was fixing. Failing to open a file is annoying. Opening a novel as forty loose JPEGs is a wrong answer delivered confidently.

So the declared format has to be checked first, before any sniffing happens:

// EPUB is a ZIP container by header, so it MUST be rejected on the declared
// format before sniffing, otherwise the detector routes it to the CBZ
// provider, which would open a book as a bag of images.
guard declared != .epub else { throw PageProviderError.unsupportedFormat }

Content sniffing tells you what a file is. It cannot tell you what it is for. When two formats share a container, the extension is the only signal you have about intent, and the fix is to keep using it for that one question rather than to throw it away because it lied to you about something else.

Being right about the error, not just the outcome

One of the ZIP signatures in that list is not a real archive. PK\x05\x06 is an empty ZIP: a valid file with nothing inside it.

I could have left it out. It is not going to extract either way. But an empty ZIP with no signature match falls to the extension fallback, gets routed to RAR, and fails as invalidFile, which tells the user their file is corrupt. Detected as a ZIP, it reaches the ZIP extractor and fails as noImagesFound, which tells them the archive is empty.

Both are failures. Only one of them is true, and only one tells the user something they can act on. Error accuracy is worth code even when it does not change whether the operation succeeds, because the error is the entire product at the moment it fires.

The test I had, and the test I needed

I already had unit tests over the signatures. They fed synthetic eight-byte arrays to container(forHeader:) and asserted the right enum came back. They passed the whole time the app was failing.

They were testing the part I got right. The bug was two layers up, in the routing, and no test touched it.

What was missing builds a real archive and drives the real code over it:

func testRealZipNamedCBRIsDetectedAndExtracts() async throws {
    let cbr = try await Self.makeRealZip(in: tempDir, named: "Volume 01.cbr", pages: 4)

    XCTAssertEqual(ArchiveContainerDetector.resolvedContainer(of: cbr), .zip,
                   "a real ZIP named .cbr must route to the ZIP extractor")

    let urls = try await Self.extractZip(at: cbr, to: extracted)
    XCTAssertEqual(urls.count, 4, "all pages must come out of the mislabelled archive")
}

It writes a genuine ZIP with the app’s own exporter, renames it .cbr, and pushes it through the real extractor. That is the user’s scenario exactly, and against 3.2 it failed in about 28 milliseconds, four times in a row, with invalidFile.

There is a control test beside it that runs the same archive under its correct name, so I can prove the fix did not trade one mislabelling for another.

The other thing the same investigation found

Once I was looking at how entries get chosen rather than how archives get opened, a second bug was sitting in plain sight.

Zip a folder in macOS Finder and you get a hidden __MACOSX directory carrying a ._page001.jpg AppleDouble sidecar for every real page001.jpg. Those sidecars have an image extension and are real entries, so my page filter accepted them. They also sort ahead of the real pages, and the PDF generator hard-failed if it could not read the first image.

The result: any CBZ zipped on a Mac failed to convert at all, with pdfCreationFailed("Could not read first image").

The page filter had been copy-pasted into three places, Set(["jpg", "jpeg", "png", "webp"]) in the ZIP path, the RAR path and the post-extraction sort, with no filter for archive metadata in any of them. That duplication also meant comics whose pages were GIF, BMP, TIFF or HEIC reported noImagesFound while being full of images iOS decodes natively.

All three now call one function that knows what a page is and what is housekeeping:

static func isMetadata(path: String) -> Bool {
    let components = path.split(separator: "/", omittingEmptySubsequences: true)
    if components.contains(where: { $0 == "__MACOSX" }) { return true }
    guard let name = components.last else { return true }
    if name.hasPrefix("._") { return true }
    let lowercased = name.lowercased()
    return lowercased == ".ds_store" || lowercased == "thumbs.db"
}

Three copies of a rule are three chances to be wrong in different ways, and I had taken all three.

What I would tell myself in August

A filename is user input. I know this about text fields and I did not know it about file extensions, which are the same thing with more steps: a string, supplied by a person, that I was treating as a fact about the bytes on disk.

Any format where the label is separable from the contents will get separated. Not occasionally. Continuously, by well-meaning people, at a rate you cannot influence. Formats that carry a magic number are telling you they expect this.

A plausible error message hides the bug behind it. invalidFile was doing its job so convincingly that I read twenty reports as twenty broken downloads. If I had picked one of them up and asked whether the file was actually broken, I would have found this four weeks earlier.

Test the routing, not the parser. My signature tests were green throughout. Confidence came from the layer that was already correct, which is the least useful place to have coverage.

There is still a list of things this does not fix. Encrypted archives need the password, split RARs need every part joined on a computer, and CB7 needs 7-Zip support I have not written. Those genuinely cannot be solved on a phone, and 3.3 at least now says which one you have hit instead of calling all of them invalid.

The reader who left that review has not come back, which is fair. His files work now.