Connection problem with firefox

[zoggy]

Firefox fails to connect via websocket, while chrome succeeds. I could track the problem down to this condition in websocket_lwt.ml, in function establish_server:

  && List.mem "upgrade"
  @@ List.map String.lowercase @@ C.Header.get_multi headers "connection"

If I remove this condition, then the connection with Firefox works. By printing the contents of the "connection" headers, Firefox sends keep-alive, Upgrade, while Chrome only send Upgrade, thus explaining why the condition fails with Firefox. Maybe splitting the string on commas and testing each element of the resulting list would be more robust ?

[vbmithr]

On 18/06/2015 19:51, Zoggy wrote:

If I remove this condition, then the connection with Firefox works. By printing the contents of the "connection" headers, Firefox sends |keep-alive, Upgrade|, while Chrome only send |Upgrade|, thus explaining why the condition fails with Firefox. Maybe splitting the string on commas and testing each element of the resulting list would be more robust ?

Ahhh right this is actually a regression, somebody sent a patch for this earlier.

I don't understand though, I thought that get_multi would return a list of headers, and trying to find upgrade in this list would be enough.

I need to have a closer look at what get_multi actually do.

Vincent

[vbmithr]

Could you make a PR and a test for this ? I would do it, but I don't know how you make firefox connect to websocket.

[vbmithr]

And then I'll make a new release :)

[zoggy]

Ok :-)

[vbmithr]

Thanks. But IMO this should be done by Cohttp, and I'm sure it is a bug there. Let me investigate on Cohttp's side first!

[zoggy]

No problem. I took a look at cohttp and it seems to handle lists in case the headers contains several times the same header.

[vbmithr]

On 19/06/2015 09:26, Zoggy wrote:

No problem. I took a look at cohttp and it seems to handle lists in case the headers contains several times the same header.

Yeah but there is definitely a bug there, they don't parse correctly the headers, they don't split on ',' at all.

cc @avsm @rgrinberg

I'm fixing this.

Vincent

[vbmithr]

Can you check if my latest commit fixes this issue ?

[zoggy]

Yes !

[avsm]

Is there a Cohttp bug on this one?

[vbmithr]

@avsm No, there is not.

[vbmithr]

@zoggy: Yes you can check or yes it solves the problem ? ;)

[zoggy]

Sorry, yes it works ;)

[vbmithr]

Ok, great!