HTTP Request with multiple Header values written to wire incorrectly?

[Brian McClain]

(or perhaps I simply misunderstand how it should be done :) )

Hi all,

Fairly new to Go, but having a blast learning it so far. I wanted to bring up some behavior I'm noticing that I'm not sure if it's expected or something may need a closer eye on.

Consider the following (again, newbie, so excuse any poor coding)

package main

import (

  "fmt"

  "net/http"

  "io/ioutil"

  "net/http/httputil"

)

func main() {

  client := &http.Client{}

  req, _ := http.NewRequest("GET", "http://localhost:4567", nil)

  req.Header.Add("Connection", "keep-alive")

  req.Header.Add("Connection", "Upgrade")

  req.Header.Add("Upgrade", "websocket")

  dump, _ := httputil.DumpRequestOut(req, false)

  fmt.Println(string(dump))

  res, _ := client.Do(req)

  body, _ := ioutil.ReadAll(res.Body)

  res.Body.Close()

  fmt.Printf("%s", body)

}

Running on port 4567 is a simple HTTP server (more on this later). A couple tests was a simple HTTP server written in Go using net/http, and another in Ruby using Sinatra/thin. The important part is the DumpRequestOut output:

GET / HTTP/1.1

Host: localhost:4567

User-Agent: Go 1.1 package http

Connection: keep-alive

Connection: Upgrade

Upgrade: websocket

Accept-Encoding: gzip

As I understand, this is what's actually written to the wire when the request is made, but I've verified this is exactly what's written over the wire with Wireshark. My concern is that the Connection header, in this case, is written twice, when it's my understanding the expected result it:

Connection: keep-alive, Upgrade

The Go net/http library on the server side handles this correctly, accessing the Headers map of the request returns an array of strings, the first with the value "keep-alive", the second with the value "Upgrade".

However, this causes issues in other servers. For example, in the Sinatra/thin server, it's expected to follow rfc2616 section 4.2, namely:

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

In the Sinatra/thin example, the latter header overwrites the former, so only the "Upgrade" header is received. Other clients (Firefox for example) also follow the above comma-seperated format (ie. Connection: keep-alive, Upgrade).

I believe the code of interest is at http://golang.org/src/pkg/net/http/transfer.go, starting on line 158. I considered attempting this change and submitting it, but didn't feel like my first contribution should radically change how the net/http library worked :)

So my question is: Is this correct behavior? Or should the writer combine the multiple values of each header into a comma-sperated list?

Thanks!

Brian

@BrianMMcClain

[Brian McClain]

Oh, and I forgot to mention, this was tested on lastest stable: 

➜  ~  go version

go version go1.2.2 darwin/amd64

- Brian

@BrianMMcClain

[Mikio Hara]

On Thu, May 29, 2014 at 11:19 AM, Brian McClain <brianm...@gmail.com> wrote:

> In the Sinatra/thin example, the latter header overwrites the former,

that's unfortunate. please file an issue if it's a serious problem to
you; http://golang.org/project.

> So my question is: Is this correct behavior?

i'd personally prefer the current behaviror. fwiw:
http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-26#section-6.1.

[Brian McClain]

Thanks Mikio. Upon further reading, you're correct, it seems both are to spec (two headers with the same field-name vs one header with the field-values in a comma-separated string)

Curiosity leads me to investigate other server middleware to see how they behave as well, but the more I've read, the more I agree that it would be best to stick with current behavior and perhaps it's those pieces of the stack that are not to spec. Certainly something to bring up with the Thin crowd.

Thanks!

Brian