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

Allow publisher_signature to have embedded path components (a prefix) #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Preston-Landers
Copy link

Hello, I'm trying to support a global URL prefix for my app, so that all URLs have a prefix like /myapp if needed.

If the prefix is enabled, I'm setting the publisher signature to something like: myapp/bowerstatic
If there is no prefix enabled the signature is just bowerstatic.

So when I do this, my URLs to bower resources are getting generated correctly, but the publisher tween fails to recognize and serve them. The problem appears to be that in publisher.py at line 22 it calls request.path_info_peek. This only returns the first component of the URL, so it only returns myapp when my prefix is enabled. This fails to match the publisher signature of myapp/bowerstatic so it fails to serve the file.

It would be nice if it could recognize a multi-part signature is in use and match the whole thing. This PR allows the publisher signature to have embedded slashes (path components).

My app is a mixture of Pyramid and a custom/legacy framework, but in theory that shouldn't matter here.

@faassen
Copy link
Owner

faassen commented Jul 21, 2015

I'm happy to accept this pull request but it needs some tests. I'm especially curious about the [1:] in the code -- does this have something to do with an initial slash? It'd be good if the tests demonstrated the multiple cases.

@Preston-Landers
Copy link
Author

Yes, it was getting rid of the initial slash. I'm pretty sure there will always be a slash there but I can verify that or add some conditional logic.

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