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

URI Too Large Errors #319

Closed
pthurlow opened this issue Aug 26, 2016 · 4 comments
Closed

URI Too Large Errors #319

pthurlow opened this issue Aug 26, 2016 · 4 comments
Assignees
Labels

Comments

@pthurlow
Copy link

Came across this issue when our security tags started getting large. It appears that the headers sent to algolia are being inlined as query params in the url. This can be seen on line 74 here. When I removed those and put them in the request header I only received a 400 when the header x-algolia-agent was removed from the query params. Leaving just that one in the query params and moving the rest to request headers solved my issue.

I'd like to understand why these headers are being inlined into the url so that I might be able to supply a patch for this issue. Also any documentation on required headers/query params would be awesome as I could not find any.

@vvo vvo self-assigned this Aug 29, 2016
@vvo
Copy link

vvo commented Aug 29, 2016

Hi @pthurlow, could you maybe send here a CURL like command that results in an error when your security tags are too big? If this is not OK given the security of your application, please send this to [email protected] (and mention me and algolia javascript client)

You can get a CURL like command from the network tab in chrome, right clicking a failed request and "copy as CURL".

We recently updated the way we provide security at Algolia and you can now always use secured API keys: https://www.algolia.com/doc/guides/security/api-keys.

Those keys will then be sent in POST when doing searches if the key is too big.

Since then, I am interested in seing what's inside your query.

As for the reasoning, we only put in the HTTP headers, headers that are considered as "simple HEADERS" given the XHR spec: https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Simple_requests

That way, we can avoid doing an extra OPTIONS request in browsers.

Let me know how it goes.

@vvo vvo added the question label Aug 29, 2016
@pthurlow
Copy link
Author

pthurlow commented Aug 30, 2016

I sent an example request header to your support. When you say "Those keys will then be sent in POST when doing searches if the key is too big" do you just mean the request is made as a POST rather than a GET? If that is the case this problem will still persist because this is nginx or other server limits on URI request length. Ideally these keys would be inserted into the body to avoid this issue altogether, but as I mentioned it appears the js client is simply inlining all security tags and api keys into the url query string.

@pthurlow
Copy link
Author

After a bit more digging I think I've found the issue. The logic which determines whether to stick the apiKey into the body seen here assumes that the body object contains a params field. However this is not the case for non fallback requests as there is a query request array on the body with the params field on each array element. This is done here

If I remove the check for body.params it correctly inserts the api key into the POST body. So maybe you can advise on the best course of action for a fix.

as a related issue, we also use the setSecurityTags() method to send what we thought were required security tags by the api so it can compare it to the encoded api key. This also causes issues because it appears this header cannot be stuck in the post body like the api key (i tried but it didn't recognize any names i guessed, heh). since security tags are long just like the api key this causes the same URI too large issue but there is no discernible way to put them into the body. Of course, when I tried just not sending the security tags it still worked! so I supposed those are not required after all (maybe you can comment on that)

@vvo
Copy link

vvo commented Aug 30, 2016

After a bit more digging I think I've found the issue. The logic which determines whether to stick the apiKey into the body seen here assumes that the body object contains a params field. However this is not the case for non fallback requests as there is a query request array on the body with the params field on each array element. This is done here

It seems not to be an issue with "fallback requests" but an issue being that requests done using client.search() will not have their API keys inlined when too big.

If I remove the check for body.params it correctly inserts the api key into the POST body. So maybe you can advise on the best course of action for a fix.

I will fix this and release.

Of course, when I tried just not sending the security tags it still worked! so I supposed those are not required after all (maybe you can comment on that)

Yes there's no more a need to use setSecurityTags if you already generated a secured API key with all the tags. All our security features are now provided by secured API keys and can be found here: https://www.algolia.com/doc/guides/security/api-keys.

vvo pushed a commit that referenced this issue Aug 30, 2016
@vvo vvo added bug and removed question labels Aug 30, 2016
vvo pushed a commit that referenced this issue Aug 30, 2016
vvo pushed a commit that referenced this issue Aug 31, 2016
@vvo vvo closed this as completed in 01b6742 Aug 31, 2016
vvo pushed a commit that referenced this issue Aug 31, 2016
  * fix(client.search): accept very long API keys
    fixes #319

    also fix uglify-js version because it's buggy in IE8:
    mishoo/UglifyJS#1039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants