[swift-server-dev] Prototype of the discussed HTTP API Spec

[Helge Heß]

On 26. May 2017, at 22:00, Chris Bailey via swift-server-dev <swift-server-dev at swift.org> wrote:
> HTTPRequest:        https://github.com/carlbrown/HTTPSketch/blob/master/Sources/HTTPSketch/HTTPRequest.swift 


  /// HTTP Methods handled by http_parser.[ch] supports
  public enum HTTPMethod: String {
    // case custom(method: String)
    case UNKNOWN

Don’t restrict the API to a specific HTTP parser implementation. The number of methods supported by http_parser changed multiple times in the past (I myself added one to the official tree).

I still highly recommend using constants instead of an enum here. The methods *will* change in the future. (At least BATCH is going to be added)
At the minimum do the custom(String) variant. UNKNOWN seems unacceptable to me.

In the same run, I don’t understand why an enum is used for methods but not for headers. Extra weird because the method is just a header in HTTP/2 …
I’d prefer constants for both.


> func writeBody(data: DispatchData, completion: @escaping (Result<POSIXError, ()>) -> Void)

Do we really want to expose POSIXError’s? I’d say have an own error type for HTTP level errors and maybe something like this:

  enum HTTPError {
    case connectionClosed(cause: POSIXError?)
    ...
  }

Also: DispatchData vs Data vs Ptr.


> public enum HTTPResponseStatus: UInt, RawRepresentable {
>    /* The original spec used custom if you want to use a non-standard response code or
>     have it available in a (UInt, String) pair from a higher-level web framework. 
>     
>     Swift 3 doesn't like it - will revisit in Swift 4*/
>    //case custom(code: UInt, reasonPhrase: String)

Swift 3 does like it, you just can’t combine raw w/ extra values. That just doesn’t make sense. I think the only thing you could hope for is that this works on a raw:

  case custom(code: UInt)

Also, why a `UInt`? Do you foresee 64-bit status codes? Should be UInt16.


Again: I’d prefer constants because status codes are defined as part of RFCs and
will *definitely* change in the future. Something like

  struct HTTPStatus {
    let code : UInt16

    static let `continue` : UInt16 = 100
    etc.
  }

That is future and source-compat proof.


> public struct HTTPHeaders {
>     var storage: [String:[String]]     /* lower cased keys */
>     var original: [(String, String)]   /* original casing */

I don’t think the storage semantics should be part of the API. I’d prefer
a protocol here providing indexed access.


Finally, I still propose calling it HTTPRequestHead, HTTPResponseHead. The body belongs to the request/response. Don’t care too much, but would be better.

hh