handle repeated HTTP header names correctly

[GoogleCodeExporter]

This is on trunk, r1143:

In buckets/headers_buckets.c:serf_bucket_headers_setx(), there's a comment
that says:

   /* ### should check if the header exists before adding. overwrite. */

The first part of the comment is correct: the code should check to see if
we've already received a header with that name.  But if we have, we
shouldn't overwrite the old value with the new value.  Instead, we should
append the new header value to the old value, separated by a comma.  See
http://www.ietf.org/rfc/rfc2616.txt, section 4.2:

   Multiple message-header fields with the same field-name MAY be
   present in a message if and only if the entire field-value for
   that header field is defined as a comma-separated list [i.e.,
   #(values)].  It MUST be possible to combine the multiple header
   fields into one "field-name: field-value" pair, without changing
   the semantics of the message, by appending each subsequent
   field-value to the first, each separated by a comma. The order
   in which header fields with the same field-name are received is
   therefore significant to the interpretation of the combined
   field value, and thus a proxy MUST NOT change the order of these
   field values when a message is forwarded.

I'm working on a patch now.

Original issue reported on code.google.com by kfo...@gmail.com on 24 Oct 2007 at 9:40

[GoogleCodeExporter]

Original comment by kfo...@gmail.com on 24 Oct 2007 at 11:46

Attachments:

• issue-29-patch.txt

[GoogleCodeExporter]

I had some trouble posting to the dev list about this.  Here's the
correspondence about that between Justin Erenkrantz and me:

> > Ack.  You do have to subscribe to post, but if you'd like I can
> > post your message - or rather, you can just paste that into the
> > issue tracker?
> 
> Hmmm.  I did subscribe, after the first bounce, but I subscribed in
> "no email (I'll read it on the web)" mode.  I wonder if that wasn't
> enough to give me clearance to post.  Anyway, I've attached it to the
> issue now.

And here is what I tried to post:

   A while back, I promised Justin a patch for a buglet in Serf, whereby
   it didn't handle repeated HTTP headers correctly (I mean headers where
   the header name appears multiple times, with different values).  This
   is now done, and I've attached the patch to:

      http://code.google.com/p/serf/issues/detail?id=29

   As the note in the log message indicates, the change raises some
   further issues, ones that I'm not sure how/whether to resolve.

Original comment by kfo...@gmail.com on 25 Oct 2007 at 12:18

[GoogleCodeExporter]

I also ran into this issue when writing mod_serf.  Quite annoying.

Original comment by paul.que...@gmail.com on 13 Nov 2007 at 4:22

[GoogleCodeExporter]

Here's an updated version of the patch that updates the docs for
serf_bucket_headers_setn to note that a copy can actually be performed in the 
case
where the header already appears and adds a test for the new behavior.

Original comment by roo...@gmail.com on 14 Nov 2007 at 8:24

Attachments:

• issue-29-patch-2.diff

[GoogleCodeExporter]

Fix committed in r1150.

Original comment by roo...@gmail.com on 14 Nov 2007 at 9:12

• Changed state: Fixed