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

Fixing trailing slash bug #715

Merged
merged 1 commit into from
Nov 25, 2015
Merged

Fixing trailing slash bug #715

merged 1 commit into from
Nov 25, 2015

Conversation

subnetmarco
Copy link
Member

Closes issue #675.

local response, status = http_client.get(spec_helper.PROXY_URL.."/test-trailing-slash4/request", { hello = "world"})
assert.equal(200, status)
assert.equal("http://www.mockbin.org/request?hello=world", cjson.decode(response).url)
end)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if some more use cases could be described in the unit tests too: https://github.com/Mashape/kong/blob/next/spec/unit/core/resolver_spec.lua

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added one more

subnetmarco added a commit that referenced this pull request Nov 25, 2015
@subnetmarco subnetmarco merged commit 4281bdc into next Nov 25, 2015
@subnetmarco subnetmarco deleted the fix/trailing-slash branch November 25, 2015 01:34
@marcelmfs
Copy link

I think this change breaks https://www.ietf.org/rfc/rfc3986.txt which defines that a resource indicator in the html starting without a leading forward slash should be respected as a relative path. The change strip_request_path removing the trailing slash breaks the behaviour described in the RFC 3986. I lost the ability to migrate to latest version of kong because of this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants