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.

Sign up for GitHub

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

Jump to bottom

Url decoding is only applied on the query parameters #992

Closed
paulharris opened this issue Feb 6, 2025 · 7 comments · May be fixed by #993
Closed

Url decoding is only applied on the query parameters #992

paulharris opened this issue Feb 6, 2025 · 7 comments · May be fixed by #993
Labels
question Issue can be closed by providing information

Comments

@paulharris
Copy link
Contributor

If I GET a url such as:
https://myserver.com/imgs/image with space.png?param=one two
the URL seen by the server is:
https://myserver.com/imgs/image%20with%20space.png?param=one%20two

The query parameters are decoded correctly, into:
param --> one two

However, the URL is still imgs/image%20with%20space.png

Shouldn't crowcpp automatically decode the URL path before it is passed to a route handler?

ie I would expect my handler to see imgs/image with space.png
Or are we expected to manually decode this?
@gittiver gittiver added the question Issue can be closed by providing information label Feb 6, 2025
@gittiver
Copy link
Member

gittiver commented Feb 6, 2025

Crow currently does not decode the URL path.
Such a decoder could be added, but only makes sense in a few cases and adds additional runtime to request handling.
Therefore there is no plan to do the URL path decoding in crow.

@gittiver gittiver closed this as completed Feb 6, 2025
@paulharris
Copy link
Contributor Author

Ok, I'm fine with this, however, is this typical for webservers? It seems like surprising behaviour.

Can we document this?
And add a nice obviously named function?

My route was coded something like:

CROW_ROUTE(app, "/<string>/<path>")
([]( const crow::request& req, crow::response& res, const string_view url_leaf, string url_path_partial )
{
   // decode URL path
   url_path_partial.resize( crow::qs_decode(&url_path_partial[0]) );
}

IF this is the correct approach, then it would be nice to have a function that accepts a string and in-place decodes it.

What do you think?

@gittiver
Copy link
Member

gittiver commented Feb 6, 2025

Yo are welcome to add the function and the documentation.

@paulharris paulharris mentioned this issue Feb 7, 2025
Open
@paulharris
Copy link
Contributor Author

I've pushed a PR with test case. Was trying codespaces, is there a way to run the unit tests in there?

@gittiver
Copy link
Member

gittiver commented Feb 7, 2025

https://github.com/CrowCpp/Crow/pull/993/checks
they run now, I had to approve.

@paulharris
Copy link
Contributor Author

I've forced-pushed. Do you have to re-approve each time?

@gittiver
Copy link
Member

gittiver commented Feb 7, 2025

It seems. :)

@gittiver gittiver linked a pull request Feb 10, 2025 that will close this issue
Add crow::url_decode() function for decoding from percent-encoding. #993
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question Issue can be closed by providing information
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add crow::url_decode() function for decoding from percent-encoding. paulharris/Crow
2 participants
@gittiver @paulharris