harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From enh <...@google.com>
Subject Re: [classlib] URLConnection.{get,set,add}RequestProperty behaviour
Date Fri, 19 Feb 2010 21:30:23 GMT
On Fri, Feb 19, 2010 at 12:04, Mark Hindess <mark.hindess@googlemail.com> wrote:
>
> Help please!  I'm struggling with:
>
>  HttpUrlConnection converts request headers to lowercase
>  HttpUrlConnection.addRequestProperty overrides existing properties
>  https://issues.apache.org/jira/browse/HARMONY-6452
>
> The JIRA is slightly confusing because it contains two issues.
> However, they are rather closely linked and this confusion is trivial
> in comparison to the confusion I now face trying to figure out
> what behaviour to implement to be compatible with the reference
> implementation.
>
> The reference implementation seems rather inconsistent so I am wonder
> if it is really worth trying to match its behaviour rather than merely
> meeting the spec with a simpler implementation.
>
> For example, my testing suggests that the reference implementation
> treats keys provided to the setRequestProperty method case-insensitively
> and additionally preserves the case of the first instance of a
> particular key string.  So if you do:
>
>  URLConnection conn =
>    (URLConnection)(new URL("http://example.org/").openConnection());
>  conn.setRequestProperty("KEY", "upper");
>  conn.setRequestProperty("key", "lower");
>  System.out.println("KEY=" + conn.getRequestProperty("KEY"));
>  System.out.println("key=" + conn.getRequestProperty("key"));
>  System.out.println("properties=" + conn.getRequestProperties());
>
> then the RI outputs:
>
>  KEY=lower
>  key=lower
>  properties={KEY=[lower]}
>
> and if you do:
>
>  URLConnection conn =
>    (URLConnection)(new URL("http://example.org/").openConnection());
>  conn.setRequestProperty("key", "lower");
>  conn.setRequestProperty("KEY", "upper");
>  System.out.println("KEY=" + conn.getRequestProperty("KEY"));
>  System.out.println("key=" + conn.getRequestProperty("key"));
>  System.out.println("properties=" + conn.getRequestProperties());
>
> then the RI outputs:
>
>  KEY=upper
>  key=upper
>  properties={key=[upper]}
>
> This appears to be consistent - albeit over-complicated IMHO.  But
> my testing also shows that the addRequestProperty (which appends the
> value to the value list associated with the key) does not treat keys
> case-insensitively so if you do:
>
>  URLConnection conn =
>    (URLConnection)(new URL("http://example.org/").openConnection());
>  conn.setRequestProperty("KEY", "upper");
>  conn.addRequestProperty("key", "lower");
>  System.out.println("KEY=" + conn.getRequestProperty("KEY"));
>  System.out.println("key=" + conn.getRequestProperty("key"));
>  System.out.println("properties=" + conn.getRequestProperties());
>
> you get:
>
>  KEY=lower
>  key=lower
>  properties={KEY=[upper], key=[lower]}
>
> where as I'd have expected:
>
>  properties={KEY=[lower,upper]}
>
> (since that is what you get if you do set* and add* with "KEY" in upper
> case for both calls).
>
> Personally, I struggle to see any consistency in the RI and suspect that
> we'd be better of just treating keys case-sensitively in all cases.
> This would at least have the virtue of being self-consistent and more
> predictable than the current implementation.  I tried writing test cases
> to map out the behaviour of the RI but just when I thought I'd figured
> it out I'd write another test and find that it didn't behave as I'd have
> expected.
>
> What do people think?  By my reading, treating keys case-sensitively in
> all cases would be consistent with the spec but will this cause problems
> for users?  Am I missing something and in fact the RI consistent?

the HTTP RFC says header fields are case-insensitive. i fixed this in
Android by switching from HashMap to
TreeMap(String.CASE_INSENSITIVE_ORDER), so we're case-insensitive but
case-preserving.

another thing to note is that the RI's getHeaderFields returns a
case-sensitive map, which seems like a mistake because it means
getHeaderField(x) is not equivalent to getHeaderFields().get(x).

to me, the HTTP RFC should override the RI, and we ought to file bugs
against the RI suggesting that it be consistently case-insensitive.

 --elliott

> I think since the same class is used to implement response headers, we
> may have to "be liberal"[0] and treat keys case-insensitively for the
> getRequestProperty() method or at least lookup case-sensitively thent
> also look them up case-insensitively before returning null.
>
> Regards,
>  Mark.
>
> [0] "Be liberal in what you accept, and conservative in what you send."
>        - Jon Postel
>
>
>



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/

Mime
View raw message