Disallow negative or hexadecimal content-length headers.

[brianquinlan]

Change

Generate a HttpException when HttpHeaders.add(...) encounters a negative or hexadecimal encoded content-length. Currently both are acceptable.

Also, change non-numeric input to raise a HttpException rather than a FormatException so that a consistent Exception is thrown.

Rationale

RFC 7230 says that the production for content-length is:

     Content-Length = 1*DIGIT

So we are currently accepting invalid input that might cause clients/servers to misbehave.

The possibility of HttpHeaders.add(...) throwing FormatException is undocumented so (partially) consolidate to HttpException (which is what is what is thrown for most parse failures).

Impact

This change does not break any Google tests.

Note that this does not change the semantics of the HttpHeaders.contentLength property where a negative value means remove the header completely.

Mitigation

Users should not use HttpHeaders.add(...) to set content-length headers that are not allowed by the RFC.

[itsjustkevin]

@vsmenon @a-siva this breaking change has been in for a while as it did not have the breaking-change-request label applied. For early triage, I intend to place this in the next milestone while we await a decision.

[brianquinlan]

@itsjustkevin - do we no longer need approval from Hixie and grouma?

[a-siva]

the breaking change lgtm, not sure who else needs to approve it.

[itsjustkevin]

@grouma and @Hixie thoughts on this breaking change request?

[Hixie]

LGTM

[vsmenon]

lgtm

[grouma]

LGTM

[brianquinlan]

Fixed in 981bcf6

[MaryaBelanger]

@brianquinlan just want to clarify for my own understanding (and documentation),

Note that this does not change the semantics of the HttpHeaders.contentLength property where a negative value means remove the header completely.

Did you bring this up because content-length headers and the HttpHeaders.contentLength property are completely different items, and you don't want them getting conflated?

Or, did you mean that the statement on the api docs page "The value is negative if there is no content length set." doesn't need to change, because it's equivalent to the statement you made "a negative value means remove the header completely."