-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[hotfix/resolver] avoid URL decoding in URI proxying #691
Conversation
e7a79f7
to
566a834
Compare
566a834
to
5164bc4
Compare
@thefosk We're on Docker :) so I guess something like mashape/kong:hostfix_uri will work :D |
BTW looks like the build system is broke :( |
Yes, the TravisCI and CircleCI integrations are not working properly, but it works locally if you execute I am building the docker package, please give me a few minutes. |
The Docker image is
|
Thanks ! I'll give a try and let you know. BTW aren't you guys using automated builds from Docker Hub with dynamic branches? That seems a pretty neat though I haven't had the time to test it myself ... |
Nope, we are not using automated builds - would be cool to spend some time doing that (also #219). Let me know if this hotfix addresses the issue. |
@thefosk Just wanted to confirm that your fix works. Thanks!! |
@@ -194,7 +194,8 @@ local function find_api(uri) | |||
end | |||
|
|||
function _M.execute(conf) | |||
local uri = ngx.var.uri | |||
local uri = stringy.split(ngx.var.request_uri, "?")[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why the split on "?" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some plugins, like the Request Transformations, can update the querystring. Here we are calculating the base URI, and then later in kong.lua
after all the "access" handlers in the plugins have been executed, we append the possibly updated querystring at the end. So here, since I just want the base URI for resolving the API using the path, I remove the querystring.
I personally don't like the split
on every request, but I can't see how we can do this differently. On the other side the split
function should be fast because it invokes stringy, which invokes strchr
under the hood.
Manually merged into both |
@thefosk Can you please let me know once the new tag is available on Docker Hub? |
@yoanisgil it's going to happen in the next version - we will announce it on Gitter and on our blog. |
This should fix issue #688.
@yoanisgil to test this do you need a distribution package? If yes, just tell me your system and I can create it for you.