Correctly handle repeated headers.

[Stebalien]

HeaderX: a
HeaderX: b

MUST be interpreted as

HeaderX: a, b

See: https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

Fixes #683

[GitCop]

Thanks for contributing! Unfortunately, I'm here to tell you there were the following style issues with your Pull Request:

• Commit: 2ba4227
  • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md

---

This message was auto-generated by https://gitcop.com

[Stebalien]

Why did you test this case?

[seanmonstar]

For the test_duplicates test, from RFC7230#3.3.2:

If a message is received that has multiple Content-Length header
fields with field-values consisting of the same decimal value, or a
single Content-Length header field with a field value containing a
list of identical decimal values (e.g., "Content-Length: 42, 42"),
indicating that duplicate Content-Length header fields have been
generated or combined by an upstream message processor, then the
recipient MUST either reject the message as invalid or replace the
duplicated field-values with a single valid Content-Length field
containing that decimal value prior to determining the message body
length or forwarding the message.

For the test_duplicates_vary test, step 4 of RFC7230#3.3.3:

If a message is received without Transfer-Encoding and with
either multiple Content-Length header fields having differing
field-values or a single Content-Length header field having an
invalid value, then the message framing is invalid and the
recipient MUST treat it as an unrecoverable error.

The request should error out with varying Content-Lengths, which hyper doesn't do, but at the very least, it shouldn't pick any of the stated values as valid.

[Stebalien]

Ok, this change never picks any of the stated values even if they are all identical.

[seanmonstar]

It looks like the parsing of ContentLength has been left alone, so this test should still pass. I'd rather keep the functionality of ContentLength the same for now.

[Stebalien]

Good point, my diagnosis was wrong (guessing is bad). This change doesn't change the behavior of ContentLength as you say. However, it changes the behavior of the test_header macro to correctly verify that parsing and then formatting a header returns the original header. However, in this case, hyper will "clean" the header removing the second content length so this test will fail.

[coveralls]

Coverage increased (+0.03%) to 90.608% when pulling 9d39576 on Stebalien:repeated-header into 028f586 on hyperium:master.

[seanmonstar]

This looks fantastic. If you could add the ContentLength test back in, this will be good to go!

[coveralls]

Coverage increased (+0.04%) to 90.62% when pulling 70c6914 on Stebalien:repeated-header into 028f586 on hyperium:master.

[seanmonstar]

Thanks!