Skip to content
Merged
125 changes: 125 additions & 0 deletions Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -34450,6 +34450,131 @@
}
}
},
"browser.download.defaultFilename": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "download"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "ダウンロード"
}
},
"zh-Hans": {
"stringUnit": {
"state": "translated",
"value": "下载"
}
},
"zh-Hant": {
"stringUnit": {
"state": "translated",
"value": "下載"
}
},
"ko": {
"stringUnit": {
"state": "translated",
"value": "다운로드"
}
},
"de": {
"stringUnit": {
"state": "translated",
"value": "Download"
}
},
"es": {
"stringUnit": {
"state": "translated",
"value": "descarga"
}
},
"fr": {
"stringUnit": {
"state": "translated",
"value": "téléchargement"
}
},
"it": {
"stringUnit": {
"state": "translated",
"value": "download"
}
},
"da": {
"stringUnit": {
"state": "translated",
"value": "download"
}
},
"pl": {
"stringUnit": {
"state": "translated",
"value": "pobieranie"
}
},
"ru": {
"stringUnit": {
"state": "translated",
"value": "загрузка"
}
},
"bs": {
"stringUnit": {
"state": "translated",
"value": "preuzimanje"
}
},
"ar": {
"stringUnit": {
"state": "translated",
"value": "تنزيل"
}
},
"nb": {
"stringUnit": {
"state": "translated",
"value": "nedlasting"
}
},
"pt-BR": {
"stringUnit": {
"state": "translated",
"value": "download"
}
},
"th": {
"stringUnit": {
"state": "translated",
"value": "ดาวน์โหลด"
}
},
"tr": {
"stringUnit": {
"state": "translated",
"value": "indirme"
}
},
"uk": {
"stringUnit": {
"state": "translated",
"value": "завантаження"
}
},
"km": {
"stringUnit": {
"state": "translated",
"value": "ទាញយក"
}
}
}
},
"browser.downloading": {
"extractionState": "manual",
"localizations": {
Expand Down
158 changes: 158 additions & 0 deletions Sources/Panels/BrowserDownloadFilenameResolver.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import Foundation
import ImageIO
import UniformTypeIdentifiers

nonisolated enum BrowserDownloadHTTPStatusDecision: Equatable, Sendable {
case allow
case reject(statusCode: Int)
}

nonisolated struct BrowserDownloadFilenameResolver: Sendable {
func httpStatusDecision(for response: URLResponse?) -> BrowserDownloadHTTPStatusDecision {
guard let httpResponse = response as? HTTPURLResponse else {
return .allow
}
guard (200...299).contains(httpResponse.statusCode) else {
return .reject(statusCode: httpResponse.statusCode)
}
return .allow
}

func imageType(forImageData data: Data) -> UTType? {
guard let imageSource = CGImageSourceCreateWithData(data as CFData, nil),
let typeIdentifier = CGImageSourceGetType(imageSource) as String?,
let type = UTType(typeIdentifier),
type.conforms(to: .image) else {
return nil
}
return type
}

func imageType(forDownloadedFileAt fileURL: URL) -> UTType? {
guard let imageSource = CGImageSourceCreateWithURL(fileURL as CFURL, nil),
let typeIdentifier = CGImageSourceGetType(imageSource) as String?,
let type = UTType(typeIdentifier),
type.conforms(to: .image) else {
return nil
}
return type
}

func suggestedFilename(
suggestedFilename: String?,
response: URLResponse?,
sourceURL: URL,
imageType: UTType?
) -> String {
let fallbackURL = response?.url ?? sourceURL
let filenameCandidate = suggestedFilename
?? response?.suggestedFilename
?? fallbackURL.lastPathComponent
let safeCandidate = sanitizedFilename(filenameCandidate, fallbackURL: fallbackURL)

guard let imageType else {
return safeCandidate
}

return imageFilename(
candidate: safeCandidate,
imageType: imageType
)
}

func suggestedFilename(
suggestedFilename: String?,
response: URLResponse?,
sourceURL: URL,
imageData: Data
) -> String {
self.suggestedFilename(
suggestedFilename: suggestedFilename,
response: response,
sourceURL: sourceURL,
imageType: imageType(forImageData: imageData)
)
}

func suggestedFilename(
suggestedFilename: String?,
sourceURL: URL,
imageFileURL: URL
) -> String {
self.suggestedFilename(
suggestedFilename: suggestedFilename,
response: nil,
sourceURL: sourceURL,
imageType: imageType(forDownloadedFileAt: imageFileURL)
)
}

private func imageFilename(
candidate: String,
imageType: UTType
) -> String {
if hasImageExtension(candidate, matching: imageType) {
return candidate
}

let strippedCandidate = strippingNonImageExtensions(from: candidate, matching: imageType)
if strippedCandidate != candidate {
return strippedCandidate
}

let filenameExtension = preferredFilenameExtension(for: imageType)
let base = baseNameByRemovingFinalExtension(from: candidate)
return "\(base).\(filenameExtension)"
}

private func sanitizedFilename(_ raw: String, fallbackURL: URL?) -> String {
let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines)
let candidate = (trimmed as NSString).lastPathComponent
let fromURL = fallbackURL?.lastPathComponent ?? ""
let base = candidate.isEmpty ? fromURL : candidate
let replaced = base.replacingOccurrences(of: ":", with: "-")
let safe = replaced.trimmingCharacters(in: .whitespacesAndNewlines)
return safe.isEmpty ? defaultFilename : safe
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private func strippingNonImageExtensions(from filename: String, matching imageType: UTType) -> String {
var candidate = filename
while !hasImageExtension(candidate, matching: imageType) {
let next = baseNameByRemovingFinalExtension(from: candidate)
guard next != candidate else { break }
candidate = next
}
return hasImageExtension(candidate, matching: imageType) ? candidate : filename
}

private func baseNameByRemovingFinalExtension(from filename: String) -> String {
let nsFilename = filename as NSString
let base = nsFilename.deletingPathExtension
return base.isEmpty ? defaultFilename : base
}

private var defaultFilename: String {
String(localized: "browser.download.defaultFilename", defaultValue: "download")
}

private func hasImageExtension(_ filename: String, matching imageType: UTType) -> Bool {
let pathExtension = (filename as NSString).pathExtension
guard !pathExtension.isEmpty,
let extensionType = UTType(filenameExtension: pathExtension),
extensionType.conforms(to: .image) else {
return false
}

return extensionType.conforms(to: imageType) || imageType.conforms(to: extensionType)
}

private func preferredFilenameExtension(for imageType: UTType) -> String {
if imageType.conforms(to: .jpeg) {
return "jpg"
}
if let preferred = imageType.preferredFilenameExtension, !preferred.isEmpty {
return preferred
}
return "img"
}
}
46 changes: 22 additions & 24 deletions Sources/Panels/BrowserPanel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8808,9 +8808,10 @@ private extension NSObject {
/// Handles WKDownload lifecycle by saving to a temp file synchronously (no UI
/// during WebKit callbacks), then showing NSSavePanel after the download finishes.
class BrowserDownloadDelegate: NSObject, WKDownloadDelegate {
private struct DownloadState {
private struct DownloadState: Sendable {
let tempURL: URL
let suggestedFilename: String
let sourceURL: URL
}

/// Tracks active downloads keyed by WKDownload identity.
Expand All @@ -8826,16 +8827,6 @@ class BrowserDownloadDelegate: NSObject, WKDownloadDelegate {
return dir
}()

private static func sanitizedFilename(_ raw: String, fallbackURL: URL?) -> String {
let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines)
let candidate = (trimmed as NSString).lastPathComponent
let fromURL = fallbackURL?.lastPathComponent ?? ""
let base = candidate.isEmpty ? fromURL : candidate
let replaced = base.replacingOccurrences(of: ":", with: "-")
let safe = replaced.trimmingCharacters(in: .whitespacesAndNewlines)
return safe.isEmpty ? "download" : safe
}

private func storeState(_ state: DownloadState, for download: WKDownload) {
activeDownloadsLock.lock()
activeDownloads[ObjectIdentifier(download)] = state
Expand Down Expand Up @@ -8864,18 +8855,23 @@ class BrowserDownloadDelegate: NSObject, WKDownloadDelegate {
completionHandler: @escaping (URL?) -> Void
) {
// Save to a temp file — return synchronously so WebKit is never blocked.
let safeFilename = Self.sanitizedFilename(suggestedFilename, fallbackURL: response.url)
let filenameResolver = BrowserDownloadFilenameResolver()
if case .reject = filenameResolver.httpStatusDecision(for: response) {
completionHandler(nil)
return
}
let sourceURL = response.url ?? URL(fileURLWithPath: suggestedFilename)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Semantic mismatch: filename string used as a file path

When response.url is nil, URL(fileURLWithPath: suggestedFilename) creates a file:/// URL from what is actually a bare filename string (e.g. "photo.jpg"). The resolver only uses this for lastPathComponent, so it works today, but the file:// scheme means any future caller that inspects .scheme or .host on sourceURL will get unexpected values. A URL(string:) or a direct URL(fileURLWithPath:) on FileManager.default.temporaryDirectory.appendingPathComponent(_:) would be more semantically accurate.

Suggested change
let sourceURL = response.url ?? URL(fileURLWithPath: suggestedFilename)
let sourceURL = response.url ?? URL(string: suggestedFilename.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? suggestedFilename) ?? URL(fileURLWithPath: suggestedFilename)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

let safeFilename = filenameResolver.suggestedFilename(suggestedFilename: suggestedFilename, response: response, sourceURL: sourceURL, imageType: nil)
let tempFilename = "\(UUID().uuidString)-\(safeFilename)"
let destURL = Self.tempDir.appendingPathComponent(tempFilename, isDirectory: false)
try? FileManager.default.removeItem(at: destURL)
storeState(DownloadState(tempURL: destURL, suggestedFilename: safeFilename), for: download)
storeState(DownloadState(tempURL: destURL, suggestedFilename: safeFilename, sourceURL: sourceURL), for: download)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
notifyOnMain { [weak self] in
self?.onDownloadStarted?(safeFilename)
}
#if DEBUG
cmuxDebugLog("download.decideDestination file=\(safeFilename)")
#endif
NSLog("BrowserPanel download: temp path=%@", destURL.path)
completionHandler(destURL)
}

Expand All @@ -8889,27 +8885,29 @@ class BrowserDownloadDelegate: NSObject, WKDownloadDelegate {
#if DEBUG
cmuxDebugLog("download.finished file=\(info.suggestedFilename)")
#endif
NSLog("BrowserPanel download finished: %@", info.suggestedFilename)

// Show NSSavePanel on the next runloop iteration (safe context).
DispatchQueue.main.async {
let filenameResolver = BrowserDownloadFilenameResolver()
Task { @MainActor in
let imageType = await Task.detached(priority: .utility) {
filenameResolver.imageType(forDownloadedFileAt: info.tempURL)
}.value
self.onDownloadReadyToSave?()
let suggestedFilename = filenameResolver.suggestedFilename(suggestedFilename: info.suggestedFilename, response: nil, sourceURL: info.sourceURL, imageType: imageType)
let savePanel = NSSavePanel()
savePanel.nameFieldStringValue = info.suggestedFilename
savePanel.nameFieldStringValue = suggestedFilename
savePanel.canCreateDirectories = true
savePanel.directoryURL = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask).first

savePanel.begin { result in
guard result == .OK, let destURL = savePanel.url else {
try? FileManager.default.removeItem(at: info.tempURL)
return
}
do {
try? FileManager.default.removeItem(at: destURL)
try FileManager.default.moveItem(at: info.tempURL, to: destURL)
NSLog("BrowserPanel download saved: %@", destURL.path)
if FileManager.default.fileExists(atPath: destURL.path) {
_ = try FileManager.default.replaceItemAt(destURL, withItemAt: info.tempURL)
} else {
try FileManager.default.moveItem(at: info.tempURL, to: destURL)
}
} catch {
NSLog("BrowserPanel download move failed: %@", error.localizedDescription)
try? FileManager.default.removeItem(at: info.tempURL)
}
}
Expand Down
Loading
Loading