Add support for reading and writing zip files. by al45tair · Pull Request #1699 · swiftlang/swift-docc

al45tair · 2026-10-16T16:29:23Z

Add ZipFileReader, ZipFileDataSource and ZipFileWriter, along with the ZLib and CZLib modules they use for deflate compression. ZipFileReader also conforms to ReadOnlyFileManagerProtocol, so that the contents of a zip file can be used wherever a read-only file system is expected.

This raises the minimum macOS deployment target to 15 for the span types the reader uses.

rdar://190233135

Add ZipFileReader, ZipFileDataSource and ZipFileWriter, along with the ZLib and CZLib modules they use for deflate compression. ZipFileReader also conforms to ReadOnlyFileManagerProtocol, so that the contents of a zip file can be used wherever a read-only file system is expected. This raises the minimum macOS deployment target to 15 for the span types the reader uses. rdar://190233135

On Windows, we need to fetch and build zlib, because SwiftPM can't do that on its own. Also, we no longer support Swift 6.1, so exclude that version. rdar://190233135

al45tair · 2026-10-16T16:29:35Z

@swift-ci Please test

The Zip code was taken from a separate repo elsewhere, which is why it was all `public`. Make it `package` scope here as we don't need it outside of DocC (and we don't want the DocC repo to become a source for zip functionality). rdar://190233135

There is currently a problem preventing us from using `Span` types in this repository. Add some shims for now. We can remove these when the problem is resolved. rdar://190233135

al45tair · 2026-10-17T14:19:57Z

@swift-ci Please test

d-ronnqvist

This is a very large change and I've only started to look at it but here's some initial feedback from that first pass.

d-ronnqvist · 2026-10-22T14:06:36Z

+        do {
+            let data = try contents(of: URL(filePath: path))
+            return data
+        } catch {
+            return nil
+        }


minor: I think this can be written more simply using try?

Suggested change

do {

let data = try contents(of: URL(filePath: path))

return data

} catch {

return nil

}

try? contents(of: URL(filePath: path))

d-ronnqvist · 2026-10-22T14:07:28Z

+        let buffer = UnsafeMutableRawBufferPointer.allocate(byteCount: 65536,
+                                                            alignment: 16)


It would be good to have a comment that explains this byte count and alignment.

d-ronnqvist · 2026-10-22T14:13:30Z

+        internal(set) package var name: String
+        internal(set) package var date: Date
+        internal(set) package var comment: String?
+
+        internal(set) package var flags: UInt16
+        internal(set) package var compression: UInt16
+        internal(set) package var crc32: UInt32
+        internal(set) package var compressedSize: Int
+        internal(set) package var uncompressedSize: Int


Question: What's modifying these properties that require them to be internal(set)? Would it be possible to restrict this further (private(set) or fileprivate(set))?

Probably. I'll try restricting them some more and see if anything breaks.

d-ronnqvist · 2026-10-22T14:16:33Z

+        while pos >= 0 {
+            let ch = span.bytes.unsafeLoad(fromByteOffset: pos, as: UInt8.self)
+            switch ch {
+            case 0x50:  // 'P'


FYI: you can initialize a UInt8 value for an ascii literal to make this more readable.

Suggested change

case 0x50: // 'P'

case .init(ascii: "P"):

I think that's more typing and less readable, honestly. We're really trying to find a magic number here too — the fact the PK part is ASCII is largely irrelevant.

d-ronnqvist · 2026-10-22T14:19:08Z

+        let disk = try source.read(from: eocdOffset + 4, as: UInt16.self)
+        let directoryDisk = try source.read(from: eocdOffset + 6, as: UInt16.self)
+        let entriesOnDisk = try source.read(from: eocdOffset + 8, as: UInt16.self)
+        let totalEntries = try source.read(from: eocdOffset + 10, as: UInt16.self)
+
+        if disk != directoryDisk || entriesOnDisk != totalEntries {
+            throw ZipFileError.spannedArchivesNotSupported
+        }
+
+        let cdSize = Int(try source.read(from: eocdOffset + 12, as: UInt32.self))
+        let cdOffset = Int(try source.read(from: eocdOffset + 16, as: UInt32.self))
+
+        let resolvedDirectory = try resolveZip64EndOfCentralDirectory(
+            eocdOffset: eocdOffset, cdSize: cdSize, cdOffset: cdOffset)
+
+        let commentLength = try source.read(from: eocdOffset + 20, as: UInt16.self)


Rather than having to update the offset for each read like this, I wonder if it's be worth having a "cursor" that automatically updates by MemoryLayout<T>.size for each successful read... what do you think?

d-ronnqvist · 2026-10-22T14:23:40Z

+                if span.bytes.unsafeLoad(fromByteOffset: pos + 1, as: UInt8.self) == 0x4b
+                    && span.bytes.unsafeLoad(fromByteOffset: pos + 2, as: UInt8.self) == 0x05
+                    && span.bytes.unsafeLoad(fromByteOffset: pos + 3, as: UInt8.self) == 0x06
+                {


FYI: IIRC it's possible to do a 3 byte load using a tuple type

Suggested change

if span.bytes.unsafeLoad(fromByteOffset: pos + 1, as: UInt8.self) == 0x4b

&& span.bytes.unsafeLoad(fromByteOffset: pos + 2, as: UInt8.self) == 0x05

&& span.bytes.unsafeLoad(fromByteOffset: pos + 3, as: UInt8.self) == 0x06

{

if span.bytes.unsafeLoad(fromByteOffset: pos + 1, as: (UInt8, UInt8, UInt8).self) == (0x4b, 0x05, 0x06) {

That's a nice improvement. I hadn't considered that.

d-ronnqvist · 2026-10-22T14:26:19Z

Are these types surfaced in the Zip module's API or can they have an internal access level?

They are in the API; ZipFileReader's File type, for instance, has a read(into span: inout OutputRawSpan) method.

d-ronnqvist · 2026-10-22T14:30:01Z

+            try sink.write(UInt32(0x0606_4b50))  // Zip64 EOCD record signature
+            try sink.write(UInt64(44))  // size of remaining record (fixed part only)
+            try sink.write(UInt16(45))  // version made by
+            try sink.write(UInt16(45))  // version needed to extract
+            try sink.write(UInt32(0))  // number of this disk
+            try sink.write(UInt32(0))  // disk with the start of the central directory
+            try sink.write(UInt64(files.count))  // entries on this disk
+            try sink.write(UInt64(files.count))  // entries in total
+            try sink.write(UInt64(centralDirectorySize))  // size of central directory
+            try sink.write(UInt64(centralDirectoryOffset))  // offset of central directory
+
+            bytesWritten += 56


Is there a way that the write operation could accumulate the number of bytes written to eliminate the risk that this manually tracked offset gets out of sync?

d-ronnqvist · 2026-10-22T14:31:18Z

+            try sink.write(UInt16(0))
+        }
+
+        try sink.close()


Is it expected that the sink doesn't close if it encounters a write error along the way? In other words; should this close() call be in a defer statement earlier in this method body?

That's a good idea. Let's put it in a defer at the top.

d-ronnqvist · 2026-10-22T14:32:35Z

I would rather that we don't commit any formatting configuration files.

Drat. I hadn't intended to add that (again). I should add it to .gitignores or similar, maybe.

al45tair added 2 commits October 16, 2026 17:26

[Build] Fix build scripts.

de79004

On Windows, we need to fetch and build zlib, because SwiftPM can't do that on its own. Also, we no longer support Swift 6.1, so exclude that version. rdar://190233135

al45tair requested a review from a team as a code owner October 16, 2026 16:29

al45tair added 2 commits October 17, 2026 15:18

We can't use Span in this repo. Add shims.

c055aee

There is currently a problem preventing us from using `Span` types in this repository. Add some shims for now. We can remove these when the problem is resolved. rdar://190233135

al45tair requested a review from d-ronnqvist October 22, 2026 09:29

d-ronnqvist reviewed Oct 22, 2026

View reviewed changes

		let buffer = UnsafeMutableRawBufferPointer.allocate(byteCount: 65536,
		alignment: 16)

Conversation

al45tair commented Oct 16, 2026

Uh oh!

al45tair commented Oct 16, 2026

Uh oh!

al45tair commented Oct 17, 2026

Uh oh!

d-ronnqvist left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants