Subject: [Technical Errata Reported] RFC7230 (4667)

From: RFC Errata System <[email-masked]>
Date: Wed, 13 Apr 2016 09:05:04 -0700 (PDT)

The following errata report has been submitted for RFC7230,
"Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing".

--------------------------------------
You may review the report below and at:
http://www.rfc-editor.org/errata_search.php?rfc=7230&eid=4667

--------------------------------------
Type: Technical
Reported by: Alex Rousskov <[email-masked]>

Section: 4.1.1

Original Text
-------------
chunk-ext      = *( ";" chunk-ext-name [ "=" chunk-ext-val ] )

Corrected Text
--------------
chunk-ext      = *( ";" OWS chunk-ext-name [ "=" chunk-ext-val ] )

Notes
-----
The infamous "implicit *LWS" syntax rule in RFC 2616 allowed whitespace between ";" and chunk-ext-name in chunk-ext. Some HTTP agents generate that whitespace. In my experience, HTTP agents that can parse chunk extensions usually can handle that whitespace. Moreover, ICAP, which generally relies on HTTP/1 for its message syntax, uses that whitespace when defining the "ieof" chunk extension in RFC 3507 Section 4.5:

      \r\n
      0; ieof\r\n\r\n

IMHO, RFC 7230 should either allow OWS before chunk-ext-name or at the very least explicitly document the HTTP/1 syntax change and its effect on parsers used for both ICAP and HTTP/1 messages (a very common case for ICAP-supporting HTTP intermediaries and ICAP services).

I also recommend adding BWS around "=", for consistency and RFC 2616 backward compatibility reasons. HTTPbis RFCs already do that for transfer-parameter and auth-param that have similar syntax.

Please also consider adding OWS _before_ ";" for consistency and RFC 2616 backward compatibility reasons. HTTPbis RFCs already do that for transfer-extension, accept-ext,  t-ranking, and other constructs with similar syntax.

If all of the above suggestions are applied, the final syntax becomes:

chunk-ext      = *( OWS  ";" OWS chunk-ext-name [ BWS  "=" BWS chunk-ext-val ] )

Instructions:
-------------
This erratum is currently posted as "Reported". If necessary, please
use "Reply All" to discuss whether it should be verified or
rejected. When a decision is reached, the verifying party (IESG)
can log in to change the status and edit the report, if necessary. 

--------------------------------------
RFC7230 (draft-ietf-httpbis-p1-messaging-26)
--------------------------------------
Title               : Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing
Publication Date    : June 2014
Author(s)           : R. Fielding, Ed., J. Reschke, Ed.
Category            : PROPOSED STANDARD
Source              : Hypertext Transfer Protocol Bis APP
Area                : Applications
Stream              : IETF
Verifying Party     : IESG

---

From: Willy Tarreau <[email-masked]>
Date: Wed, 13 Apr 2016 18:36:15 +0200

> "Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing".

I think Alex is prefectly right here. I remember we spent less time on
chunk extensions by lack of clearly identified users, so it's very likely
that some corner cases slipped through the cracks. 

FWIW, haproxy does indeed accept spaces above as Alex suggests them.

Regards,
Willy

---

From: Mark Nottingham <[email-masked]>
Date: Thu, 14 Apr 2016 11:34:57 +1000

Digging into the history of this a bit, I see that the implied WSP was made explicit as of draft -16:
  https://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-16#section-6.2.1
... and subsequently, Julian started to convert it to OWS.

However, Roy took it out with this commit:
  https://github.com/httpwg/http11bis/commit/9ff47b1d187bbf2a#diff-48ead163ebcead27fb688b09acb76a43L2282

There doesn't seem to have been any discussion of that change on the list or the issue that the commit was linked to, and no one seems to have noticed until now.

Roy, any additional context here?

> On 14 Apr 2016, at 2:36 AM, Willy Tarreau <[email-masked]> wrote:
> 

--
Mark Nottingham   https://www.mnot.net/

---

From: Willy Tarreau <[email-masked]>
Date: Thu, 14 Apr 2016 07:29:24 +0200

>From what I'm seeing the WSP was first added between draft 4 and
draft 5 with this commit :

  commit cadd886ae27d63ed0725aa56a1d7caebbb48b71a
  Author: Julian Reschke <[email-masked]>
  Date:   Wed Nov 12 23:07:08 2008 +0000

    start adding BWS/OWS/RWS (related to #36)

I remember there was this problem about the poor WSP definition that Roy
had to work on resulting in the patch you pointed above. Maybe the chunk
part was fixed using the definitions from draft 4 as a starting point
instead of draft 5, I don't know.

Cheers,
Willy

---

From: "Roy T. Fielding" <[email-masked]>
Date: Thu, 14 Apr 2016 12:20:12 -0600

> On Apr 13, 2016, at 7:34 PM, Mark Nottingham <[email-masked]> wrote:
> 

We discussed it as an editorial issue #36, mostly in Newport Beach and later in Prague, IIRC.
The question was whether the implied whitespace rule from HTTP/1.0 had ever been
intended to apply to chunked encoding (a feature added for HTTP/1.1 in the body of
a message).  It had not.  The next was if there were any examples we knew of where space
was included there.  None.  [ICAP, by the way, is not HTTP.]  That was all discussed
as part of addressing #36.

Later we deprecated extensions in #343, and then we brought them back but with
only the syntax that we know works (no implied whitespace).  Hence, we didn't have the
history or implementations to justify unnecessary whitespace.

Apache httpd's chunk parser doesn't make use of chunk-ext unless it is bypassed
(i.e., a module can implement its own chunk parser).  The core will allow up to
10 bad whitespace between the chunk-size and chunk-ext (to allow for space-padding
of the chunk-size in fixed buffers), and then skip anything other than invalid
control chars between the ";" and the end of line; i.e.,

   chunk-ext      = 0*10<BWS> ";" *( OWS / VCHAR / )

IOW, that parser would support such a fix, but only by accident.  I seriously doubt
we can assume that arbitrary HTTP implementations would expect whitespace in those
places.  It was a stretch to expect that all implementations can handle chunk-ext,
but at least for that we had examples of how they would be implemented.

I don't have a problem with adding whitespace back in there, but I am not at all
confidant that such a choice would be less likely to break things.  I don't want
to play errata ping pong.

....Roy

---

From: Willy Tarreau <[email-masked]>
Date: Thu, 14 Apr 2016 21:32:59 +0200

On Thu, Apr 14, 2016 at 12:20:12PM -0600, Roy T. Fielding wrote:
(...)

Thanks for the detailed history Roy!

(...)
> IOW, that parser would support such a fix, but only by accident.

In haproxy it's basically the same. It doesn't make any use of these
extensions but happily skips these spaces if they are there.

> I seriously doubt

At least now we have an indication that at least one user exists :-)

> to play errata ping pong.

Well after all that's pretty much what the BWS was for : "you may have to
accept it for improved backwards compatibility but you must not emit it".

Regards,
Willy

---

From: Alex Rousskov <[email-masked]>
Date: Thu, 14 Apr 2016 15:58:20 -0600

On 04/14/2016 12:20 PM, Roy T. Fielding wrote:

> The next was if there were any examples we knew of where space
> was included there.  None.

> Apache httpd [allows] for space-padding
> of the chunk-size in fixed buffers

Too bad nobody from the Apache team was present during that discussion :-).

As you said, Apache httpd essentially uses the old syntax (and violates
the new syntax in two places!) to accommodate space-padding (at least):

I know Squid and several ICAP agents that use HTTP parsers do similar
things.

>  [ICAP, by the way, is not HTTP.]

ICAP is not HTTP but it explicitly uses big parts of HTTP syntax.
AFAICT, such IETF protocol reuse is a good thing and should be
encouraged and protected by IETF. If an HTTP*bis* RFC invalidates HTTP
syntax very prominently used in another IETF RFC, something went wrong.
Errata can fix that.

> confidant that such a choice would be less likely to break things.  

Both choices break things. The HTTPbis choice to delete LWS in chunks
breaks things today with a likelihood of 100% (my errata was inspired by
a real-world bug report related to this change). When HTTPbis is fixed,
someday, somewhere an implementation will misinterpret that whitespace.

Since that whitespace does exist in real messages, breaks ICAP RFC, and
causes no known specific harm, we ought to undo this syntax change IMO.

> I don't want to play errata ping pong.

The WG can always deny responsibility and add BWS instead of restoring
OWS. BWS cannot cause errata ping pong AFAICT. BWS does screw ICAP, but
nobody likes that kid anyway ;-).

Thank you,

Alex.

---

From: "Roy T. Fielding" <[email-masked]>
Date: Thu, 14 Apr 2016 16:39:42 -0600

Don't confuse the various lenient ways in which implementations parse HTTP with the requirements on generating HTTP messages that are defined by the ABNF. The ABNF is intended to be more restrictive. 

The scope of what Apache allows was reduced recently for security reasons, and I expect it to be reduced further the next time I get a chance to test and commit. Do not depend on such lenient parsing to exist in the future.  In many cases, we have just been waiting for RFC7230 to age enough before reducing the code substantially.

Likewise, don't confuse the parsing of ICAP messages (which are entirely defined by ICAP and its normative references) with requirements of RFC7230. If you need an exception, all you have to do is add it to ICAP  when (or if) that spec is updated to refer to RFC7230. ICAP hasn't changed until it does.

And, no, it is NEVER a good idea for new IETF protocols to effectively alias other IETF protocols. That has been proven many times by the three protocols that forked HTTP instead of just defining extensions within HTTP. Reuse of code in unexpected ways is just one of many problems that result.

....Roy

> On Apr 14, 2016, at 3:58 PM, Alex Rousskov <[email-masked]> wrote:
>

---

From: Alex Rousskov <[email-masked]>
Date: Thu, 14 Apr 2016 18:50:43 -0600

On 04/14/2016 04:39 PM, Roy T. Fielding wrote:

> Don't confuse the various lenient ways in which implementations parse
> HTTP with the requirements on generating HTTP messages that are
> defined by the ABNF. The ABNF is intended to be more restrictive.

I fully agree, but we are not discussing ABNF creation IMO. We are
discussing a syntax change by an HTTPbis RFC. To change HTTP/1 syntax
that has been in use for many years, the "Founders Intent" alone is not
enough IMHO. There must be other compelling reasons. The only other
reason given so far was "lack of known examples", followed by your
discussion of "space padding" as a known usage example. I expect the bar
for HTTP/1 syntax change to be significantly higher.

> Likewise, don't confuse the parsing of ICAP messages (which are
> entirely defined by ICAP and its normative references) with
> requirements of RFC7230. If you need an exception, all you have to do
> is add it to ICAP  when (or if) that spec is updated to refer to
> RFC7230. ICAP hasn't changed until it does.

AFAICT, the core of our disagreement is that you are treating RFC 7230
as a new protocol, not an HTTPbis document that polishes and clarifies
HTTP/1 while avoiding unnecessary breaking changes. If HTTP/1 defined by
RFC 7230 is a new protocol, then my ICAP defense plea is indeed invalid
(all other reasons to add OWS or at least BWS remain).

> And, no, it is NEVER a good idea for new IETF protocols to
> effectively alias other IETF protocols.

AFAICT, ICAP does not alias HTTP. It uses RFC 2616 to define HTTP
messages. This is similar to RFC 7230 using URI definitions from RFC
3986. When URIbis obsoletes RFC 3986, I expect the authors to be very
careful not to accidentally invalidate HTTP/1 messages. IMHO, HTTPbis
should offer the same courtesy to ICAP.

Thank you,

Alex.
P.S. Please do not misinterpret the ICAP part of my argument as an
admiration for RFC 3507. I know that RFC has lots of problems. I am
thinking about ICAP developers that generally want to reuse HTTP/1
parsers. Such reuse now requires RFC 7230 violations and that feels
rather wrong [without compelling reasons for the syntax change].

---

From: Willy Tarreau <[email-masked]>
Date: Fri, 15 Apr 2016 06:49:24 +0200

> for HTTP/1 syntax change to be significantly higher.

Alex, it's not that black or white. We focused on maximized interoperability,
so you need to understand that when some people report that product X,Y or Z
doesn't even support chunk extensions, that other products are simply broken
regarding this and we realize that nobody produces them, it's natural to
deprecate them. They were apparently re-added in a stricter way based on
identified implementations to optimize the intersection between producers
and consumers.

It's very difficult to satisfy every implementation, especially with HTTP
that everyone tried to implement without looking at the doc in the late
1990 and early 2000 resulting in many incompatible implementations.

So here there's no intent to break anything, instead there's an intent to
make everything work together based on all the data everyone could collect.
It's not rocket science but it proved to work pretty well given all the
interoperability issues that could be addressed (eg: header folding,
multiple content-length, CRLF after body, etc).

I do think that adding the BWS back could be enough. And maybe even adding
the only one ICAP uses. After all it already took something like 5 years
for someone to notice this change, maybe ICAP is the only exception to the
rule and is sufficient to address without further breaking existing
implementations.

> should offer the same courtesy to ICAP.

Not exactly in fact, RFC3507 says this :

   ICAP is a request/response protocol similar in semantics and usage to
   HTTP/1.1 [4].  Despite the similarity, ICAP is not HTTP, nor is it an
   application protocol that runs over HTTP. (...) ICAP uses TCP/IP as a
   transport protocol.

So in short it allows implementers to save time by reusing their HTTP
parsers but does not expect to be strictly compatible. There are even
some intended differences, such as :

   Note in particular that the "Transfer-Encoding" option is not
   allowed. (...) Encapsulated bodies MUST be transferred using the
   "chunked" transfer-coding described in Section 3.6.1 of [4].
   However, encapsulated headers MUST NOT be chunked.

These ones alone prevent reliable forwarding over HTTP gateways. But I
do agree that if we don't break anything by adding the BWS back it
would be better, at least because we're now pretty sure that people
who need to adapt their HTTP parsers to also support ICAP will support
it anyway.

Regards,
Willy

---

From: Amos Jeffries <[email-masked]>
Date: Sat, 16 Apr 2016 00:11:37 +1200

> I know Squid and several ICAP agents that use HTTP parsers do similar

Well, for the record. Squid used to just absorb anything at all before
the line terminator LF. It was only when we changed 4.x beta to a parser
with strict syntax validation and thus not accepting SP in these
locations that this came to light.

At least one server ("IBM_HTTP_Server") is sending SP characters to pad
out the chunk-size field to 4 octets.

We have several others mentioning chunk decoding issues being logged,
but not yet investigated closely to say if its the same server
implementation or other interoperability issues.

Note that the Eratta proposed syntax, including the extra suggestions
will still not successfully match what Squid is sighting. We would also
need to allow BWS / OWS / SP trailing the chunk-size value when no
chunk-ext is present.

Amos

---

From: Alex Rousskov <[email-masked]>
Date: Fri, 15 Apr 2016 10:19:40 -0600

> Alex, it's not that black or white.

I wonder which part of my argument you consider to be "black or white".

> We focused on maximized interoperability,
> deprecate them.

Agreed. However, we are not discussing deprecation (that did not happen)
but the syntax change (that did). Those two issues are completely
different IMO.

> They were apparently re-added in a stricter way based on

"Stricter way" does not automatically "optimize the intersection between
producers and consumers". Please re-read the history summary by Roy: The
WG removed whitespace because it was deemed "unnecessary", not because
there were any known implementations that did not support whitespace.
There is a big difference between "removing something that does not
break any known implementation" and "removing something that breaks
known implementations".

I understand that the WG did not know about implementations using
whitespace. However, using prior lack of knowledge to _justify_ leaving
a mistake in a *bis RFC seems strange.

> the only one ICAP uses. 

The following two changes would be enough to cover _known_ use cases:

  1. BWS after ";" to accommodate the widely used ieof extension.
  2. BWS after chunk-size to accommodate space padding.

> After all it already took something like 5 years

For the record, I noticed the syntax change because of a incompatibility
bug report against the new Squid _HTTP_ parser. I discovered the fact
that it also breaks the extension used by ICAP while writing the errata.

I am saddened by your audacity to use the "5 years" argument to prove
correctness of a subtle syntax change in a *bis* RFC that was published
less than 2 years ago and that is not even applicable the latest
protocol version.

> parsers but does not expect to be strictly compatible.

IMO, the above RFC 3507 text matches what I said and does not imply any
allowance for incompatibility with HTTP chunked encoding syntax. ICAP is
not HTTP but ICAP message bodies use HTTP chunked encoding.

> There are even
> These ones alone prevent reliable forwarding over HTTP gateways. 

I am sorry, but I believe you misunderstand what ICAP is and, hence,
misinterpret its specs. ICAP does not use HTTP as transport and does not
work with HTTP agents. ICAP uses HTTP chunked _encoding_ for sending
HTTP message bodies over TCP connections between ICAP agents. The syntax
change in HTTPbis breaks ICAP use of HTTP chunked encoding.

> But I

And we also know that HTTP agents use that whitespace for padding.

Alex.

---

From: Alex Rousskov <[email-masked]>
Date: Fri, 15 Apr 2016 10:51:03 -0600

> need to allow BWS / OWS / SP trailing the chunk-size value when no

As Roy has eloquently said, "Don't confuse the various lenient ways in
which implementations parse HTTP with the requirements on generating
HTTP messages that are defined by the ABNF". This errata is about fixing
[other parts of] the ABNF for the reasons specified in the errata. Squid
will be fixed for other reasons, regardless of the errata acceptance or
the final ABNF shape.

Needless to say, somebody may submit another errata to accommodate
chunk-size space padding without chunk extensions. However, they would
face even greater obstacles justifying such change because it is
debatable whether the "implied *LWS" rule applies to that case -- CR is
not a "token" or "quoted string" that the poorly worded LWS rule kind of
requires... The best they can hope for, after hearing the "be lenient"
argument, is adding an informal RFC sentence alerting implementations of
known space padding issues.

Cheers,

Alex.

---

From: Willy Tarreau <[email-masked]>
Date: Fri, 15 Apr 2016 21:53:37 +0200

> I wonder which part of my argument you consider to be "black or white".

The way I read your comment makes me think you believe that there was
an intent to purposely break existing implementations, which has been
the opposite during all this work. It's not because some breakage results
from the change that this breakage was intended.

> but the syntax change (that did). Those two issues are completely

A lot of things have been enforced with the new spec, based on the feedback
gathered. For example the HTTP-version has now become one digit dot one digit.

> there were any known implementations that did not support whitespace.

"deemed unnecessary" happens after not finding any counter example. That
doesn't imply they don't exist. Conversely there were examples of
implementations that did not support extensions at all.

> break any known implementation" and "removing something that breaks

Yes exactly and as long as no known implementation it found, you're
neither in any case. That's what I meant by "black or white". Here
it's simply removing something which is expected not to break any
implementation and which will make others more interoperable.

> a mistake in a *bis RFC seems strange.

But how do you retro-document a protocol that has lived for 15 years
with everyone doing his own fork because he knows better, or not
implementing what is considered too difficult and useless, or even
makes different choices from others on grey areas resulting in possible
smuggling attacks for example ? At some point you have to decide this
causes trouble to a number of implementations and is apparently not used
by anybody, let's make it clearer.

>   2. BWS after chunk-size to accommodate space padding.

OK.

> less than 2 years ago and that is not even applicable the latest

It's not audacity, it's just a fact. This change was made 5 years ago and
this problem is reported now. Like any bug, until nobody discovers it it
can live long. It simply confirms that if it took 5 years for the first
report of valid use case to come, there were *really* few implementations
relying on this and it is understandable they were not spotted. That's all
I'm saying.

> not HTTP but ICAP message bodies use HTTP chunked encoding.

Not only the body, but the request line, response lines, headers an
delimitation between headers and body is made to work like HTTP.

> misinterpret its specs.

No, I've used it 15 years ago. I've even seen some people use it over some
lenient HTTP gateways.

> ICAP does not use HTTP as transport and does not work with HTTP agents.
> ICAP uses HTTP chunked _encoding_ for sending HTTP message bodies over
> TCP connections between ICAP agents. The syntax change in HTTPbis breaks
> ICAP use of HTTP chunked encoding.

Absolutely. It's too bad it was not identified by then, but we can circle
in loops forever asking why is was not identified instead of proposing a
solution. Your proposal might be fine, and maybe in 2 years we'll get
another report for it not being enough and another errata will simply be
emitted. That's the purpose of erratas.

> And we also know that HTTP agents use that whitespace for padding.

OK. I wasn't aware of this one.

Willy

---

From: Alex Rousskov <[email-masked]>
Date: Fri, 15 Apr 2016 17:50:06 -0600

> an intent to purposely break existing implementations,

Just to avoid misunderstanding: I may be wrong, but I am not that dumb.
The "Founders Intent" phrase refers to Question #1 in Roy's history: Did
the authors intend to apply the "implied *LWS" rule to chunking? The
answer was "No, there was no such intent". AFAICT, that lack of intent
was used as the first reason to remove LWS, and I acknowledged that in
the above paragraph while claiming that reason #1 alone is not enough to
justify keeping the syntax as it is now.

I am now deleting most of my long response because I eventually realized
that you were just defending the way HTTPbis decisions were made. I was
_not_ attacking that (and understand the complexities of that process).

> solution. 

We might, but I am not asking that question and am proposing a solution.

> Your proposal might be fine, and maybe in 2 years we'll get

Agreed.

Alex.

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 19 Apr 2016 16:18:04 +1000

I *think* we've come to a place where there's agreement on accepting the errata, but with BWS replacing OWS throughout; i.e.:

chunk-ext      = *( BWS  ";" BWS chunk-ext-name [ BWS  "=" BWS chunk-ext-val ] )

Everyone OK with that?

If so -- Alexey, can we just annotate the errata with that when it's accepted, or should this one be rejected and a new (smaller and correct from the start) one be filed?

Regards,

> On 14 Apr 2016, at 2:05 AM, RFC Errata System <[email-masked]> wrote:
>

---

From: Alex Rousskov <[email-masked]>
Date: Tue, 19 Apr 2016 08:22:33 -0600

> chunk-ext      = *( BWS  ";" BWS chunk-ext-name [ BWS  "=" BWS chunk-ext-val ] )

FWIW, I am OK with that "better BWS than nothing" solution.

Thank you,

Alex.

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 19 Apr 2016 16:41:01 +0200

> FWIW, I am OK with that "better BWS than nothing" solution.

And I'm OK as well.

Cheers,
Willy