Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Deprecation HTTP headers normalize comma in messages #22986

Closed
Mpdreamz opened this issue Feb 5, 2017 · 2 comments
Closed

Should Deprecation HTTP headers normalize comma in messages #22986

Mpdreamz opened this issue Feb 5, 2017 · 2 comments
Assignees
Labels
:Core/Infra/REST API REST infrastructure and utilities

Comments

@Mpdreamz
Copy link
Member

Mpdreamz commented Feb 5, 2017

Not entirely sure what the HTTP spec status of response headers is but with request headers

Key: Value1, Value2

is equivalent to:

Key: Value1
Key: Value2

Quite a few deprecation messages contain a comma which causes an ambiguity when I get to actually parse these (from .NET BCL in this case.

e.g:

HTTP/1.1 200 OK
Warning: Deprecated field [fielddata_fields] used, expected [docvalue_fields] instead
Warning: [groovy] scripts are deprecated, use [painless] scripts instead
content-type: application/json; charset=UTF-8
content-length: 3676

Yields:

image

Which is impossible to stitch back together as two messages.

@nik9000 nik9000 added :Core/Infra/REST API REST infrastructure and utilities discuss labels Feb 5, 2017
@nik9000
Copy link
Member

nik9000 commented Feb 5, 2017

Bleh. Looks like we shouldn't use comma, yeah. This SO question has some fun discussion about it. It looks like a lot of standard headers contain , and can't be merged, but well behaved headers don't contain ,. It links to this (MPLed) code that shows how a browser does it....

@jasontedor
Copy link
Member

It is indeed the case that the warning header can be multi-valued which means that we do need to address the comma-separated issue here. On closer inspection of the HTTP spec, we are not following the warning header specification. We should be prepending each warning with a code (we can just use 199 miscellaneous) and an agent (we can just use the Elasticsearch version), and then we should be quoting each warning string to address the comma issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/REST API REST infrastructure and utilities
Projects
None yet
Development

No branches or pull requests

3 participants