[Bug libgcj/26487] New: Weird handling of HTTP Headers

[Bug libgcj/26487] New: Weird handling of HTTP Headers

[ifoox at redhat dot com]

When retrieving the headers from a URLConnection the results differ from what
happens with the Sun jvm. Specifically some headers get mashed together instead
of apearing as 2 different headers. I think this happens if a header with the
same name appears twice (with different values). I will attach a test case.


-- 
           Summary: Weird handling of HTTP Headers
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libgcj
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: ifoox at redhat dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #1 from ifoox at redhat dot com  2006-02-27 21:42 -------
On further investigation it seems like the headers are represented similarly
when they are recieved but URLConnection.getHeaderFieldKey and
URLConnection.getHeaderField don't behave as expected.


-- 

ifoox at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ifoox at redhat dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #2 from ifoox at redhat dot com  2006-02-27 21:44 -------
Created an attachment (id=10926)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10926&action=view)
Test case

Compile simply with 'TestLoginCookie.java' and run with 'java TestLoginCookie'.

I'll also attach what I found from this test case next.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #3 from ifoox at redhat dot com  2006-02-27 21:47 -------
Here's what the output of URLConnection.getHeaderFields() is for GCJ:
{null=[HTTP/1.1 200 OK], Date=[Mon, 27 Feb 2006 21:34:40 GMT],
Server=[Apache/2.0.46 (Red Hat)], Set-Cookie=[Bugzilla_login=192617;
path=/bugzilla; expires=Fri, 01-Jan-2038 00:00:00 GMT,
Bugzilla_logincookie=653228; path=/bugzilla; expires=Fri, 01-Jan-2038 00:00:00
GMT], Keep-Alive=[timeout=15, max=100], Connection=[Keep-Alive],
Transfer-Encoding=[chunked], Content-Type=[text/html; charset=]}

Here's the output for Sun 1.5:
{Connection=[Keep-Alive], Set-Cookie=[Bugzilla_logincookie=653232;
path=/bugzilla; expires=Fri, 01-Jan-2038 00:00:00 GMT, Bugzilla_login=192617;
path=/bugzilla; expires=Fri, 01-Jan-2038 00:00:00 GMT], null=[HTTP/1.1 200 OK],
Date=[Mon, 27 Feb 2006 21:35:37 GMT], Keep-Alive=[timeout=15, max=100],
Server=[Apache/2.0.46 (Red Hat)], Content-Type=[text/html; charset=],
Transfer-Encoding=[chunked]}

These are the same, but when I loop through the headers and print them out
here's what GCJ produces:
null:      'HTTP/1.1 200 OK'
Date:      'Mon, 27 Feb 2006 21:34:40 GMT'
Server:      'Apache/2.0.46 (Red Hat)'
Set-Cookie:      'Bugzilla_login=192617; path=/bugzilla; expires=Fri,
01-Jan-2038 00:00:00 GMT, Bugzilla_logincookie=653228; path=/bugzilla;
expires=Fri, 01-Jan-2038 00:00:00 GMT'
Keep-Alive:      'timeout=15, max=100'
Connection:      'Keep-Alive'
Transfer-Encoding:      'chunked'
Content-Type:      'text/html; charset='

and here's Sun:
null:      'HTTP/1.1 200 OK'
Date:      'Mon, 27 Feb 2006 21:35:37 GMT'
Server:      'Apache/2.0.46 (Red Hat)'
Set-Cookie:      'Bugzilla_login=192617; path=/bugzilla; expires=Fri,
01-Jan-2038 00:00:00 GMT'
Set-Cookie:      'Bugzilla_logincookie=653232; path=/bugzilla; expires=Fri,
01-Jan-2038 00:00:00 GMT'
Keep-Alive:      'timeout=15, max=100'
Connection:      'Keep-Alive'
Transfer-Encoding:      'chunked'
Content-Type:      'text/html; charset='

So it seems like libgcj's version of these 2 functions considers the two
instances of Set-Cookie to be one header whereas Sun considers them to be 2
separate headers. IBM 1.4.1 shows them as 2 seperate headers, although it
returns an empty Map from URLConnection.getHeaderFields() for some reason.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[mark at gcc dot gnu dot org]

------- Comment #4 from mark at gcc dot gnu dot org  2006-02-28 11:38 -------
Confirmed. It looks like getHeader(int), getHeader(String) and getHeaders()
have slightly different semantics. Where the first should return the actual
n-th header field value only, the second should return the value of the last
header with that name and the third should return a list of all values for that
header.

This seems slightly related to the bug just fixed by David Daney (bug #25851).
Adding him and Chris to the CC since they might know more about how to adopt
the design to support all three different semantics efficiently.

P.S. The testcase will not work at the moment against classpath CVS since it is
using a https URL, but Raif already has a patch for that which should hopefully
go through all the testing soon.


-- 

mark at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dog at gnu dot org, mark at
                   |                            |gcc dot gnu dot org, ddaney
                   |                            |at avtrex dot com
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-02-28 11:38:10
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #5 from ifoox at redhat dot com  2006-02-28 16:26 -------
Having looked at the Javadoc for these functions, Mark's description is
correct. Only that the function names are getHeaderField(int) (and
getHeaderFieldKey(int)), getHeaderField(String), and getHeaderFields().

I've changed the testcase to use the gnome bugzilla which does not use https.
Here we get 3 headers for Set-Cookie instead of 2.

I've also changed the test case to also try to use
getHeaderField("Set-Cookie"), with libgcj I get all three headers that have the
name "Set-Cookie":
Bugzilla_login=110573; path=/, Bugzilla_logincookie=E11DnsKscK; path=/,
DEFAULTFORMAT=short; path=/; expires=Fri, 01-Jan-2038 00:00:00 GMT

While Sun returns only the last one:
DEFAULTFORMAT=short; path=/; expires=Fri, 01-Jan-2038 00:00:00 GMT

The three headers were:
Set-Cookie:      'Bugzilla_login=110573; path=/'
Set-Cookie:      'Bugzilla_logincookie=yFd0huFJ3I; path=/'
Set-Cookie:      'DEFAULTFORMAT=short; path=/; expires=Fri, 01-Jan-2038
00:00:00 GMT'

I've changed the


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #6 from ifoox at redhat dot com  2006-02-28 16:27 -------
Created an attachment (id=10939)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10939&action=view)
Non-https test case

This test case uses http instead of https, it also exercises
getHeaderField(String) in addition to getHeaderField(int).


-- 

ifoox at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #10926|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[daney at gcc dot gnu dot org]

------- Comment #7 from daney at gcc dot gnu dot org  2006-02-28 16:54 -------
It is broken.  I am looking at it now.


-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |daney at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2006-02-28 11:38:10         |2006-02-28 16:54:38
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[daney at gcc dot gnu dot org]

------- Comment #8 from daney at gcc dot gnu dot org  2006-02-28 22:05 -------
Proposed patch is here:


http://developer.classpath.org/pipermail/classpath-patches/2006-February/000708.html


-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #9 from ifoox at redhat dot com  2006-03-01 16:11 -------
Hi David,

I tried to get classpath and try out applying the patch to test it out, but I
had some problems with it. I'll try again in a bit but I have some general
comments in the meanwhile.

It seems more appropriate to keep the headers in a Map than a List, especially
since getHeaderFields() has to return a map, and most opeartions are just a
simple getHeaderField(sometype). It seems that a LinkedHashMap (which Headers
extends) should be able to handle same-name headers, because it will just chain
them together. In theory :)

If this doesn't work out, it might be possible to store it in a Map in a
structure like: String -> [String, String, ...] where [] is a List. So we would
always have a String to List mapping and the List may contain 1 or more values
for that header.

Does that make any sense? :)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ddaney at avtrex dot com]

------- Comment #10 from ddaney at avtrex dot com  2006-03-01 17:10 -------
Subject: Re:  Weird handling of HTTP Headers

ifoox at redhat dot com wrote:
> ------- Comment #9 from ifoox at redhat dot com  2006-03-01 16:11 -------
> Hi David,
> 
> I tried to get classpath and try out applying the patch to test it out, but I
> had some problems with it. I'll try again in a bit but I have some general
> comments in the meanwhile.
> 
> It seems more appropriate to keep the headers in a Map than a List, especially
> since getHeaderFields() has to return a map, and most opeartions are just a
> simple getHeaderField(sometype). It seems that a LinkedHashMap (which Headers
> extends) should be able to handle same-name headers, because it will just chain
> them together. In theory :)
> 
> If this doesn't work out, it might be possible to store it in a Map in a
> structure like: String -> [String, String, ...] where [] is a List. So we would
> always have a String to List mapping and the List may contain 1 or more values
> for that header.
> 
> Does that make any sense? :)

You should be able to check-out the classpath HEAD and apply the patch 
to that.  Then copy the entire gnu/java/net/protocol/http directory 
contents directly into the corresponding place in the classpath 
directory of libgcj and rebuild.

My first attempt at fixing was to do as you suggested.  The problem is 
that you lose the order of the headers in the case where there were more 
than one header of the same name.  Enumerating the headers in the manner 
of your test program becomes quite complicated.  In the general case, 
the ability to delete some headers complicates things further.

I gave up and went down the road of a somewhat simpler implementation at 
the expense of lookup overhead.  In most cases there are fewer than 
about a dozen headers, so linear searching an ArrayList should not be 
too bad.

I think I will get a second opinion from Tromey et al. ...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[ifoox at redhat dot com]

------- Comment #11 from ifoox at redhat dot com  2006-03-01 18:10 -------
> I gave up and went down the road of a somewhat simpler implementation at 
> the expense of lookup overhead.  In most cases there are fewer than 
> about a dozen headers, so linear searching an ArrayList should not be 
> too bad.

You are right, I think that given this, it should be OK to go for the simpler
implementation for now.

> I think I will get a second opinion from Tromey et al. ...
Good idea. :)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug libgcj/26487] Weird handling of HTTP Headers

[daney at gcc dot gnu dot org]

------- Comment #12 from daney at gcc dot gnu dot org  2006-03-03 23:24 -------
Fixed by this patch to GNU classpath:

2006-03-03  David Daney  <ddaney@avtrex.com>

    * gnu/java/net/protocol/http/HTTPURLConnection.java
    (getRequestProperties): Rewrote.
    (addRequestProperty): Rewrote.
    (getHeaderFields): Rewrote.
    (getHeaderField): Rewrote.
    (getHeaderFieldKey): Rewrote.
    (getHeaderField): Removed useless cast.
    * gnu/java/net/protocol/http/Headers.java: Entire class rewritten.
    * gnu/java/net/protocol/http/Request.java (dispatch): Use new Headers
    interface.
    (notifyHeaderHandlers): Use new Headers interface.


libgcj will inherit the fix on the next classpath import.


-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487

[Bug classpath/26487] Weird handling of HTTP Headers

[daney at gcc dot gnu dot org]

-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libgcj                      |classpath
            Product|gcc                         |classpath
   Target Milestone|---                         |0.21
            Version|4.1.0                       |0.20


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26487