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

An incredibly arcane missing header in response to a POST #19079

Closed
jeffsilverm opened this issue Jun 26, 2016 · 3 comments · Fixed by #19509
Closed

An incredibly arcane missing header in response to a POST #19079

jeffsilverm opened this issue Jun 26, 2016 · 3 comments · Fixed by #19509
Assignees
Labels
:Core/Infra/REST API REST infrastructure and utilities good first issue low hanging fruit

Comments

@jeffsilverm
Copy link

Elasticsearch version: 2.3.3

JVM version:
openjdk version "1.8.0_91"
OpenJDK Runtime Environment (build 1.8.0_91-8u91-b14-0ubuntu4~16.04.1-b14)
OpenJDK 64-Bit Server VM (build 25.91-b14, mixed mode)

OS version:
Linux jeff-desktop 4.4.0-24-generic #43-Ubuntu SMP Wed Jun 8 19:27:37 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
(Ubuntu 16.04)

Description of the problem including expected versus actual behavior:
This is going to sound incredibly arcane. There is a missing header in the response to a successful PUT. The following curl commands work exactly as they are supposed to, there is no problem there.

jeffs@jeff-desktop:~$ curl -i -XPOST 'jeffsilverm.ddns.net:9200/customer/external?pretty' -d '
{
  "name": "Jane Eerickson"
}'
HTTP/1.1 201 Created
Content-Type: application/json; charset=UTF-8
Content-Length: 201

{
  "_index" : "customer",
  "_type" : "external",
  "_id" : "AVWLfkcV0A40HdQsPZ72",
  "_version" : 1,
  "_shards" : {
    "total" : 2,
    "successful" : 1,
    "failed" : 0
  },
  "created" : true
}

jeffs@jeff-desktop:~$ curl -i -XGET 'jeffsilverm.ddns.net:9200/customer/external/AVWLfkcV0A40HdQsPZ72?pretty'
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 173

{
  "_index" : "customer",
  "_type" : "external",
  "_id" : "AVWLfkcV0A40HdQsPZ72",
  "_version" : 1,
  "found" : true,
  "_source" : {
    "name" : "Jane Eerickson"
  }
}jeffs@jeff-desktop:~$ curl -i -XGET 'jeffsilverm.ddns.net:9200/customer/external/AVWLn7k00A40HdQsPZ78?pretty'
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 162

{
  "_index" : "customer",
  "_type" : "external",
  "_id" : "AVWLn7k00A40HdQsPZ78",
  "_version" : 1,
  "found" : true,
  "_source" : {
    "name" : "Bob"
  }
}
jeffs@jeff-desktop:~$ 
jeffs@jeff-desktop:~$ 

The problem is that, according to RFC 2616 section 10.2.2, (https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html) the Location header should be included in the 201 response:

10.2.2 201 Created

The request has been fulfilled and resulted in a new resource being created. The newly created resource can be referenced by the URI(s) returned in the entity of the response, with the most specific URI for the resource given by a Location header field. The response SHOULD include an entity containing a list of resource characteristics and location(s) from which the user or user agent can choose the one most appropriate. The entity format is specified by the media type given in the Content-Type header field. The origin server MUST create the resource before returning the 201 status code. If the action cannot be carried out immediately, the server SHOULD respond with 202 (Accepted) response instead.

A 201 response MAY contain an ETag response header field indicating the current value of the entity tag for the requested variant just created, see section 14.19.

Steps to reproduce:
1.curl -i -XPOST 'jeffsilverm.ddns.net:9200/customer/external?pretty' -d ' { "name": "Bob" }' | fgrep -i Location
returns nothing.
3.

Describe the feature:
There should be a Location header. In the example above, the URL 'jeffsilverm.ddns.net:9200/customer/external/AVWLn7k00A40HdQsPZ78?pretty' works flawlessly.

jeffs@jeff-desktop:~$ curl -i -XGET 'jeffsilverm.ddns.net:9200/customer/external/AVWLn7k00A40HdQsPZ78?pretty'
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 162

{jeffs@jeff-desktop:~$ curl -i -XGET 'jeffsilverm.ddns.net:9200/customer/external/AVWLn7k00A40HdQsPZ78?pretty'
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 162

{
  "_index" : "customer",
  "_type" : "external",
  "_id" : "AVWLn7k00A40HdQsPZ78",
  "_version" : 1,
  "found" : true,
  "_source" : {
    "name" : "Bob"
  }
}
jeffs@jeff-desktop:~$ 
  "_index" : "customer",
  "_type" : "external",
  "_id" : "AVWLn7k00A40HdQsPZ78",
  "_version" : 1,
  "found" : true,
  "_source" : {
    "name" : "Bob"
  }
}
jeffs@jeff-desktop:~$ 

The only problem is the missing Location field in the returned HTTP header when doing the POST. Note that since the GET returns status code 200 (correctly), no Location header is required by RFC 2616. The POST returns status code 201 (correctly), and that is where the Location header is required.

The work around is to parse the response and look for the _id field, which can be done using jq or the python json.loads method. So I believe that this is a very low priority issue.

@clintongormley clintongormley added discuss :Core/Infra/REST API REST infrastructure and utilities labels Jun 27, 2016
@s1monw s1monw added help wanted adoptme and removed discuss labels Jul 1, 2016
@s1monw
Copy link
Contributor

s1monw commented Jul 1, 2016

lets fix it - thanks for reporting.. the location should just be the GET URL I guess?

@s1monw s1monw added the good first issue low hanging fruit label Jul 1, 2016
@jeffsilverm
Copy link
Author

In the case of

jeffsilverm.ddns.net:9200/customer/external?pretty' -d '
{
"name": "Jane Eerickson"
}'

I my example, elasticsearch returns

HTTP/1.1 201 Created
Content-Type: application/json; charset=UTF-8
Content-Length: 201

{
"_index" : "customer",
"_type" : "external",
"_id" : "AVWLfkcV0A40HdQsPZ72",
"_version" : 1,
"_shards" : {
"total" : 2,
"successful" : 1,
"failed" : 0
},
"created" : true
}

I believe the resulting header should be

HTTP/1.1 201 Created
Content-Type: application/json; charset=UTF-8
Location: jeffsilverm.ddns.net:9200/customer/_AVWLfkcV0A40HdQsPZ72_Content-Length:
SOMETHING
{ "_index" : "customer", "_type" : "external", "_id" :
"AVWLfkcV0A40HdQsPZ72", "_version" : 1, "_shards" : { "total" :
2, "successful" : 1, "failed" : 0 }, "created" : true}

The use case is the ability to create a list of pointers to documents in a
client of elasticsearch.

Thank you

Jeff

On Fri, Jul 1, 2016 at 2:24 AM, Simon Willnauer notifications@github.com
wrote:

lets fix it - thanks for reporting.. the location should just be the GET
URL I guess?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#19079 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAhhh1TWE2ilECXZDuyBrJisTyLIDkcmks5qRNy3gaJpZM4I-gTq
.

Jeff Silverman, linux sysadmin
nine two four twentieth avenue east
Seattle, WA, nine eight one one two -3507
(253) 459-2318
jeffsilverm@gmail.c0m (note the zero!)
http://www.commercialventvac.com
See my portfolio of writings and talks
http://www.commercialventvac.com/portfolio.html

@nik9000 nik9000 self-assigned this Jul 19, 2016
@nik9000 nik9000 removed the help wanted adoptme label Jul 19, 2016
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Jul 25, 2016
This adds a header that looks like `Location: /test/test/1` to the
response for the index/create/update API. The requirement for the header
comes from https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

https://tools.ietf.org/html/rfc7231#section-7.1.2 claims that relative
URIs are OK. So we use an absolute path which should resolve to the
appropriate location.

Closes elastic#19079

This makes large changes to our rest test infrastructure, allowing us
to write junit tests that test a running cluster via the rest client.
It does this by splitting ESRestTestCase into two classes:
* ESRestTestCase is the superclass of all tests that use the rest client
to interact with a running cluster.
* ESClientYamlSuiteTestCase is the superclass of all tests that use the
rest client to run the yaml tests. These tests are shared across all
official clients, thus the `ClientYamlSuite` part of the name.
@vinniefalco
Copy link

Is a 201 response with no content required to include "Content-Length: 0" ?

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 good first issue low hanging fruit
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants