diff --git a/BitMatchTests/XMLMetadataDetectionBoundsTests.swift b/BitMatchTests/XMLMetadataDetectionBoundsTests.swift new file mode 100644 index 00000000..cfd55c1c --- /dev/null +++ b/BitMatchTests/XMLMetadataDetectionBoundsTests.swift @@ -0,0 +1,100 @@ +import Foundation +import Testing +@testable import BitMatch + +struct XMLMetadataDetectionBoundsTests { + @Test func smallCameraMetadataStillIdentifiesCamera() throws { + let dir = try makeDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + try "SonyFX3" + .write(to: dir.appendingPathComponent("metadata.xml"), atomically: true, encoding: .utf8) + + #expect(XMLMetadataDetectionService.shared.detectCameraFromXML(at: dir) == "Sony FX3") + } + + @Test func oversizedXMLIsSkippedRatherThanParsedAsAPrefix() throws { + let dir = try makeDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let oversized = dir.appendingPathComponent("metadata.xml") + try Data("Wrong Camera".utf8).write(to: oversized) + let handle = try FileHandle(forWritingTo: oversized) + try handle.truncate(atOffset: UInt64(XMLMetadataDetectionService.maximumXMLBytes + 1)) + try handle.close() + + #expect(XMLMetadataDetectionService.shared.detectCameraFromXML(at: dir) == nil) + + try "Right Camera" + .write(to: dir.appendingPathComponent("other.xml"), atomically: true, encoding: .utf8) + #expect(XMLMetadataDetectionService.shared.detectCameraFromXML(at: dir) == "Right Camera") + } + + @Test func exactlyFourMiBXMLStillParses() throws { + let dir = try makeDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let prefix = "Boundary Camera" + let padding = String(repeating: " ", count: XMLMetadataDetectionService.maximumXMLBytes - prefix.utf8.count) + try (prefix + padding).write(to: dir.appendingPathComponent("boundary.xml"), atomically: true, encoding: .utf8) + + #expect(XMLMetadataDetectionService.shared.detectCameraFromXML(at: dir) == "Boundary Camera") + } + + @Test func linkedXMLIsNotFollowed() throws { + let dir = try makeDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + let outside = try makeDirectory() + defer { try? FileManager.default.removeItem(at: outside) } + let target = outside.appendingPathComponent("target.xml") + try "Outside Camera".write(to: target, atomically: true, encoding: .utf8) + try FileManager.default.createSymbolicLink( + at: dir.appendingPathComponent("linked.xml"), + withDestinationURL: target + ) + + #expect(XMLMetadataDetectionService.shared.detectCameraFromXML(at: dir) == nil) + } + + @Test func cancellationAfterReadNeverPublishesCameraName() async throws { + let dir = try makeDirectory() + defer { try? FileManager.default.removeItem(at: dir) } + try "Too Late" + .write(to: dir.appendingPathComponent("camera.xml"), atomically: true, encoding: .utf8) + + let gate = XMLReadGate() + defer { gate.release() } + let detection = Task.detached { + XMLMetadataDetectionService.shared.detectCameraFromXML(at: dir, onFileRead: { + gate.pauseAfterRead() + }) + } + let reachedRead = await Task.detached { gate.waitForRead() }.value + try #require(reachedRead, "camera detection never reached its file read") + detection.cancel() + gate.release() + + #expect(await detection.value == nil) + } + + private func makeDirectory() throws -> URL { + let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + return dir + } +} + +private final class XMLReadGate: @unchecked Sendable { + private let read = DispatchSemaphore(value: 0) + private let proceed = DispatchSemaphore(value: 0) + + func pauseAfterRead() { + read.signal() + proceed.wait() + } + + func waitForRead() -> Bool { + read.wait(timeout: .now() + 5) == .success + } + + func release() { + proceed.signal() + } +} diff --git a/Shared/Core/Services/Camera/XMLMetadataDetectionService.swift b/Shared/Core/Services/Camera/XMLMetadataDetectionService.swift index ceccc4e3..2179e65f 100644 --- a/Shared/Core/Services/Camera/XMLMetadataDetectionService.swift +++ b/Shared/Core/Services/Camera/XMLMetadataDetectionService.swift @@ -1,77 +1,88 @@ // Core/Services/Camera/XMLMetadataDetectionService.swift +import Darwin import Foundation -/// Service for detecting cameras via generic XML metadata files +/// Service for detecting cameras via generic XML metadata files. final class XMLMetadataDetectionService: Sendable { static let shared = XMLMetadataDetectionService() - private init() {} + static let maximumXMLBytes = 4 * 1024 * 1024 + private static let readChunkBytes = 64 * 1024 - // MARK: - Public Interface + private init() {} - /// The `onEnumerationStarted` hook is invocation-specific and defaults - /// to nil: production callers omit it, so no shared mutable state is - /// involved and parallel detections cannot cross-signal. - func detectCameraFromXML(at url: URL, onEnumerationStarted: (() -> Void)? = nil) -> String? { + /// Hooks are invocation-specific and nil in production. Tests use them to + /// order cancellation without shared mutable state or timing guesses. + /// Keep `onEnumerationStarted` last so existing trailing-closure callers + /// still observe enumeration rather than the later file read. + func detectCameraFromXML( + at url: URL, + onFileRead: (() -> Void)? = nil, + onEnumerationStarted: (() -> Void)? = nil + ) -> String? { let fm = FileManager.default - - // Common XML metadata files - let xmlFiles = [ - "metadata.xml", - "info.xml", - "camera.xml", - "settings.xml" - ] - - // Search for XML files in directory structure + onEnumerationStarted?() if let enumerator = fm.enumerator(at: url, includingPropertiesForKeys: [.isRegularFileKey], options: [.skipsHiddenFiles]) { for case let fileURL as URL in enumerator { guard !Task.isCancelled else { return nil } - if fileURL.pathExtension.lowercased() == "xml" { - if let cameraInfo = parseCameraFromXML(fileURL) { - return cameraInfo - } - } - - // Also check specific metadata files - for xmlFile in xmlFiles { - if fileURL.lastPathComponent.lowercased() == xmlFile { - if let cameraInfo = parseCameraFromXML(fileURL) { - return cameraInfo - } - } + guard fileURL.pathExtension.lowercased() == "xml" else { continue } + // metadata.xml, info.xml and the other conventional names are + // already covered by the extension check; parse each once. + if let cameraInfo = parseCameraFromXML(fileURL, onFileRead: onFileRead) { + return cameraInfo } } } - + return nil } - - // MARK: - XML Parsing - - private func parseCameraFromXML(_ xmlFile: URL) -> String? { - do { - let xmlContent = try String(contentsOf: xmlFile, encoding: .utf8) - - // Look for common camera metadata tags + + private func parseCameraFromXML(_ xmlFile: URL, onFileRead: (() -> Void)?) -> String? { + guard !Task.isCancelled else { return nil } + + // Camera naming is advisory. Refuse links and non-files, and never + // load an unbounded XML file just to look for a camera label. + let fd = xmlFile.path.withCString { + Darwin.open($0, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC) + } + guard fd >= 0 else { return nil } + defer { _ = Darwin.close(fd) } + + var info = stat() + guard fstat(fd, &info) == 0, + (info.st_mode & S_IFMT) == S_IFREG, + info.st_size >= 0, + info.st_size <= off_t(Self.maximumXMLBytes) else { + return nil + } + + // A pool per candidate also drains Foundation/regex temporaries when + // a card contains many XML files but no camera metadata. + return autoreleasepool { + guard let xmlData = readBoundedXML(fd: fd, expectedSize: Int(info.st_size)) else { return nil } + onFileRead?() + guard !Task.isCancelled, + let xmlContent = String(data: xmlData, encoding: .utf8) else { return nil } + let cameraPatterns = [ - // Generic patterns "]*>([^<]+)", "]*>([^<]+)", "]*>([^<]+)", "]*>([^<]+)", - - // Attribute patterns "camera=\"([^\"]+)\"", "make=\"([^\"]+)\"", "model=\"([^\"]+)\"" ] - + var make: String? var model: String? - + for pattern in cameraPatterns { + guard !Task.isCancelled else { return nil } if let range = xmlContent.range(of: pattern, options: .regularExpression) { + // A cancellation during the regex must not publish a + // camera name after the caller has moved to another card. + guard !Task.isCancelled else { return nil } let match = String(xmlContent[range]) if let valueRange = match.range(of: ">([^<]+)<", options: .regularExpression) ?? match.range(of: "=\"([^\"]+)\"", options: .regularExpression) { @@ -81,32 +92,46 @@ final class XMLMetadataDetectionService: Sendable { .replacingOccurrences(of: "=\"", with: "") .replacingOccurrences(of: "\"", with: "") .trimmingCharacters(in: .whitespacesAndNewlines) - + if pattern.contains("make") || pattern.contains("manufacturer") { make = value } else if pattern.contains("model") { model = value } else if pattern.contains("camera") { - return value // Direct camera name + return value } } } } - - // Combine make and model if available - if let make = make, let model = model { - return "\(make) \(model)" - } else if let make = make { - return make - } else if let model = model { - return model + + guard !Task.isCancelled else { return nil } + if let make, let model { return "\(make) \(model)" } + return make ?? model + } + } + + /// Reads through the first byte beyond the cap, so a file that grows + /// after fstat is still skipped rather than parsed as a valid prefix. + private func readBoundedXML(fd: Int32, expectedSize: Int) -> Data? { + var data = Data() + data.reserveCapacity(expectedSize) + var buffer = [UInt8](repeating: 0, count: Self.readChunkBytes) + + while data.count <= Self.maximumXMLBytes { + guard !Task.isCancelled else { return nil } + let remaining = Self.maximumXMLBytes + 1 - data.count + let count = min(buffer.count, remaining) + let bytesRead = buffer.withUnsafeMutableBytes { bytes in + Darwin.read(fd, bytes.baseAddress, count) + } + if bytesRead < 0 { + if errno == EINTR { continue } + return nil } - - } catch { - // Failed to read XML file - return nil + if bytesRead == 0 { return data } + data.append(contentsOf: buffer[..