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

Unquote necessary url parts during routing #85

Merged
merged 3 commits into from
May 25, 2016

Conversation

bhearsum
Copy link
Contributor

I did some digging and AFAICT there's no way to make uwsgi unquote the path before handing them off to the app. This is different than Apache which seems to do that by default.

I was considering doing manual conversation of @, /, and : in the uwsgi config, but I thought this would be a more flexible approach.

The path unquoting should go away when #80 lands and removes URL-style permissions.

@bhearsum
Copy link
Contributor Author

From IRC, this isn't the right place to handle this. According to benoitc/gunicorn#1211 (comment), it seems like uwsgi should be able to do this. I couldn't find a way or any other confirmation that the wsgi sever is the right place to do unquoting, so I filed unbit/uwsgi#1263. If that doesn't work out, @catlee suggested that middleware would be a better place for this.

@bhearsum
Copy link
Contributor Author

I'm not sure why tc-gh isn't firing, but this latest version replaces the routing gunk with middleware to unquote the path. Tests all pass locally, and usernames/permissions aren't mangled when I add them locally.

@catlee
Copy link
Contributor

catlee commented May 25, 2016

Looks fine to me. Is there a risk of double decoding causing issues if this is run under apache or other servers that do the path decoding for us?

@bhearsum
Copy link
Contributor Author

I just verified this with a local apache - I can add permissions to users with @ in their name and they show up correctly. I tested both an "admin" permission and an URL-style one. I'll do some additional verification in dev, just in case my local config was too different.

@bhearsum bhearsum merged commit 34ce415 into mozilla-releng:master May 25, 2016
jcristau added a commit that referenced this pull request Jan 24, 2023
This was added in #85 to work around a bug in uwsgi, which has long
since been fixed (unbit/uwsgi#1267)
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