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

Get a document return 400 bad request if id contains both comma and slash #1186

Open
iceking2001 opened this issue Sep 17, 2019 · 3 comments
Assignees
Milestone

Comments

@iceking2001
Copy link

iceking2001 commented Sep 17, 2019

Which version of Elastic are you using?

[x] elastic.v6 (for Elasticsearch 6.x)

Please describe the expected behavior

Get and update a document successed if its id contains both comma and slash.

Please describe the actual behavior

The es return 400 bad request error.

Any steps to reproduce the behavior?

Insert a document by bulk api , which id is "c7ccd3c6f4a7: <[email protected]/O=, @gg.hop.com", then retrieves this document by GetService api, print its request likes:

GET /index/_doc/c7ccd3c6f4a7:%20%20%[email protected]/O=,%[email protected]?routing=102345 

the return is:

{"error":"no handler found for uri [/index/_doc/c7ccd3c6f4a7:%20%20%[email protected]/O=,%[email protected]?routing=102345] and method [GET]"}

I notice that if the version <= v6.2.13, there is no such issue. With compare between v6.2.13 and v6.2.14, found that some modify in aws_v4.go cause the problem.

req.URL.Scheme = "https"
if strings.Contains(req.URL.RawPath, "%2C") {
    // Escaping path
    req.URL.RawPath = url.PathEscape(req.URL.RawPath)
}

If I comment out this code, it works. It has a correct request, and with the correct url encoding:

GET /index/_doc/c7ccd3c6f4a7%3A%20%20%3CMichael.Lee%40chi.com%2FO%3D%2C%20%40gg.hop.com?routing=102345 

So i am curious about what this code does, if comments out them, what will happen?

@olivere
Copy link
Owner

olivere commented Sep 19, 2019

I will have to review why that is. Maybe it was a temporary problem we were trying to fix. Seems wrong to me as well: fiddling with the URL encoding.

@olivere olivere self-assigned this Sep 19, 2019
@olivere olivere added the bug label Sep 19, 2019
@olivere olivere added this to the 7.0.7 milestone Sep 19, 2019
@olivere
Copy link
Owner

olivere commented Oct 4, 2019

It came in with #962, and was carried over from this client. Can you check if that hack is still required?

@olivere olivere modified the milestones: 7.0.7, 7.0.8 Oct 6, 2019
@olivere olivere modified the milestones: 7.0.8, 7.0.9 Oct 14, 2019
@iceking2001
Copy link
Author

iceking2001 commented Oct 24, 2019

Sorry for the late replay. I check three case without this hack:

  1. contains both comma and slash
  2. only have comma
  3. only have slash

All of above don't get 403 Forbidden error from aws.

@olivere olivere modified the milestones: 7.0.9, 7.0.10 Nov 4, 2019
@olivere olivere modified the milestones: 7.0.10, 7.0.11 Jan 4, 2020
@olivere olivere modified the milestones: 7.0.11, 7.0.12 Feb 7, 2020
@olivere olivere modified the milestones: 7.0.12, 7.0.13 Feb 27, 2020
@olivere olivere modified the milestones: 7.0.13, 7.0.14 Mar 27, 2020
@olivere olivere modified the milestones: 7.0.14, 7.0.15 Apr 10, 2020
@olivere olivere modified the milestones: 7.0.15, 7.0.16 May 1, 2020
@olivere olivere modified the milestones: 7.0.16, 7.0.17, 7.0.18 Jun 2, 2020
@olivere olivere removed this from the 7.0.18 milestone Jul 13, 2020
@olivere olivere added this to the 7.0.19 milestone Jul 13, 2020
@olivere olivere modified the milestones: 7.0.19, 7.0.20 Jul 25, 2020
@olivere olivere modified the milestones: 7.0.20, 7.0.21 Aug 30, 2020
@olivere olivere modified the milestones: 7.0.21, 7.x Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants