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

fix(resolver) percent-encode querystring before proxying #752

Merged
merged 3 commits into from
Dec 2, 2015

Conversation

thibaultcha
Copy link
Member

Percent-encode query args when re-attaching them to the upstream_uri.
Since ngx.encode_args does not perform percent-encoding on various
reserved characters, this implements a custom utils.encode_args
function which uses LuaSocket's url.encode function. It tries to mimic
the ngx.encode_uri behaviour 100%.

Ideally, ngx.encode_args would proceed to the percent-encoding itself (see
openresty/lua-nginx-module#542).

This also makes some perf and style changes, as well as implementing better
unit testing for resolver.execute().

Fix #749

@thibaultcha thibaultcha added the pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) label Dec 1, 2015
- directly test `resolver.execute()`
- adds a stub of `ngx.shared.DICT` for unit testing where
  `kong.tools.cache` is being used (it requires shm dicts)
- tiny refactors for testability
- more use cases covered in specs
Percent-encode query args when re-attaching them to the `upstream_uri`.
Since `ngx.encode_args` does not perform percent-encoding on various
reserved characters, this implements a custom `utils.encode_args`
function which uses LuaSocket's `url.encode` function. It tries to mimic
the `ngx.encode_uri` behaviour 100%.

Ideally, `ngx.encode_args` would proceed to the percent-encoding itself (see
openresty/lua-nginx-module#542).

This also makes some perf and style changes.

Fix #749
Also fix a bunch of style issues.
@thibaultcha thibaultcha force-pushed the fix/resolver-query-encoding branch from 491233d to 6274cc4 Compare December 1, 2015 04:39
thibaultcha added a commit that referenced this pull request Dec 2, 2015
fix(resolver) percent-encode querystring before proxying
@thibaultcha thibaultcha merged commit 51918e8 into next Dec 2, 2015
@thibaultcha thibaultcha deleted the fix/resolver-query-encoding branch December 2, 2015 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant