Subject: #173: CR and LF in chunk extension values

From: Mark Nottingham <[email-masked]>
Date: Tue, 11 Aug 2009 05:31:38 +1000

This was discussed in Stockholm, and there was agreement in the room  
that the proper way to address this is to disallow CR and LF in *any*  
quoted-string.

Comments?

On 25/06/2009, at 3:53 PM, Mark Nottingham wrote:

> Now #173:
>  http://trac.tools.ietf.org/wg/httpbis/trac/ticket/173
>
> We probably need to have a more general discussion of chunk- 
> extensions as well...
>
>
> On 18/06/2009, at 4:07 AM, Bjoern Hoehrmann wrote:
>
>> Hi,
>>
>> A chunk extension value is defined as either token or quoted- 
>> string. A
>> quoted-string allows CRs and LFs for folding and in escaped form  
>> under
>> RFC 2616; we have since outlawed the escaped form, and in headers,  
>> but
>> not chunk extension values, we now outlaw producing them for  
>> folding as-
>> well. Accepting and processing the latter correctly still appears  
>> to be
>> a SHOULD level requirement; I am not sure about the former.
>>
>> It appears that implementations usually just read a line and ignore  
>> any-
>> thing after the first ";" character at the beginning of a chunk.  
>> Perhaps
>> the specification should use a CRLF-free quoted-string instead for  
>> this;
>> if not, the considerations for obs-fold should apply to chunk  
>> extension
>> values aswell, or obs-fold should not be used for chunk extension  
>> values
>> (which would require a separate quoted-string production aswell).
>>
>> regards,
>> -- 
>> Björn Höhrmann · mailto:[email-masked] · http://bjoern.hoehrmann.de
>> Am Badedeich 7 · Telefon: +49(0)160/4415681 · http://www.bjoernsworld.de
>> 25899 Dagebüll · PGP Pub. KeyID: 0xA4357E78 · http://www.websitedev.de/
>>
>
>
> --
> Mark Nottingham     http://www.mnot.net/
>
>

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

---

From: Henrik Nordstrom <[email-masked]>
Date: Tue, 11 Aug 2009 00:50:54 +0200

> that the proper way to address this is to disallow CR and LF in *any*  

Escaped newlines or \0 characters in the form of quoted-pair very likely
to cause many parsers to fail no matter where these are seen. I know I
have always understood this as a mechanism intended for quoting special
characters like " ( and ),  and not including CTLs.

Regarding chunked encoding allowing any newlines there is a very very
bad idea. Folding is not supported there, and no one expects to see
newlines in the middle of a chunk header quoted or not.

I would propose changing quoted-pair to restrict the allowable set to
non-CTLs to match most expectations on what values may be seen, not only
excluding CR or LF.

    quoted-pair  = "\" <any CHAR except CTLs>

instead of

    quoted-pair  = "\" CHAR

Regards
Henrik

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 12 Aug 2009 16:43:33 +1000

Right now, it's defined as:

> A string of text is parsed as a single word if it is quoted using
> double-quote marks.
>
>   quoted-string  = DQUOTE *( qdtext / quoted-pair ) DQUOTE
>   qdtext         = OWS / %x21 / %x23-5B / %x5D-7E / obs-text
>                  ; OWS / <VCHAR except DQUOTE and "\"> / obs-text
>   obs-text       = %x80-FF
>
> The backslash character ("\") MAY be used as a single-character
> quoting mechanism only within quoted-string and comment constructs.
>
>   quoted-text    = %x01-09 /
>                    %x0B-0C /
>                    %x0E-FF ; Characters excluding NUL, CR and LF
>   quoted-pair    = "\" quoted-text

So it seems like we need to:

1) Consider removing OWS from qdtext, replacing it with space and tab  
only. While we could use BWS here, receivers are required to accept  
it, which I don't think is the desired effect. And,

2) Consider removing obs-text from qdtext, as it's a hole that a truck  
can drive through. Otherwise, modify it to explicitly disallow CTLs.  
And,

3) Restrict the allowable set of characters in quoted-text to disallow  
CTLs. VCHAR?

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 24 Aug 2009 14:45:40 +1000

That leaves us at:

1) Replace OWS in qdtext with space and tab, and
2) Remove obs-text from qdtext, and
3) Restrict quoted-text to VCHAR.

Milestone assigned for -08; barring any other discussion, we'll see  
what the editors come up with in that revision.

On 12/08/2009, at 4:43 PM, Mark Nottingham wrote:

> 1) Consider removing OWS from qdtext, replacing it with space and  
> tab only. While we could use BWS here, receivers are required to  
> accept it, which I don't think is the desired effect. And,
>
> 2) Consider removing obs-text from qdtext, as it's a hole that a  
> truck can drive through. Otherwise, modify it to explicitly disallow  
> CTLs. And,
>
> 3) Restrict the allowable set of characters in quoted-text to  
> disallow CTLs. VCHAR?
>
>
>

---

From: Julian Reschke <[email-masked]>
Date: Mon, 24 Aug 2009 15:18:40 +0200

Mark Nottingham wrote:
> Milestone assigned for -08; barring any other discussion, we'll see what 
> the editors come up with in that revision.
> ...

1)

   qdtext         = WSP / %x21 / %x23-5B / %x5D-7E / obs-text
                  ; WSP / <VCHAR except DQUOTE and "\"> / obs-text
   obs-text       = %x80-FF

2)

What's the problem with obs-text? It doesn't contain controls...

3)

It seems to me that the purpose of quoted-text is to allow any character 
in qdtext, plus DQUOTE and "\", which would make it

quoted-text = qdtext / DQUOTE / "\"

While we're at it, we probably should rename it to quoted-char, and also 
add a short statement what the semantics of a quoted-pair is.

BR, Julian

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 25 Aug 2009 16:04:34 +1000

>                 ; WSP / <VCHAR except DQUOTE and "\"> / obs-text

Looks good.

> What's the problem with obs-text? It doesn't contain controls...

Mea culpa; misread that. Never mind #2.

> 3)
>
> It seems to me that the purpose of quoted-text is to allow any  
> character in qdtext, plus DQUOTE and "\", which would make it
>
> While we're at it, we probably should rename it to quoted-char, and  
> also add a short statement what the semantics of a quoted-pair is.

I had to read that a few times, but I think I agree. However, "quoted- 
char" may be confusing, as it's very similar to "quoted-pair".

---

From: Julian Reschke <[email-masked]>
Date: Tue, 25 Aug 2009 09:11:13 +0200

Mark Nottingham wrote:
> I had to read that a few times, but I think I agree. However, 
> "quoted-char" may be confusing, as it's very similar to "quoted-pair".

And yes,

	qdtext / DQUOTE / "\"

is the same as

	WSP / VCHAR / obs-text

...but I think the former is more clear in that it adds DQUOTE and "\".

But.

quoted-pair is also used in comments. Are we ok with restricting the set 
here as well? And, if yes, shouldn't we then also adjust the allowed set 
for non-quoted characters in comments?

Currently it reads 
(<http://greenbytes.de/tech/webdav/draft-ietf-httpbis-p1-messaging-latest.html#rfc.section.3.2>):

   comment        = "(" *( ctext / quoted-pair / comment ) ")"
   ctext          = OWS / %x21-27 / %x2A-5B / %x5D-7E / obs-text
                  ; OWS / <VCHAR except "(", ")", and "\"> / obs-text

To make it consistent with quoted-string it would need to change to:

   ctext          = BWS / %x21-27 / %x2A-5B / %x5D-7E / obs-text
                  ; BWS / <VCHAR except "(", ")", and "\"> / obs-text

Feedback appreciated,

Julian

---

From: Henrik Nordstrom <[email-masked]>
Date: Tue, 25 Aug 2009 14:21:05 +0200

Should probably change topic here, but it's still relevant so keeping
the issue topic. Most of this is taking a more generic view of
quoted-pair, not isolated to chunk extension values.

> for non-quoted characters in comments?

What? Restricting how? I thought we were talking about restricting the
use of CTLs?

Now some further rambling on the use of quoted-pair and the difficulties
this causes for parsers:

qdtext is for text within a quoted-string, and MUST NOT include '"' or
'\'. Those two must be produced as quoted-pair to be used within a
quoted-string.

ctext is the same but for comment, and MUST NOT include '(', ')' or '\'.
Those three must be produced as quoted-pair to be used within a comment.

Neither of qdtext or ctext allows for CTLs, except for HT or obsoleted
CRLF folding (from OWS).

Specifications (2616) is very strict on where quoted-pair is alowed to
be used, but it's at the same time very subtle where those areas are
creating a large grey area where parsing is somewhat non-obvious.

It's the same question as been raised earlier regarding comments. A
construct looking like a comment is only a comment if the header in
question is defined to allow comments, if not it's literally part of the
header value.

Quoted-string is also only quoted-string if the header in question is
defined to accept quoted-string, if not it may be a literal part of the
header value even if it may look like a quoted-string (for a header
defined as taking *TEXT as value, 2616 has no such headers however)

RFC2616 BNF and relevant comments:

      generic-message = start-line
                        *(message-header CRLF)
                        CRLF
                        [ message-body ]
       message-header = field-name ":" [ field-value ]
       field-name     = token
       field-value    = *( field-content | LWS )
       field-content  = <the OCTETs making up the field-value
                        and consisting of either *TEXT or combinations
                        of token, separators, and quoted-string>

       TEXT           = <any OCTET except CTLs,
                        but including LWS>

   A CRLF is allowed in the definition of TEXT only as part of a header
   field continuation.

   Comments can be included in some HTTP header fields by surrounding
   the comment text with parentheses. Comments are only allowed in
   fields containing "comment" as part of their field value definition.
   In all other fields, parentheses are considered part of the field
   value.

       comment        = "(" *( ctext | quoted-pair | comment ) ")"
       ctext          = <any TEXT excluding "(" and ")">

The allowable characters in *TEXT overlaps completely with token,
separators and quoted-string in the allowable characters except that
*TEXT do not allow CTLs other than LWS (HT), and within *TEXT the '\'
character have no special meaning.

Which means that to properly parse '\' quoted constructs one must know
in detail every header processed in order to know if the '\' is quoting
the next character or if it's just a literal '\'.

Because of this it's important that the overall message parsing is the
same regardless if quoted-pair is processed or not, only producing
slightly different results in the raw header value. Or put in other
words, it needs to be possible to completely defer quoting and comment
processing until the header value as such is examined in detail, with
general message parsing using *TEXT for all header values. And for chunk
headers *TEXT minus folding for the general message format, only needing
to dive into quoting etc when eventually processing the chunk extension
values (if at all).

Regarding the allowable characters there imho is absolutely no need to
allow for control characters anywhere in HTTP headers or chunk headers,
quoted or not, and it's additionally very very likely many parsers will
fail on such constructs making them quite non-interoperable.

And additionally if restricting the allowed set of quoted characters to
exclude \x00, NL and CR as already done in HTTPbis then it becomes very
questionable from a technical point of view (ignoring parsing) to allow
the use of other CTLs in quoted form. The use of having CTLs in header
values is very limited to begin with, basically only needed to support
transmission of (non-UTF8) multibyte charactersets or binary non-text
data, in which case having those three excluded is already a signifcant
issue for such use.

So imho quoted-pair should be

    quoted-text = %x09 / %x20-%x7E / obs-text
                ; WSP / VCHAR / obs-text
    quoted-pair = "\" qchar	

to match the use of *TEXT in 2616, making comments and quoted strings
all fit within *TEXT as those constructs is only used in detailed forms
which should be a subset of the more generic *TEXT.

This reasoning is also consistent with the current field-content
definition using VTEXT etc..

    field-value    = *( field-content / OWS )
    field-content  = *( WSP / VCHAR / obs-text )

This field-content definition DOES NOT allow for CTLs other than HT.
Allowing quoted-pair to include CTLs other than HT is incompatible with
the above (from latest p1) definition of field-content.

If you look closely you'll notice the quoted-text and field-contents
definitions above are equal. Perhaps a common term should be defined for
that similar to the *TEXT element used in 2616. There is probably more
places where using said term would make sense. And sorry, no I do not
have a good suggested BNF name for this construct.. TEXT would be
confusing with 2616 and text in lower case too generic to be used in
describing text. general-text?

Regards
Henrik

---

From: Julian Reschke <[email-masked]>
Date: Tue, 25 Aug 2009 14:47:12 +0200

> What? Restricting how? I thought we were talking about restricting the

Yes. I wanted to confirm that we do that for quoted-strings *and* 
comments. Do we?

> Neither of qdtext or ctext allows for CTLs, except for HT or obsoleted

Yes. But quoted-string and comment allow quoted-pair which currently 
does allow CTLs.

> the next character or if it's just a literal '\'.

Yes.

> fail on such constructs making them quite non-interoperable.

Agreed.

> data, in which case having those three excluded is already a signifcant

Yes.

> which should be a subset of the more generic *TEXT.

"qchar" being...?

> describing text. general-text?

"characters"?

Anyway, my take away from your analysis is: "yes, CTLs need to be 
disallowed both in comments and quoted-text", right?

BR, julian

---

From: Julian Reschke <[email-masked]>
Date: Tue, 25 Aug 2009 15:29:40 +0200

OK, so my understanding is that we disallow all control characters 
except HTAB in comment and quoted-string, escaped or not.

Proposed patch: 
<http://trac.tools.ietf.org/wg/httpbis/trac/attachment/ticket/173/173.diff>.

Relevant changes in Part 1:

-- snip --

    The backslash character ("\") can be used as a single-character
    quoting mechanism only within quoted-string and comment constructs:

      quoted-pair    = "\" ( WSP / VCHAR / obs-text )

    Note that quoted-pair includes those characters otherwise disallowed
    in quoted-string or comment (Section 3.2).

...

      comment        = "(" *( ctext / quoted-pair / comment ) ")"
      ctext          = WSP / %x21-27 / %x2A-5B / %x5D-7E / obs-text
                     ; WSP / <VCHAR except "(", ")", and "\"> / obs-text

...

    Rules about implicit linear whitespace between certain grammar
    productions have been removed; now it's only allowed when
    specifically pointed out in the ABNF.  Control characters other than
    HTAB are no longer allowed in comment and quoted-string text (escaped
    or not).  Non-ASCII content in header fields and reason phrase has
    been obsoleted and made opaque (the TEXT rule was removed)
    (Section 1.2.2)

-- snip --

Feedback appreciated,

Julian

---

From: Henrik Nordstrom <[email-masked]>
Date: Tue, 25 Aug 2009 16:29:05 +0200

> > which should be a subset of the more generic *TEXT.

A typo

> > describing text. general-text?

Is WSP and obs-text characters? Other than that no opinion either way..

> disallowed both in comments and quoted-text", right?

Yes. CTLs should be disallowed in quoted-pair except for those included
in WSP (HT).

Regards
Henrik

---

From: Henrik Nordstrom <[email-masked]>
Date: Tue, 25 Aug 2009 16:58:25 +0200

Yes.

>     or not).

Note: CRLF in the form of obs-fold is still allowed in both, just as it
has always been. It's just quoting using '\' which has been restricted.

> Feedback appreciated,

Looks good to me.

Regards
Henrik

---

From: Julian Reschke <[email-masked]>
Date: Thu, 27 Aug 2009 11:53:03 +0200

> has always been. It's just quoting using '\' which has been restricted.

OK, I have applied the change with 
<http://trac.tools.ietf.org/wg/httpbis/trac/changeset/686>.

BR, Julian

---

From: Henrik Nordstrom <[email-masked]>
Date: Thu, 27 Aug 2009 12:11:41 +0200

> <http://trac.tools.ietf.org/wg/httpbis/trac/changeset/686>.

Looking again.. and no it's not entirely fine.

ctext and qdtext should not be changed from OWS to WSP. The change is
only in quoted-text. We can not disallow folding here.

Sorry for not seeing this earlier.

Regards
Henrik

---

From: Julian Reschke <[email-masked]>
Date: Thu, 27 Aug 2009 14:03:29 +0200

> Sorry for not seeing this earlier.

It happens; thanks for checking anyway (and this was exactly the reason 
I wanted people to verify this change :-).

For now, I undid the change with 
<http://trac.tools.ietf.org/wg/httpbis/trac/changeset/687>.

It appears that we *do* have consensus for disallowing controls in 
quoted-pairs, thus for:

However, if that's all that we do we won't have addresses issue #173 
after all.

Proposal:

- add a new issue for disallowing CTLs in quoted-pair

- address #173 by tuning the definition of chunk-ext-val

BR, Julian

---

From: Henrik Nordstrom <[email-masked]>
Date: Fri, 28 Aug 2009 01:28:00 +0200

Yes.

> However, if that's all that we do we won't have addresses issue #173 

Indeed.

Yes.

> - address #173 by tuning the definition of chunk-ext-val

Which means defining a new variant of quoted-string which do not allow
for folding for use in chunk-ext-val.

    chunk-ext-val    = token / quoted-string-nf
    quoted-string-nf = DQUOTE *( qdtext-nf / quoted-pair ) DQUOTE
    qdtext-nf        = WSP / %x21 / %x23-5B / %x5D-7E / obs-text
                     ; WSP / <VCHAR except DQUOTE and "\"> / obs-text 

assuming quoted-pair is fixed as discussed.

Perhaps is should also be noted in text that folding is explicitly forbidden in chunk headers.

Comments are thankfully not allowed in chunk extensions from what I can tell.

Regards
Henrik

---

From: "Roy T. Fielding" <[email-masked]>
Date: Thu, 27 Aug 2009 18:45:23 -0700

On Aug 27, 2009, at 5:03 AM, Julian Reschke wrote:
> However, if that's all that we do we won't have addresses issue  
> #173 after all.
>
> Proposal:
>

I suggest we make the issue "Disallow quoted-pair productions that are
never used in practice nor needed for parsing", with the fix being

    quoted-pair    = "\" ( "\" / DQUOTE / "(" / ")" )

....Roy

---

From: Henrik Nordstrom <[email-masked]>
Date: Fri, 28 Aug 2009 04:07:43 +0200

>     quoted-pair    = "\" ( "\" / DQUOTE / "(" / ")" )

Fine with me.

Did not want to push it that far in my proposal for quoted-pair.

Regards
Henrik

---

From: Julian Reschke <[email-masked]>
Date: Fri, 28 Aug 2009 11:35:01 +0200

>    quoted-pair    = "\" ( "\" / DQUOTE / "(" / ")" )

- I have opened issue #194 for this.

- Restricting it to only these four characters is attractive, but may 
break content where people quoted too defensively.

BR, Julian