Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Clean up uses of URLRequest and URLResponse
  • Loading branch information
MaxDesiatov committed Aug 2, 2023
commit c1b5529499d432f812937683a933177b62034cb2
9 changes: 9 additions & 0 deletions Package.resolved

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ let package = Package(
.product(name: "SymbolKit", package: "swift-docc-symbolkit"),
.product(name: "CLMDB", package: "swift-lmdb"),
.product(name: "Crypto", package: "swift-crypto"),
.product(name: "HTTPTypes", package: "swift-http-types"),
],
swiftSettings: swiftSettings
),
Expand Down Expand Up @@ -139,6 +140,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
.package(url: "https://github.com/apple/swift-docc-symbolkit", branch: "main"),
.package(url: "https://github.com/apple/swift-crypto.git", from: "2.5.0"),
.package(url: "https://github.com/apple/swift-docc-plugin", from: "1.2.0"),
.package(url: "https://github.com/apple/swift-http-types.git", .upToNextMinor(from: "0.1.0")),
]
} else {
// Building in the Swift.org CI system, so rely on local versions of dependencies.
Expand All @@ -149,5 +151,6 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
.package(path: "../swift-argument-parser"),
.package(path: "../swift-docc-symbolkit"),
.package(path: "../swift-crypto"),
.package(path: "../swift-http-types"),
]
}
53 changes: 45 additions & 8 deletions Sources/SwiftDocC/Servers/DocumentationSchemeHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@
*/

import Foundation

#if canImport(WebKit)
import WebKit
import HTTPTypes

@available(*, deprecated, renamed: "DocumentationSchemeHandler")
public typealias TopicReferenceSchemeHandler = DocumentationSchemeHandler
public class DocumentationSchemeHandler: NSObject {

public typealias FallbackResponseHandler = (URLRequest) -> (URLResponse, Data)?

enum Error: Swift.Error {
case noURLProvided
}

public typealias FallbackResponseHandler = (HTTPRequest) -> (HTTPTypes.HTTPResponse, Data)?

// The schema to support the documentation.
public static let scheme = "doc"
public static var fullScheme: String {
Expand Down Expand Up @@ -71,7 +72,7 @@ public class DocumentationSchemeHandler: NSObject {
}

/// Returns a response to a given request.
public func response(to request: URLRequest) -> (URLResponse, Data?) {
public func response(to request: HTTPRequest) -> (HTTPTypes.HTTPResponse, Data?) {
var (response, data) = fileServer.response(to: request)
if data == nil, let fallbackHandler = fallbackHandler,
let (fallbackResponse, fallbackData) = fallbackHandler(request) {
Expand All @@ -82,11 +83,47 @@ public class DocumentationSchemeHandler: NSObject {
}
}

#if canImport(WebKit)
import WebKit

// MARK: WKURLSchemeHandler protocol
extension DocumentationSchemeHandler: WKURLSchemeHandler {

public func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) {
let (response, data) = self.response(to: urlSchemeTask.request)
let request = urlSchemeTask.request

// Render authority pseudo-header in accordance with https://www.rfc-editor.org/rfc/rfc3986.html#section-3.2
let authority: String
guard let url = request.url else {
urlSchemeTask.didFailWithError(Error.noURLProvided)
return
}

let userAuthority: String
if let user = url.user {
userAuthority = "\(user)@"
} else {
userAuthority = ""
}

let portAuthority: String
if let port = url.port {
portAuthority = ":\(port)"
} else {
portAuthority = ""
}

authority = "\(userAuthority)\(url.host ?? "")\(portAuthority)"
let httpRequest = HTTPRequest(method: .get, scheme: request.url?.scheme, authority: authority, path: request.url?.path)

let (httpResponse, data) = self.response(to: httpRequest)

let response = URLResponse(
url: url,
mimeType: httpResponse.headerFields[.contentType],
expectedContentLength: httpResponse.headerFields[.contentLength].flatMap(Int.init) ?? -1,
textEncodingName: httpResponse.headerFields[.contentEncoding]
)
urlSchemeTask.didReceive(response)
if let data = data {
urlSchemeTask.didReceive(data)
Expand Down
35 changes: 18 additions & 17 deletions Sources/SwiftDocC/Servers/FileServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/

import Foundation
import HTTPTypes
import SymbolKit

fileprivate let slashCharSet = CharacterSet(charactersIn: "/")
Expand Down Expand Up @@ -53,49 +54,49 @@ public class FileServer {
/**
Returns the data for a given URL.
*/
public func data(for url: URL) -> Data? {
public func data(for path: String) -> Data? {
let providerKey = providers.keys.sorted { (l, r) -> Bool in
l.count > r.count
}.filter { (path) -> Bool in
return url.path.trimmingCharacters(in: slashCharSet).hasPrefix(path)
}.filter { (providerPath) -> Bool in
return path.trimmingCharacters(in: slashCharSet).hasPrefix(providerPath)
}.first ?? "" //in case missing an exact match, get the root one
guard let provider = providers[providerKey] else {
fatalError("A provider has not been passed to a FileServer.")
}
return provider.data(for: url.path.trimmingCharacters(in: slashCharSet).removingPrefix(providerKey))
return provider.data(for: path.trimmingCharacters(in: slashCharSet).removingPrefix(providerKey))
}

/**
Returns a tuple with a response and the given data.
- Parameter request: The request coming from a web client.
- Returns: The response and data which are going to be served to the client.
*/
public func response(to request: URLRequest) -> (URLResponse, Data?) {
guard let url = request.url else {
return (HTTPURLResponse(url: baseURL, statusCode: 400, httpVersion: "HTTP/1.1", headerFields: nil)!, nil)
public func response(to request: HTTPRequest) -> (HTTPTypes.HTTPResponse, Data?) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a source breaking change. Is there anywhere to maintain both declarations to ease this transition?

Copy link
Contributor Author

@MaxDesiatov MaxDesiatov Aug 2, 2023

Choose a reason for hiding this comment

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

I can guard the old function that uses URLRequest and URLResponse on #if canImport(Darwin) if that's ok? Otherwise its presence on non-Darwin platforms adds a dependency on FoundationNetworking with curl and the rest of the system networking/crypto libraries, which is what this PR was meant to remove in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. I'm concerned that someone could have an existing setup on Linux with curl and the rest of the system networking libraries and that their setup would break when they update to a new DocC version.

We try to provide a transition period for breaking changes. If that's not possible then so be it (although we would try to inform people about it ahead of time in the Forums). But it there's a way to stage this, then I think that it would be less disruptive that way.

Would it be possible to use a compile time define to exclude FoundationNetworking? That way we could have a 3 stage transition (first opt-in to exclude, then exclude by default, and finally remove it completely)

guard let path = request.path as NSString? else {
return (.init(status: 400), nil)
}
var data: Data? = nil
let response: URLResponse
let response: HTTPTypes.HTTPResponse

let mimeType: String

// We need to make sure that the path extension is for an actual file and not a symbol name which is a false positive
// like: "'...(_:)-6u3ic", that would be recognized as filename with the extension "(_:)-6u3ic". (rdar://71856738)
if url.pathExtension.isAlphanumeric && !url.lastPathComponent.isSwiftEntity {
data = self.data(for: url)
mimeType = FileServer.mimeType(for: url.pathExtension)
if path.pathExtension.isAlphanumeric && !path.lastPathComponent.isSwiftEntity {
data = self.data(for: path as String)
mimeType = FileServer.mimeType(for: path.pathExtension)
} else { // request is for a path, we need to fake a redirect here
if url.pathComponents.isEmpty {
xlog("Tried to load an invalid URL: \(url.absoluteString).\nFalling back to serve index.html.")
if path.pathComponents.isEmpty {
xlog("Tried to load an invalid path: \(path).\nFalling back to serve index.html.")
}
mimeType = "text/html"
data = self.data(for: baseURL.appendingPathComponent("/index.html"))
data = self.data(for: path.appendingPathComponent("/index.html"))
}

if let data = data {
response = URLResponse(url: url, mimeType: mimeType, expectedContentLength: data.count, textEncodingName: nil)
response = .init(status: .ok, headerFields: [.contentType: mimeType, .contentLength: "\(data.count)"])
} else {
response = URLResponse(url: url, mimeType: nil, expectedContentLength: 0, textEncodingName: nil)
response = .init(status: .ok, headerFields: [.contentType: mimeType])
}

return (response, data)
Expand Down
27 changes: 10 additions & 17 deletions Tests/SwiftDocCTests/Servers/DocumentationSchemeHandlerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/

import Foundation
import HTTPTypes

import XCTest
@testable import SwiftDocC
Expand All @@ -21,41 +22,37 @@ class DocumentationSchemeHandlerTests: XCTestCase {
forResource: "TestBundle", withExtension: "docc", subdirectory: "Test Bundles")!

func testDocumentationSchemeHandler() {
#if !os(Linux) && !os(Android) && !os(Windows)
let topicSchemeHandler = DocumentationSchemeHandler(withTemplateURL: templateURL)

let request = URLRequest(url: baseURL.appendingPathComponent("/images/figure1.jpg"))
let request = HTTPRequest(path: "/images/figure1.jpg")

var (response, data) = topicSchemeHandler.response(to: request)
XCTAssertNotNil(data)
XCTAssertEqual(response.mimeType, "image/jpeg")

let failingRequest = URLRequest(url: baseURL.appendingPathComponent("/not/found.jpg"))
let failingRequest = HTTPRequest(path: "/not/found.jpg")
(response, data) = topicSchemeHandler.response(to: failingRequest)
XCTAssertNil(data)

topicSchemeHandler.fallbackHandler = { (request: URLRequest) -> (URLResponse, Data)? in
guard let url = request.url else { return nil }
let response = URLResponse(url: url, mimeType: "text/html", expectedContentLength: helloWorldHTML.count, textEncodingName: nil)
topicSchemeHandler.fallbackHandler = { (request: HTTPRequest) -> (HTTPTypes.HTTPResponse, Data)? in
let response = HTTPResponse(mimeType: "text/html", expectedContentLength: helloWorldHTML.count)
return (response, helloWorldHTML)
}

(response, data) = topicSchemeHandler.response(to: failingRequest)
XCTAssertEqual(data, helloWorldHTML)
XCTAssertEqual(response.mimeType, "text/html")
#endif
}

func testSetData() {
#if !os(Linux) && !os(Android) && !os(Windows)
let topicSchemeHandler = DocumentationSchemeHandler(withTemplateURL: templateURL)

let data = "hello!".data(using: .utf8)!
topicSchemeHandler.setData(data: ["a.txt": data])

XCTAssertEqual(
topicSchemeHandler.response(
to: URLRequest(url: baseURL.appendingPathComponent("/data/a.txt"))
to: HTTPRequest(path: "/data/a.txt")
).1,
data
)
Expand All @@ -64,20 +61,16 @@ class DocumentationSchemeHandlerTests: XCTestCase {

XCTAssertEqual(
topicSchemeHandler.response(
to: URLRequest(url: baseURL.appendingPathComponent("/data/b.txt"))
to: HTTPRequest(path: "/data/b.txt")
).1,
data
)

XCTAssertNil(
topicSchemeHandler.response(
to: URLRequest(url: baseURL.appendingPathComponent("/data/a.txt"))
to: HTTPRequest(path: "/data/a.txt")
).1,
"a.txt should have been deleted because we set the daata to b.txt."
"a.txt should have been deleted because we set the data to b.txt."
)
#endif
}
}



Loading