http-rs / http-types

Common types for HTTP operations
https://docs.rs/http-types
Apache License 2.0
200 stars 83 forks source link

Add `other::Expect` header #266

Closed yoshuawuyts closed 3 years ago

yoshuawuyts commented 3 years ago

Add other::Expect header. Ref #99. Thanks!

Fishrock123 commented 3 years ago

Is 100-continue the only relevant value here?

yoshuawuyts commented 3 years ago

@Fishrock123 yep, and it seems unlikely any others will be added anytime soon. No known browser currently uses this mechanism; it seems curl is the primary user of this right now.

Fishrock123 commented 3 years ago

Should we even implement this? Does Curl require it or something? i.e. is it useful?

jbr commented 3 years ago

We use it in async-h1 server, and when async-h1 client behavior is slightly more mature it should also use this header. I don't really see much value in including it as a reusable type, though. It's just as easy to do .insert_header("Expect", "100-continue"). We're trying to wrap up headers like there's a finite set of them, but there will always be headers that need to be set with string values

yoshuawuyts commented 3 years ago

@jbr I mostly included it for completion. We also have typed headers for e.g. Content-Length which is arguably trivial too.

Fishrock123 commented 3 years ago

Parsing Content-Length is not exactly uncommon though.

yoshuawuyts commented 3 years ago

It seems the conversation now is about useful this header will turn out to be, rather than whether the implementation is correct. I don't see any concrete downsides in including this; the worst case seems to be that nobody will care. With the upside that this might actually be helpful to some.

Making a decision to merge this.