Subject: Re: TICKET 259: 'treat as invalid' not defined

From: Julian Reschke <[email-masked]>
Date: Fri, 10 Dec 2010 10:59:20 +0100

On 12.11.2010 08:53, Julian Reschke wrote:
> On 12.11.2010 05:58, Mark Nottingham wrote:
>> I'm confused. I thought that we were going to talk about error
>> handling in an appendix, but it appears you're starting to talk about
>> it here.
>
> 1) Yes, it should be an appendix.
>
> 2) Well, it's parsing advice. It appears that some readers have trouble
> understanding how to derive a parsing strategy from the way how we
> currently write specs, so this is an attempt to describe just that.
 > ...

Here's an updated proposal (see also 
<http://trac.tools.ietf.org/wg/httpbis/trac/attachment/ticket/259/i259.diff>):

-- snip --Appendix D.  Parsing

    This document does not require any specific handling of invalid
    header field values.  With this in mind, the text below describes a
    simple strategy for parsing the header field and detecting problems
    in general, or in specific parameters.

D.1.  Combine Multiple Instances of Content-Disposition

    If the HTTP message contains multiple instances of the Content-
    Disposition header field, combine all field values into a single one
    as specified in Section 4.2 of [RFC2616].

D.2.  Parsing for Disposition Type and Parameters

    Using the simplified grammar below:

      field-value = disp-type *( ";" param )
      disp-type   = token
      param       = token "=" value

    ...parse the field value into a disp-type (disposition type) and a
    sequence of parameters (pairs of name (token) and value).  Lower-case
    all disposition types and parameter names.

    If the field value does not conform to the grammar (such as when not
    exactly one disposition type is specified), ignore the whole header
    field.

D.3.  Checking Cardinality Constraints

    If the parameter sequence contains multiple instances of the same
    parameter name, ignore the whole header field.

D.4.  Post-Process Parameter Values

    For each parameter, post-process the associated value part according
    to the grammar:

    o  According to Section 3.2.1 of [RFC5987] for parameters using the
       RFC 5987 syntax (such as "filename*").  If this fails, just ignore
       this parameter.

    o  According to the grammar for quoted-string (Section 2.2 of
       [RFC2616]) for values starting with a double quote character (").

    o  Verbatim otherwise.

    Note that this step starts with an octet sequence obtained from the
    HTTP message, and results in a sequence of Unicode characters.

D.5.  Extracting the Disposition Type

    The parsing step (Appendix D.2) has returned the disposition type (to
    be matched case-insensitively), which can be "attachment", "inline",
    or an extension type.  If the type is unknown, treat it like
    "attachment" (see Section 3.2).

D.6.  Determining the File Name

    The parsing and post-processing steps resulted in a set of parameters
    (name/value pairs).  The suggested file name is the value of the
    "filename*" parameter (when present), otherwise the value of the
    "filename" parameter.

    If neither is given, the UA can determine a name based on the
    associated URI; for instance based on the last path segment.

    Otherwise, the UA ought to post-process the suggested filename
    according following Section 3.3. [[anchor10: We could say here that
    UAs may reject filenames for security reasons, such as those with a
    path separator character.]]
-- snip --

I'm still nervous going even so far; imagining how much additional text 
we'd need in Part 1..7 to do this for all headers.

Best regards, Julian

---

From: Adam Barth <[email-masked]>
Date: Sat, 11 Dec 2010 11:42:46 -0800

Thanks for making a counter-proposal.  A few notes below.

>   exactly one disposition type is specified), ignore the whole header

This doesn't cover cases like the following:

Content-Disposition: attachment; inline; filename=foo.exe

We want to treat those as an attachment.  Another grammer we could use
might be the following:

     field-value = item *( ";" item )
     item          = disp-type / param
     disp-type   = <OCTET, except ";" and "=">
     param       = param-name "=" param-value
     param-name = <OCTET, except "=">
     param-value = <OCTET, except ";">

We could then say that first disp-type and the first param are the
ones that matter.  (I'm not sure this grammar handles <"> correctly,
but I'm sure we can sort that out.)

>   parameter name, ignore the whole header field.

We'd prefer to use the first one rather than ignore the header field.

>      [RFC2616]) for values starting with a double quote character (").

Does this imply \-decoding?  We don't want to do \-decoding.

>   o  Verbatim otherwise.

We'd like to do %-decoding both for the quoted and unquoted cases.

>   HTTP message, and results in a sequence of Unicode characters.

Somewhere we want to say what character set we're using.

>   "attachment" (see Section 3.2).

What if there's no disposition type?

Content-Disposition: filename=foo.exe
Content-Disposition: foo=bar

If I remember correctly, we're supposed to treat the former as inline
and the later as attachment.

>   path separator character.]]

I'll update the wiki shortly to respond to your previous feedback and
with information from this message.

Thanks,
Adam

---

From: Julian Reschke <[email-masked]>
Date: Sat, 11 Dec 2010 23:46:59 +0100

> Thanks for making a counter-proposal.  A few notes below.

(where the initial version has been around for over a month)

> Content-Disposition: attachment; inline; filename=foo.exe

Yes, this proposal is strictly about parsing valid headers for now.

>       disp-type   =<OCTET, except ";" and "=">
>       param-name =<OCTET, except "=">
>       param-value =<OCTET, except ";">
>
> ones that matter.  (I'm not sure this grammar handles<">  correctly,

If you did that, you'd be inconsistent with IE8: 
<http://localhost:8080/tc2231/#attandinline>.

> We'd prefer to use the first one rather than ignore the header field.

<http://localhost:8080/tc2231/#attwith2filenames>

Most UAs do indeed pick the first one, but it would be useful to 
understand whether this is purely academic or not. Can you provide any 
evidence about happening this in practice?

> Does this imply \-decoding?  We don't want to do \-decoding.

Yes, that's implied by quoted-string.

> We'd like to do %-decoding both for the quoted and unquoted cases.

I realize that (we have a separate issue for that, I believe).

> Somewhere we want to say what character set we're using.

Indeed. Will fix.

> and the later as attachment.

Dunno what you mean by "we're supposed to".

It SHOULD be handled like

Content-Disposition: filename=foo.exe, foo=bar

which is invalid. This needs a test case.

Best regards, Julian

---

From: Adam Barth <[email-masked]>
Date: Sat, 11 Dec 2010 15:03:59 -0800

Your web site seems to be down, so I'm working from memory:

http://greenbytes.de/tech/tc2231/

HTTP/1.1 200 OK
Date: Sat, 11 Dec 2010 22:54:51 GMT
Server: Apache/2.2.14 (Ubuntu)
Last-Modified: Thu, 09 Dec 2010 13:51:48 GMT
ETag: "11002c-0-496fa89fea500"
Accept-Ranges: bytes
Content-Length: 0
Content-Type: text/html

> Yes, this proposal is strictly about parsing valid headers for now.

Ok.  We're explicitly interested in handling invalid headers.

> <http://localhost:8080/tc2231/#attandinline>.

Indeed.  Agreement between all the browsers isn't required to make progress.

> Most UAs do indeed pick the first one, but it would be useful to understand
> whether this is purely academic or not. Can you provide any evidence about
> happening this in practice?

I don't have any data to present at this time.  However, we still want
to define how to handle these cases.  If it turns out not to affect
any web sites, that's fine.

> Yes, that's implied by quoted-string.

Ok, then that's not acceptable.  We don't want to do \-decoding.

> which is invalid. This needs a test case.

I agree that it's invalid.  However, we want to define how to handle
invalid headers.

Adam

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 13 Dec 2010 10:11:45 +1100

> Ok, then that's not acceptable.  We don't want to do \-decoding.

Adam,

Generally in the IETF we act as individuals making technical arguments, not as representatives of companies that are negotiating product roadmaps.

So, saying "we don't want to do \-decoding" isn't helpful. Can you please tell give us some technical reasoning for people to consider?

Also, if you do say "we" please be very clear about its scope -- it's not clear if you're speaking informally about you and some friends, or you're formally representing Google, or the Chrome browser team, or...  

Thanks,

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

---

From: Mark Nottingham <[email-masked]>
Date: Mon, 13 Dec 2010 10:35:26 +1100

> Indeed.  Agreement between all the browsers isn't required to make progress.

No, but given that according to Julian's tests, all browsers currently ignore headers with multiple disp-types, *except* for IE8, which *doesn't* pick the first one, it seems we have a strong motivation for defining error handling here to be compatible with IE8, so that we don't create yet more incompatibility.

Do you have any technical justification for another approach?

> any web sites, that's fine.

As long as it's not a requirement, I don't have any problem suggesting the first one.

---

From: Adam Barth <[email-masked]>
Date: Sun, 12 Dec 2010 15:36:33 -0800

> So, saying "we don't want to do \-decoding" isn't helpful. Can you please tell give us some technical reasoning for people to consider?

Sure.  I feel like I'm repeating myself, but I'll write it all here for clarity.

>From Julian's http://greenbytes.de/tech/tc2231/#attwithasciifnescapedchar test:

Content-Disposition: attachment; filename="f\oo.html"

FF3	fail (apparently does not treat the backslash as escape character,
replaces it with '_' (see Mozilla Bug 588389))
MSIE8	fail (apparently does not treat the backslash as escape
character, replaces it with '_')
Opera	pass
Safari	fail (apparently does not treat the backslash as escape
character, replaces it with '-')
Konq	pass
Chrome	fail (saves "oo.html" (skips the unescaping of "\", and then
mistakes it for a path separator, see Chrome Issue 52577))
Chrome9	fail (saves "oo.html" (skips the unescaping of "\", and then
mistakes it for a path separator, see Chrome Issue 52577))
Android	fail (apparently does not treat the backslash as escape
character, replaces it with '-')

The only browsers in his test that \-decode the filename parameter are
Opera and Konquerer.  The other browsers, representing some 99% of the
market, do not \-decode.

Based on this data lone, I'd be quite hesitant to implement \-decoding
in my hypothetical user agent.  Worse, the \ character is actually
quite commonly used in file paths because it is the path separator on
Windows.  It seems entirely likely that some number of servers send
absolute paths rather than file names,

Content-Disposition: attachment; filename="C:\foo\newsheet.html"

or even just relative paths, like "foo\bar.html".

Now, I haven't gone out and measured the prevalence of the \ character
in the headers on the web.  If some would like to run that experiment,
I'd certainly be open to considering that data.  However, in the
absence of such data, I think it's unlikely that browsers will change
their behavior.

Finally, I don't think we should require user agents to implement
behavior we have reason to believe will not be implemented by a number
of major implementations.  Every time we do that, we take one more
step down the path to irrelevance.  I'd like to see more engagement
between the browser vendors and the IETF, not less.

> Also, if you do say "we" please be very clear about its scope -- it's not clear if you're speaking informally about you and some friends, or you're formally representing Google, or the Chrome brows…

I shouldn't have used the term "we."  I was just frustrated at Julian.

Adam

---

From: Adam Barth <[email-masked]>
Date: Sun, 12 Dec 2010 15:42:12 -0800

> No, but given that according to Julian's tests, all browsers currently ignore headers with multiple disp-types, *except* for IE8, which *doesn't* pick the first one, it seems we have a strong motiv…

Oh, that's not how I interpret these tests:

Content-Disposition: attachment; inline; filename=foo.html
Content-Disposition: inline; attachment; filename=foo.html

http://greenbytes.de/tech/tc2231/#attandinline
http://greenbytes.de/tech/tc2231/#attandinline2

> Do you have any technical justification for another approach?

In both those cases, every browser (except IE8 and possibly Konquerer)
takes the first disposition-type.  I'm just suggesting that we
recommend the error handling behavior implemented by the majority of
the browsers.

> As long as it's not a requirement, I don't have any problem suggesting the first one.

Great.  :)

Adam

---

From: Julian Reschke <[email-masked]>
Date: Mon, 13 Dec 2010 09:36:48 +0100

> absence of such data, I think it's unlikely that browsers will change

We are going in circles, and have been for months now.

We discussed this just two 10 days ago. See 
<http://lists.w3.org/Archives/Public/ietf-http-wg/2010OctDec/0536.html>, 
where I said:

-- snip --
"Fixing" means "changing things to work as specified".

So the question here is whether it would break things because there are
servers sending unescaped backslashes. As far as I can tell, sending
path separators in the filename indicates a bug in the sender, or an
attempt to trick the user agent to do something it's not supposed to do.

So the "harm" of actually doing the unescaping would be that for a
filename that needs to be postprocessed anyway, the problematic
character would be filtered in a different way.

Starting with

    filename="a\bc"

the broken implementation sees "a" and "bc" separated by a path
separator, and will prost-process this to "abc", "a_bc" or "bc" (where _ 
  could be a different replacement character).

A correct implementation sees "abc".

I don't think there's a problem here.
-- snip --

You did reply to that in 
<http://lists.w3.org/Archives/Public/ietf-http-wg/2010OctDec/0538.html> with

"None of the user agents do \-decoding.  I don't see any value in them 
starting."

...which I quite frankly didn't find helpful. It appears that we have 
differing opinions on whether it's useful to have consistent handling of 
specific syntactical constructs.

I believe having different parsers for quoted-string depending on the 
context they appear in is both a bad idea in general, and also not 
*needed* here.

> of major implementations.  Every time we do that, we take one more

On the other hand, we also shouldn't require user agents to break the 
spec that has been around for ~10 years when there's no compelling 
reason to do so.

> between the browser vendors and the IETF, not less.

Yes, so do I. That's why I'd like to hear from the other browser makers 
(not necessarily vendors), and also observe how far we'll get with 
fixing the C-D bugs in Firefox once they are past the FF4 release.

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Mon, 13 Dec 2010 10:47:44 +0100

>> Somewhere we want to say what character set we're using.

I moved that up to an earlier section (see also 
<http://trac.tools.ietf.org/wg/httpbis/trac/attachment/ticket/259/i259.2.diff>):

    ...parse the octet sequence in the field value into a disp-type
    (disposition type) and a sequence of parameters (pairs of name
    (token) and value).

    Treat the result values as characters encoded using the ISO-8859-1
    character encoding ([ISO-8859-1]).

    Lower-case all disposition types and parameter names (note that these
    characters will all fall into the US-ASCII range by definition in the
    ABNF).

(I think this better reflects how implementations will actually work; 
maybe the "treat-as-ISO" statement should even be moved to the top).

Best regards, Julian