[cp-patches] RFC: Headers fixlet

David Daney ddaney at avtrex.com
Tue Mar 7 17:18:57 UTC 2006


Wolfgang Baer wrote:
> Hi David,
> 
> David Daney wrote:
> 
>>Wolfgang Baer wrote:
>>
>>
>>>Hi,
>>>
>>>I wrote mauve tests for the various HttpURLConnection request properties
>>>methods and notices some minor bugs in the new Headers implementation.
>>>
>>>The patch fixes these and adds documentation to this class in more
>>>detail.
>>>The tests show that put method used in setRequestProperty should not
>>>remove
>>>all occurences of the property but only the last on in the list. Also as
>>>its internal case sensitive the key must not be replaced (only the
>>>value),
>>>otherwise getAsMap() can produce wrong maps. I reimplemented putAll()
>>>with
>>>the same semantics.
>>>
>>
>>I wrote both put and putAll to mimic their old bahavior.  I wanted to
>>change the behavior as little as possible, while still fixing it.
>>
>>putAll() IIRC is only used internally, to propagate the headers into the
>>Request object.  I question whether it should be changed.
> 
> 
> OK
> 
>>If you have test cases that show that the current putAll() is broken,
>>then the change should be made.  But change just for change's sake I am
>>not so sure.
> 
> 
> No, I have no testcases for putAll() - only for put(). As you said its only
> used internally to override settings. The change won't affect the current
> usage so far. The reason for reimplementation was more that people (later if
> they use that class) may assume that putAll() behaves as put() on a whole
> Headers collection which is not true.
> 
> If we don't change the method we need to document clearly that its not the
> behaviour of put() on a Headers collection. Also I think we can at least save
> one of the iterations.
> 

The thing I worry about is headers like 'Content-Length'.  In order for 
HTTP to function correctly we must supply the correct value.  I think by 
allowing putAll() to append, we might have a situation where there could 
be multiple headers for some critical protocol headers that could break 
things.




More information about the Classpath-patches mailing list