Convert HTTPVersion to struct

Having the HTTPVersion as a struct allows it to be serialized in the
request/response line in HTTP 1.1 and in the headers section in HTTP 2.0
with no problems. I don't see a problem with the current implementation for
HTTP 2.0.

GET / HTTP/1.1
Host: server.example.com
Connection: Upgrade, HTTP2-Settings
Upgrade: h2c
HTTP2-Settings: <base64url encoding of HTTP/2 SETTINGS payload>

HTTP/1.1 101 Switching Protocols
Connection: Upgrade
Upgrade: h2c

this is an upgrade from HTTP 1.1 to HTTP 2.0.. the first request and
response are HTTP 1.1 requests where the version goes in the
request/response line. The upgrade headers are an entirely different issue.

If we're not upgrading and creating an HTTP 2.0 request directly. The
version of the HTTPRequest type will be used to serialize the type
accordingly. How you're going to use that info is not directly related to
where it will be stored or how it will be serialized. It's just a piece of
information that's important for that type. Therefore I don't see an issue
with the current implementation when it comes to HTTP 2.0.

···

On 14 June 2017 at 10:27, Tanner Nelson via swift-server-dev < swift-server-dev@swift.org> wrote:

On Wed, Jun 14, 2017 at 2:11 PM Helge Heß via swift-server-dev < > swift-server-dev@swift.org> wrote:

On 14 Jun 2017, at 14:59, Paulo Faria <paulo@zewo.io> wrote:
> You're completely right we should care about HTTP 2.0 now. But let's do
it one step at a time, or else we won't get things done. The current topic
is the HTTPVersion type. So could you please give your feedback in the
HTTPVersion thread about how the current proposal of HTTPversion fits with
HTTP 2.0? We should go from lower abstractions to higher abstractions
incrementally but definitely considering the upper abstractions. Let's keep
focus and move on! :blush:

I gave my feedback on the type in the this thread already. Struct is fine
for me, tuple is OK too. I don’t actually care much as “The HTTP version
should only matter for logging purposes”. If it comes with comparison
operations and such, something is likely wrong with the API.

The only HTTP/2 specific note I have is this thing:

  From the HTTPRequest API design perspective there is a small
  change in that method/version are just headers in HTTP/2.
  I brought that up before.
  E.g. why does ‘method' deserve a special enum, but not
  content-type etc which is equally important for dispatching
  a request. Either approach is fine though.)

In that case, maybe we ditch the version and method properties in favor of
something more generic like:

(just spitballing some pseudo swift here)


    struct Request {
        ...
        enum Metadata {
            case original(major: Int, minor: Int, Method)
            case headers
        }

        var metadata: Metadata
        ...
    }

    extension Request {
        var method: Method {
            switch metadata {
            case .original(_, _, let method):
                return method
            case .headers:
                return Method(rawValue: headers["Method"])
            }
        }
    }

Here an enum's ability to exhaustively switch would be useful. Then
`req.method` is implemented as an extension similar to how `req.version`
and `req.contentType` could be implemented.

This affects the discussion as it may make sense to expose the
HTTPVersion as a regular header to mirror the new HTTP/2 setup
instead of basing the API on the old HTTP/0,1 model.
(I think either is fine, with a slight preference for going
with the ‘future’)

Or in other words: Why struct or tuple, just keep it as a string
like the other headers.

If HTTPVersion is not exposed as a String but as a specific type,
that would then affect the way headers in general are handled
(and I’m very much in favour of NOT using strings for the common
ones for various reasons).

hh

_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev

_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev

> You're completely right we should care about HTTP 2.0 now. But let's do
it one step at a time, or else we won't get things done. The current topic
is the HTTPVersion type. So could you please give your feedback in the
HTTPVersion thread about how the current proposal of HTTPversion fits with
HTTP 2.0? We should go from lower abstractions to higher abstractions
incrementally but definitely considering the upper abstractions. Let's keep
focus and move on! :blush:

I gave my feedback on the type in the this thread already. Struct is fine
for me, tuple is OK too. I don’t actually care much as “The HTTP version
should only matter for logging purposes”. If it comes with comparison
operations and such, something is likely wrong with the API.

The only HTTP/2 specific note I have is this thing:

  From the HTTPRequest API design perspective there is a small
  change in that method/version are just headers in HTTP/2.
  I brought that up before.
  E.g. why does ‘method' deserve a special enum, but not
  content-type etc which is equally important for dispatching
  a request. Either approach is fine though.)

In that case, maybe we ditch the version and method properties in favor of
something more generic like:

(just spitballing some pseudo swift here)


    struct Request {
        ...
        enum Metadata {
            case original(major: Int, minor: Int, Method)
            case headers
        }

        var metadata: Metadata
        ...
    }

    extension Request {
        var method: Method {
            switch metadata {
            case .original(_, _, let method):
                return method
            case .headers:
                return Method(rawValue: headers["Method"])
            }
        }
    }

Here an enum's ability to exhaustively switch would be useful. Then
`req.method` is implemented as an extension similar to how `req.version`
and `req.contentType` could be implemented.

···

On Wed, Jun 14, 2017 at 2:11 PM Helge Heß via swift-server-dev < swift-server-dev@swift.org> wrote:

On 14 Jun 2017, at 14:59, Paulo Faria <paulo@zewo.io> wrote:

This affects the discussion as it may make sense to expose the
HTTPVersion as a regular header to mirror the new HTTP/2 setup
instead of basing the API on the old HTTP/0,1 model.
(I think either is fine, with a slight preference for going
with the ‘future’)

Or in other words: Why struct or tuple, just keep it as a string
like the other headers.

If HTTPVersion is not exposed as a String but as a specific type,
that would then affect the way headers in general are handled
(and I’m very much in favour of NOT using strings for the common
ones for various reasons).

hh

_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev

It's worth noting that HTTP/2 refers to the method header as a special
"pseudo-header".

All HTTP/2 requests MUST include exactly one valid value for the :method,

:scheme, and :path pseudo-header fields.
https://http2.github.io/http2-spec/#rfc.section.8.1.2.3

So even though it's being passed as a "header", it's a special header
distinct from ones like content-type which could merit it continuing to be
a stored property on Request.

···

On Wed, Jun 14, 2017 at 2:41 PM Michael Chiu <hatsuneyuji@icloud.com> wrote:

So are we going to write our own http parser?

If not, I don’t see the benefit of ditching version and method properties
since both major/minor version and method are given by the http_parser, and
users are expecting to find it in the property since these components are
well defined in RFC and guaranteed its existence in a proper http req/res.
Hence they deserve to be an independent property.

On the other hand, there’s no one guarantee the existence of Content-Type
and Content-Length etc header exists in HTTP protocol (I can be wrong).

Michael.

On Jun 14, 2017, at 6:27 AM, Tanner Nelson via swift-server-dev < > swift-server-dev@swift.org> wrote:

On Wed, Jun 14, 2017 at 2:11 PM Helge Heß via swift-server-dev < > swift-server-dev@swift.org> wrote:

On 14 Jun 2017, at 14:59, Paulo Faria <paulo@zewo.io> wrote:
> You're completely right we should care about HTTP 2.0 now. But let's do
it one step at a time, or else we won't get things done. The current topic
is the HTTPVersion type. So could you please give your feedback in the
HTTPVersion thread about how the current proposal of HTTPversion fits with
HTTP 2.0? We should go from lower abstractions to higher abstractions
incrementally but definitely considering the upper abstractions. Let's keep
focus and move on! :blush:

I gave my feedback on the type in the this thread already. Struct is fine
for me, tuple is OK too. I don’t actually care much as “The HTTP version
should only matter for logging purposes”. If it comes with comparison
operations and such, something is likely wrong with the API.

The only HTTP/2 specific note I have is this thing:

  From the HTTPRequest API design perspective there is a small
  change in that method/version are just headers in HTTP/2.
  I brought that up before.
  E.g. why does ‘method' deserve a special enum, but not
  content-type etc which is equally important for dispatching
  a request. Either approach is fine though.)

In that case, maybe we ditch the version and method properties in favor of
something more generic like:

(just spitballing some pseudo swift here)


    struct Request {
        ...
        enum Metadata {
            case original(major: Int, minor: Int, Method)
            case headers
        }

        var metadata: Metadata
        ...
    }

    extension Request {
        var method: Method {
            switch metadata {
            case .original(_, _, let method):
                return method
            case .headers:
                return Method(rawValue: headers["Method"])
            }
        }
    }

Here an enum's ability to exhaustively switch would be useful. Then
`req.method` is implemented as an extension similar to how `req.version`
and `req.contentType` could be implemented.

This affects the discussion as it may make sense to expose the
HTTPVersion as a regular header to mirror the new HTTP/2 setup
instead of basing the API on the old HTTP/0,1 model.
(I think either is fine, with a slight preference for going
with the ‘future’)

Or in other words: Why struct or tuple, just keep it as a string
like the other headers.

If HTTPVersion is not exposed as a String but as a specific type,
that would then affect the way headers in general are handled
(and I’m very much in favour of NOT using strings for the common
ones for various reasons).

hh

_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev

_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev

Based on where we are at the moment - which is dealing with HTTPVersion as
a separate type - it sounds like we're converging on using a struct. As
such if there's no strong objection I'll merge the PR as-is.

That doesn't close down the conversation with respect to what we
could/should do for HTTP/2 - just spins it off separately.

Chris

To: Tanner Nelson <tanner@vapor.codes>
Cc: Paulo Faria via swift-server-dev <swift-server-dev@swift.org>
Date: 14/06/2017 15:19
Subject: Re: [swift-server-dev] Convert HTTPVersion to struct
Sent by: swift-server-dev-bounces@swift.org

So are we going to write our own http parser?

If not, I don’t see the benefit of ditching version and method
properties since both major/minor version and method are given by
the http_parser, and users are expecting to find it in the property
since these components are well defined in RFC and guaranteed its
existence in a proper http req/res. Hence they deserve to be an
independent property.

On the other hand, there’s no one guarantee the existence of
Content-Type and Content-Length etc header exists in HTTP protocol
(I can be wrong).

Michael.

> You're completely right we should care about HTTP 2.0 now. But
let's do it one step at a time, or else we won't get things done.
The current topic is the HTTPVersion type. So could you please give
your feedback in the HTTPVersion thread about how the current
proposal of HTTPversion fits with HTTP 2.0? We should go from lower
abstractions to higher abstractions incrementally but definitely
considering the upper abstractions. Let's keep focus and move on! :blush:

I gave my feedback on the type in the this thread already. Struct is
fine for me, tuple is OK too. I don’t actually care much as “The
HTTP version should only matter for logging purposes”. If it comes
with comparison operations and such, something is likely wrong with the

API.

The only HTTP/2 specific note I have is this thing:

  From the HTTPRequest API design perspective there is a small
  change in that method/version are just headers in HTTP/2.
  I brought that up before.
  E.g. why does ‘method' deserve a special enum, but not
  content-type etc which is equally important for dispatching
  a request. Either approach is fine though.)

In that case, maybe we ditch the version and method properties in
favor of something more generic like:

(just spitballing some pseudo swift here)


    struct Request {
        ...
        enum Metadata {
            case original(major: Int, minor: Int, Method)
            case headers
        }

        var metadata: Metadata
        ...
    }

    extension Request {
        var method: Method {
            switch metadata {
            case .original(_, _, let method):
                return method
            case .headers:
                return Method(rawValue: headers["Method"])
            }
        }
    }

Here an enum's ability to exhaustively switch would be useful. Then
`req.method` is implemented as an extension similar to how
`req.version` and `req.contentType` could be implemented.

This affects the discussion as it may make sense to expose the
HTTPVersion as a regular header to mirror the new HTTP/2 setup
instead of basing the API on the old HTTP/0,1 model.
(I think either is fine, with a slight preference for going
with the ‘future’)

Or in other words: Why struct or tuple, just keep it as a string
like the other headers.

If HTTPVersion is not exposed as a String but as a specific type,
that would then affect the way headers in general are handled
(and I’m very much in favour of NOT using strings for the common
ones for various reasons).

hh

_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev
_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev
_______________________________________________
swift-server-dev mailing list
swift-server-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-server-dev

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

···

swift-server-dev-bounces@swift.org wrote on 14/06/2017 14:41:48: > From: Michael Chiu via swift-server-dev <swift-server-dev@swift.org>

On Jun 14, 2017, at 6:27 AM, Tanner Nelson via swift-server-dev < > swift-server-dev@swift.org> wrote:
On Wed, Jun 14, 2017 at 2:11 PM Helge Heß via swift-server-dev < > swift-server-dev@swift.org> wrote:
On 14 Jun 2017, at 14:59, Paulo Faria <paulo@zewo.io> wrote: