Proposal: Strictly-typed HTTPHeader values


(Amir Abbas Mousavian) #1

I strongly believe we should implement typed header here. Real
frameworks which would be placed atop of this framework must focus on
things like "what is appropriate response (header and body) for this
request" or "how to implement authentication" or "validating
input/output" rather than basic things like "parsing headers" or
"(de)compressing data" or "decrypting/encrypting".

I think this PR falls into "Provide low level HTTP parsing" category
listed in https://github.com/swift-server/work-group#current-focus-areas
These are common headers and every "real framework" have to implement
this independently! It will parse raw strings into meaningful
Foundation or Swift types and allows cleaner code (using switch case)
and much less unintended and undetectable bugs which means secure
codes. These accessors would decrease code lines significantly and
would encourage developers to focus on "logic" instead of "handworks"

We are not changing that underlying `[(String:String)]` architecture
and these accessors are lazy-loaded thus won't impose performance
overhead on loading. Strict typed storage will hit performance
significantly and will restrict users to have custom header values.

···

Date: Sun, 19 Nov 2017 17:07:59 +0100
From: Helge Heß <me@helgehess.eu>

Hi,

as I already wrote in the issue:

I think we already discussed this kind of stuff and decided not to do typed headers at this level of the framework. It would be part of a real framework on top of this driver.

If we really go the route of typed headers (which I personally kinda like for efficiency), we should actually do that in the storage itself, not just wrapping the String key/value. I wrote some email on that in the list.

Since `[(String:String)]` seems to be what people decided to use (we discussed that a few times, too), I think this stuff should just live in a separate package (which higher level frameworks can use or not). It far exceeds the minimalist route this thing was trying to go.

hh

On 17. Nov 2017, at 17:39, Amir Abbas Mousavian via swift-server-dev <swift-server-dev@swift.org> wrote:

HTTP headers are simply defined as a pair of strings. But for many of headers there is a set of defined values or a structured value. Current implementation is prone to typo and human errors which will raise hard-to-debug errors. So I propose to have strictly-typed getters/setters for frequent ones.

I implemented my proposal draft in https://github.com/swift-server/http/pull/104

:memo: Please note:

- `Cookie`, `Set-Cookie` and ` Authorization` accessors are not implemented yet and marked as TODO in source code. Also test cases are not implemented yet. I will implement them in this or another PR.

- I believe raw string should be accessible by developer thus better to implement as HTTPHeaders properties rather than typing storage object. This design allows lazy parsing.

- Some setters are not necessary for a server (marked as Request headers). I implemented them but we can discuss to remove or having them in portfolio.

- Getter accessors of headers such as Accept-Encoding returns sorted values according to `q` parameter, thus developer would not need to know exact value of q parameter imo.

- Validating input values is out of scope. It imposes overhead with little benefit, e.g. EntryTag.wildcard is not a valid value for eTag, but it won't be checked by setter. Same situation for contentCache.

- ContentType struct doesn't embed parameters such as charset. I’ve implemented charset as a separate property `contentCharset` which manipulates underlying Content-Type. We may revise this design and implementing a struct which holds other parameters too.


(Helge Heß) #2

We are not changing that underlying `[(String:String)]` architecture

I suppose this is my biggest complaint w/ the PR and why I think it doesn’t belong here.
Either we do typed headers, then the API should vend them as such.
Or we don’t, then such can be part of a different framework.

These are common headers and every "real framework" have to implement
this independently!

They could still pull in your `make-headers-nice-my-way` package, if they think it is nice. For the frameworks I’m working on the provided functionality is too high level and not required (I certainly wouldn’t want to parse an Accept into an Array<Enum<String|*>> just to check a match).
So at least for me it is just baggage I don’t need, and I think we’d want to avoid bloat in this specific library.

However, as I said I'm personally in favor of typed headers and would prefer them over Strings a lot. But I don’t have any new arguments which would change the past discussion :slight_smile:

Should we have a shared cookie parser? Or an HTTP date parser? I think that would make some sense. But that should be standalone class/type/func which can be applied on a buffer passed in in the context the higher level framework sees fit.
It should not be attached to the response API (how cookies are presented is very framework specific).

and these accessors are lazy-loaded thus won't impose performance
overhead on loading. Strict typed storage will hit performance
significantly and will restrict users to have custom header values.

You are confusing typed headers w/ early parsing. Those are distinct concerns.

The type representing the header value could still decide whether it is better to parse the value very early (e.g. Content-Length), or whether it would do it later (Cookie maybe).

hh

···

On Nov 19, 2017, at 11:35 PM, Amir Abbas Mousavian via swift-server-dev <swift-server-dev@swift.org> wrote:


(Amir Abbas Mousavian) #3

There is no way to have a typed dictionary in swift, I'm afraid.
Unless we store data as [String: [Any]] and developers have to take
responsibility of down-casting, which is not a good and practical
design imo. Or there is something new in Swift which I'm not aware of.
If you think there is a better design, please give a sample or draft.

Another issue with typed storage is - as I noted before - performance
of converting and casting. It must be lazy and not being part of
initializer or append/replace methods. Furthermore, having a
string-based storage gives much more flexibility to developer.

I believe this framework should implement every aspect regard
transport layer and parsing, and allows other frameworks to implement
logic instead of working with raw data. Additionally, integrating
typed header values as properties keeps room for further
optimizations.

Making data and cookie parsers public is a separate issue. We can
discuss about them. We can make current implementation public anytime.

···

On Mon, Nov 20, 2017 at 2:51 AM, Helge Heß via swift-server-dev <swift-server-dev@swift.org> wrote:

On Nov 19, 2017, at 11:35 PM, Amir Abbas Mousavian via swift-server-dev <swift-server-dev@swift.org> wrote:

We are not changing that underlying `[(String:String)]` architecture

I suppose this is my biggest complaint w/ the PR and why I think it doesn’t belong here.
Either we do typed headers, then the API should vend them as such.
Or we don’t, then such can be part of a different framework.

These are common headers and every "real framework" have to implement
this independently!

They could still pull in your `make-headers-nice-my-way` package, if they think it is nice. For the frameworks I’m working on the provided functionality is too high level and not required (I certainly wouldn’t want to parse an Accept into an Array<Enum<String|*>> just to check a match).
So at least for me it is just baggage I don’t need, and I think we’d want to avoid bloat in this specific library.

However, as I said I'm personally in favor of typed headers and would prefer them over Strings a lot. But I don’t have any new arguments which would change the past discussion :slight_smile:

Should we have a shared cookie parser? Or an HTTP date parser? I think that would make some sense. But that should be standalone class/type/func which can be applied on a buffer passed in in the context the higher level framework sees fit.
It should not be attached to the response API (how cookies are presented is very framework specific).

and these accessors are lazy-loaded thus won't impose performance
overhead on loading. Strict typed storage will hit performance
significantly and will restrict users to have custom header values.

You are confusing typed headers w/ early parsing. Those are distinct concerns.

The type representing the header value could still decide whether it is better to parse the value very early (e.g. Content-Length), or whether it would do it later (Cookie maybe).

hh


(Helge Heß) #4

If you think there is a better design, please give a sample or draft.

Well, as mentioned we had the topic of typed headers on this list before and decided not to do them but the String stuff we have now. There is little point in designing something we don’t want :slight_smile:

Otherwise I have no new input wrt my email from 2017-11-20. I agree that it might be useful to have and share low-level, standalone parsers for some of the common fields (maybe you could split the PR into the actual parsers and the additions to the types?)
IMO PR-104 is way too much extra. I’m against taking it, but this is really just my personal opinion - the steering team may have a completely different view on this :slight_smile:

hh

P.S.: Just to be clear: I think that PR-104 is _one_ valid approach. It just doesn’t belong into this specific layer. HTTP is supposed to be a small and *non-opinioned* framework which can be shared by higher level frameworks. Which then have their own ways to represent stuff like headers to the framework user.

···

On 25. Nov 2017, at 15:13, Amir Abbas Mousavian via swift-server-dev <swift-server-dev@swift.org> wrote:

Another issue with typed storage is - as I noted before - performance
of converting and casting. It must be lazy and not being part of
initializer or append/replace methods. Furthermore, having a
string-based storage gives much more flexibility to developer.

I believe this framework should implement every aspect regard
transport layer and parsing, and allows other frameworks to implement
logic instead of working with raw data. Additionally, integrating
typed header values as properties keeps room for further
optimizations.

Making data and cookie parsers public is a separate issue. We can
discuss about them. We can make current implementation public anytime.

On Mon, Nov 20, 2017 at 2:51 AM, Helge Heß via swift-server-dev > <swift-server-dev@swift.org> wrote:

On Nov 19, 2017, at 11:35 PM, Amir Abbas Mousavian via swift-server-dev <swift-server-dev@swift.org> wrote:

We are not changing that underlying `[(String:String)]` architecture

I suppose this is my biggest complaint w/ the PR and why I think it doesn’t belong here.
Either we do typed headers, then the API should vend them as such.
Or we don’t, then such can be part of a different framework.

These are common headers and every "real framework" have to implement
this independently!

They could still pull in your `make-headers-nice-my-way` package, if they think it is nice. For the frameworks I’m working on the provided functionality is too high level and not required (I certainly wouldn’t want to parse an Accept into an Array<Enum<String|*>> just to check a match).
So at least for me it is just baggage I don’t need, and I think we’d want to avoid bloat in this specific library.

However, as I said I'm personally in favor of typed headers and would prefer them over Strings a lot. But I don’t have any new arguments which would change the past discussion :slight_smile:

Should we have a shared cookie parser? Or an HTTP date parser? I think that would make some sense. But that should be standalone class/type/func which can be applied on a buffer passed in in the context the higher level framework sees fit.
It should not be attached to the response API (how cookies are presented is very framework specific).

and these accessors are lazy-loaded thus won't impose performance
overhead on loading. Strict typed storage will hit performance
significantly and will restrict users to have custom header values.

You are confusing typed headers w/ early parsing. Those are distinct concerns.

The type representing the header value could still decide whether it is better to parse the value very early (e.g. Content-Length), or whether it would do it later (Cookie maybe).

hh

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