sockjs / sockjs-protocol

An attempt to define SockJS protocol
http://sockjs.org
203 stars 29 forks source link

Adding support for multiple single HTTP header entries. #77

Closed danbev closed 9 years ago

danbev commented 10 years ago

When parsing a HTTP response that has multiple HTTP headers as single entries, as opposed to a single header with a comma separated list of values, only the last value was added to the dictionary of headers.

This addition checks if the header name has already been added to the dictionary, and if so, appends the value to the already existing key's value.

brycekahle commented 9 years ago

Are there particular headers you are using that require this support? I realize it is allowed by HTTP, but only for headers that have a value that is defined as a comma separated list.

danbev commented 9 years ago

@brycekahle Our server returns all supported methods to take advantage of the preflight cache. I noticed that only one Access-Control-Request-Method was made available to the test for validation.

brycekahle commented 9 years ago

@danbev Are you talking about #82? This is something different.

danbev commented 9 years ago

@brycekahle This is different from #82 in that this allows the parsing of multiple header. #82 is a change to the "assert" enabling a server to specifically return all the request headers that it supports in the Access-Control-Allow-Headers. Does that make sense?

brycekahle commented 9 years ago

I understand, but this is adding the ability to parse if multiple instances of the same header are specified. Are you actually doing that?

danbev commented 9 years ago

What I can see is that the headers returned are for example:

Access-Control-Allow-Headers: a

Access-Control-Allow-Headers: b

Access-Control-Allow-Headers: c

This is from adding a print(header) line: https://github.com/sockjs/sockjs-protocol/blob/master/utils_03.py#L210

brycekahle commented 9 years ago

So my point was that is your server code generating that output. Do you need to generate in that format? Or can you combine those lines into a single one.

danbev commented 9 years ago

Ah I got it now. I take a close look and see what can be done.

torsdag 11 december 2014 skrev Bryce Kahle notifications@github.com:

So my point was that is your server code generating that output. Do you need to generate in that format? Or can you combine those lines into a single one.

— Reply to this email directly or view it on GitHub https://github.com/sockjs/sockjs-protocol/pull/77#issuecomment-66684712.

danbev commented 9 years ago

@brycekahle Sorry about the long delay on this. We have taken care of our server side to now return a single header with comma separated values. This allows all the tests to pass for us, so feel free to close this pull request unless you think that the test suite should be able to handle multiple single value headers.