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

Problem: routing url-encoded URLs doesn't work properly #173

Merged
merged 1 commit into from
Feb 22, 2018

Conversation

yrashk
Copy link
Contributor

@yrashk yrashk commented Feb 22, 2018

The URL is being decoded before any matching is done.

So, for example, /hello/%3Fa will not match /hello/a because %3F
is already decoded to ?.

Solution: use raw URL to do the routing and decode individual parts
instead.

yrashk added a commit to sit-fyi/sit that referenced this pull request Feb 22, 2018
Apparently, it is impossible to specify some more involved
JMESPath expressions, for example, ones that would contain
`?contains` or a slash in them.

Solution: make sure both frontend use percent encoding

The reason for this malfunction is that, firstly, the frontend part
didn't encode the URL components being 1) sent to the server 2) updated
in the location of the page.

Secondly, the backend's library for handling HTTP (rouille) eagerly
decoded URLs thus rendering it impossible to match on these values
as `?` or `/` would trigger stops. A forked version of roulle is used
now. If the [pull request](tomaka/rouille#173)
will be accepted, can switch back at that time.
@tomaka
Copy link
Owner

tomaka commented Feb 22, 2018

Thanks, looks good! Please also add am entry in the changelog and I'll merge.

The URL is being decoded before any matching is done.

So, for example, `/hello/%3Fa` will not match `/hello/a` because `%3F`
is already decoded to `?`.

Solution: use raw URL to do the routing and decode individual parts
instead.
@yrashk
Copy link
Contributor Author

yrashk commented Feb 22, 2018

Updated the changelog.

@tomaka tomaka merged commit 0b1d8a3 into tomaka:master Feb 22, 2018
@yrashk
Copy link
Contributor Author

yrashk commented Mar 4, 2018

@tomaka Hi Pierre, what's the ETA for the next release? I am running into vendoring/frozen builds issues packaging my app for NixOS -- I am yet to see a workaround for git-based dependencies in their process. In the meantime, the only solid way for things to work is to depend on crates.io-only packages. Hence the question about the next release!

Thanks.

@tomaka
Copy link
Owner

tomaka commented Mar 5, 2018

@yrashk If you submit a PR that bumps the version in Cargo.toml and CHANGELOG, it will automatically be published once merged.

@yrashk
Copy link
Contributor Author

yrashk commented Mar 5, 2018 via email

yrashk added a commit to sit-fyi/issue-tracking that referenced this pull request Apr 9, 2018
Apparently, it is impossible to specify some more involved
JMESPath expressions, for example, ones that would contain
`?contains` or a slash in them.

Solution: make sure both frontend use percent encoding

The reason for this malfunction is that, firstly, the frontend part
didn't encode the URL components being 1) sent to the server 2) updated
in the location of the page.

Secondly, the backend's library for handling HTTP (rouille) eagerly
decoded URLs thus rendering it impossible to match on these values
as `?` or `/` would trigger stops. A forked version of roulle is used
now. If the [pull request](tomaka/rouille#173)
will be accepted, can switch back at that time.
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.

2 participants