From 878ab7f18b467580844d2971970297703d2679ff Mon Sep 17 00:00:00 2001 From: Fuwn Date: Mon, 23 Feb 2026 11:11:20 -0800 Subject: fix: guard post detail url actions against invalid urls --- Sora/Views/Post/Details/PostDetailsImageView.swift | 38 ++++++++-- Sora/Views/Post/Details/PostDetailsView.swift | 8 +- SoraTests/ViewDerivedDataTests.swift | 88 +++++++++++++++++++--- 3 files changed, 111 insertions(+), 23 deletions(-) diff --git a/Sora/Views/Post/Details/PostDetailsImageView.swift b/Sora/Views/Post/Details/PostDetailsImageView.swift index dda1356..1e0da72 100644 --- a/Sora/Views/Post/Details/PostDetailsImageView.swift +++ b/Sora/Views/Post/Details/PostDetailsImageView.swift @@ -34,14 +34,17 @@ struct PostDetailsImageView: View { #if os(iOS) if settings.enableShareShortcut { Button { + guard let shareURL = url else { return } + keyWindow?.rootViewController?.present( UIActivityViewController( - activityItems: [url ?? URL(string: "")!], applicationActivities: nil + activityItems: [shareURL], applicationActivities: nil ), animated: true ) } label: { Label("Share", systemImage: "square.and.arrow.up") } + .disabled(url == nil) } #endif @@ -96,14 +99,19 @@ struct PostDetailsImageView: View { .keyboardShortcut("c", modifiers: [.command]) Button { - openURL(postURL(for: post?.id ?? "")) + guard let postURL = postURL(for: post?.id) else { return } + + openURL(postURL) } label: { Label("Open Post in Safari", systemImage: "safari") } + .disabled(postURL(for: post?.id) == nil) - if let source = post?.source { + if let source = post?.source, + let sourceURL = URL(string: source.trimmingCharacters(in: .whitespacesAndNewlines)) + { Button { - openURL(URL(string: source)!) + openURL(sourceURL) } label: { Label("Open Source Link in Safari", systemImage: "safari") } @@ -170,17 +178,31 @@ struct PostDetailsImageView: View { self.post = post } - private func postURL(for id: String) -> URL { + private func postURL(for id: String?) -> URL? { + guard let id, !id.isEmpty else { return nil } + + var components = URLComponents() + + components.scheme = "https" + components.host = manager.domain + switch manager.flavor { case .moebooru: - return URL(string: "https://\(manager.domain)/post/show/\(id)")! + components.path = "/post/show/\(id)" case .gelbooru: - return URL(string: "https://\(manager.domain)/index.php?page=post&s=view&id=\(id)")! + components.path = "/index.php" + components.queryItems = [ + URLQueryItem(name: "page", value: "post"), + URLQueryItem(name: "s", value: "view"), + URLQueryItem(name: "id", value: id), + ] case .danbooru: - return URL(string: "https://\(manager.domain)/posts/\(id)")! + components.path = "/posts/\(id)" } + + return components.url } #if os(macOS) diff --git a/Sora/Views/Post/Details/PostDetailsView.swift b/Sora/Views/Post/Details/PostDetailsView.swift index 8dc95d4..9c55798 100644 --- a/Sora/Views/Post/Details/PostDetailsView.swift +++ b/Sora/Views/Post/Details/PostDetailsView.swift @@ -129,9 +129,11 @@ struct PostDetailsView: View { #if os(macOS) if settings.enableShareShortcut { - ToolbarItem { - ShareLink(item: imageURL!) { - Label("Share", systemImage: "square.and.arrow.up") + if let imageURL { + ToolbarItem { + ShareLink(item: imageURL) { + Label("Share", systemImage: "square.and.arrow.up") + } } } } diff --git a/SoraTests/ViewDerivedDataTests.swift b/SoraTests/ViewDerivedDataTests.swift index 9152591..c20e9d1 100644 --- a/SoraTests/ViewDerivedDataTests.swift +++ b/SoraTests/ViewDerivedDataTests.swift @@ -193,6 +193,58 @@ final class ViewDerivedDataTests: XCTestCase { // swiftlint:disable:this type_b ) } + func testPostDetailsImageViewAvoidsForceUnwrappedRuntimeURLs() throws { + let source = try loadSource(at: "Sora/Views/Post/Details/PostDetailsImageView.swift") + let normalizedSource = strippingCommentsAndStrings(from: source) + let forcedShareFallbackURLCount = tokenCount( + matching: #"\burl\s*\?\?\s*URL\s*\(\s*string:\s*\)\s*!"#, + in: normalizedSource + ) + let forcedSourceURLCount = tokenCount( + matching: #"\bURL\s*\(\s*string:\s*source\s*\)\s*!"#, + in: normalizedSource + ) + let forcedPostURLBuilderCount = tokenCount( + matching: #"\breturn\s+URL\s*\(\s*string:\s*[^)]+\)\s*!"#, + in: normalizedSource + ) + + // swiftlint:disable:next prefer_nimble + XCTAssertEqual( + forcedShareFallbackURLCount, + 0, + "Post details share actions should not force unwrap fallback URLs." + ) + // swiftlint:disable:next prefer_nimble + XCTAssertEqual( + forcedSourceURLCount, + 0, + "Post details source links should be validated before opening." + ) + // swiftlint:disable:next prefer_nimble + XCTAssertEqual( + forcedPostURLBuilderCount, + 0, + "Post details post-url helpers should return optional URLs instead of force-unwrapping." + ) + } + + func testPostDetailsViewAvoidsForceUnwrappedShareURL() throws { + let source = try loadSource(at: "Sora/Views/Post/Details/PostDetailsView.swift") + let normalizedSource = strippingCommentsAndStrings(from: source) + let forcedShareItemCount = tokenCount( + matching: #"\bShareLink\s*\(\s*item:\s*imageURL\s*!"#, + in: normalizedSource + ) + + // swiftlint:disable:next prefer_nimble + XCTAssertEqual( + forcedShareItemCount, + 0, + "Post details share actions should not force unwrap image URLs." + ) + } + func testListViewsAvoidComparatorRandomShuffleSorting() throws { let listViewSource = try loadSource(at: "Sora/Views/Generic/GenericListView.swift") let favoritesViewSource = try loadSource(at: "Sora/Views/FavoritesView.swift") @@ -304,9 +356,11 @@ final class ViewDerivedDataTests: XCTestCase { // swiftlint:disable:this type_b named: "func url(forPosts page: Int, limit: Int, tags: [String]) -> URL?", from: source ) - let danbooruCaseStart = try XCTUnwrap(urlBuilderSection.range(of: "case .danbooru:")?.lowerBound) + let danbooruCaseStart = try XCTUnwrap( + urlBuilderSection.range(of: "case .danbooru:")?.lowerBound) let danbooruCaseEnd = try XCTUnwrap( - urlBuilderSection.range(of: "case .moebooru:", range: danbooruCaseStart.. URL?", from: source ) - let moebooruCaseStart = try XCTUnwrap(urlBuilderSection.range(of: "case .moebooru:")?.lowerBound) + let moebooruCaseStart = try XCTUnwrap( + urlBuilderSection.range(of: "case .moebooru:")?.lowerBound) let moebooruCaseEnd = try XCTUnwrap( - urlBuilderSection.range(of: "case .gelbooru:", range: moebooruCaseStart.. URL?", from: source ) - let danbooruCaseStart = try XCTUnwrap(urlBuilderSection.range(of: "case .danbooru:")?.lowerBound) + let danbooruCaseStart = try XCTUnwrap( + urlBuilderSection.range(of: "case .danbooru:")?.lowerBound) let danbooruCaseEnd = try XCTUnwrap( - urlBuilderSection.range(of: "case .moebooru:", range: danbooruCaseStart..