- 
                Notifications
    You must be signed in to change notification settings 
- Fork 159
Avoid FoundationNetworking and libcurl on non-Darwin platforms #681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
66353a1
              0db65fa
              c1b5529
              992d221
              58a5d0e
              f042a68
              71fe790
              8c4e6d0
              fb38bef
              f2b5908
              ea5bccb
              98561ac
              30500a0
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -9,10 +9,8 @@ | |
| */ | ||
|  | ||
| import Foundation | ||
| import HTTPTypes | ||
| import SymbolKit | ||
| #if canImport(FoundationNetworking) | ||
| import FoundationNetworking | ||
| #endif | ||
|  | ||
| fileprivate let slashCharSet = CharacterSet(charactersIn: "/") | ||
|  | ||
|  | @@ -56,49 +54,54 @@ public class FileServer { | |
| /** | ||
| Returns the data for a given URL. | ||
| */ | ||
| @available(*, deprecated, message: "Use 'data(for path: String)' instead.") | ||
| public func data(for url: URL) -> Data? { | ||
| return data(for: url.path) | ||
| } | ||
|  | ||
| public func data(for path: String) -> Data? { | ||
|         
                  MaxDesiatov marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
| 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?) { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can guard the old function that uses  There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: "/index.html") | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why this isn't a change in behavior? What's needless about this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only  Additionally,  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If  | ||
| } | ||
|  | ||
| 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: "application/octet-stream"]) | ||
| } | ||
|  | ||
| return (response, data) | ||
|  | @@ -175,7 +178,8 @@ public class FileSystemServerProvider: FileServerProvider { | |
|  | ||
| public func data(for path: String) -> Data? { | ||
| let finalURL = directoryURL.appendingPathComponent(path) | ||
| return try? Data(contentsOf: finalURL) | ||
| let fileHandle = try? FileHandle(forReadingFrom: finalURL) | ||
| return try? fileHandle?.readToEnd() | ||
| } | ||
|  | ||
| } | ||
|  | @@ -230,7 +234,12 @@ public class MemoryFileServerProvider: FileServerProvider { | |
|  | ||
| for file in enumerator { | ||
| guard let file = file as? String else { fatalError("Enumerator returned an unexpected type.") } | ||
| guard let data = try? Data(contentsOf: URL(fileURLWithPath: path).appendingPathComponent(file)) else { continue } | ||
| let fileURL = URL(fileURLWithPath: path).appendingPathComponent(file) | ||
| guard | ||
| let fileHandle = try? FileHandle(forReadingFrom: fileURL), | ||
| let data = try? fileHandle.readToEnd() | ||
| else { continue } | ||
|  | ||
| if recursive == false && file.contains("/") { continue } // skip if subfolder and recursive is disabled | ||
| addFile(path: "/\(trimmedSubPath)/\(file)", data: data) | ||
| } | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped to
.v11per a discussion with @franklinsch, this allows us to use throwingFileHandle.readToEnd()unlike the unsafe non-throwingFileHandle.readDataToEndOfFilethat crashes the test suite on Linux.