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

static_url_path doesn't work if it ends with a slash #3134

Closed
qwdm opened this issue Mar 29, 2019 · 9 comments

Comments

@qwdm
Copy link

commented Mar 29, 2019

self.static_url_path + '/<path:filename>',

Urljoin seems like more robust way to join urls. I just tried to set static_url_path = '/' and it failed, but static_url_path = '' succeed. IMHO, this is bad and unexpected behaviour.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

I'm pretty sure urljoin always throws away existing path segments on the first arg. .rstrip('/') is probably better here

@davidism

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

Aside, making the static url path the root is probably going to give you unexpected behavior.

@davidism davidism changed the title Why don't use urljoin here? static_url_path doesn't work if it ends with a slash Mar 29, 2019

@qil026

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

i will be taking a look at this issue

@jab

This comment has been minimized.

Copy link
Member

commented May 19, 2019

@davidism wrote:

Aside, making the static url path the root is probably going to give you unexpected behavior.

Could you say more (and should that be documented)? I’ve seen a number of Flask users do this (to serve a SPA that assumes it’s mounted under /) who haven’t yet seen unexpected behavior.

@davidism

This comment has been minimized.

Copy link
Member

commented May 19, 2019

The URL rule is /{static_url_path}/<path:filename>. When static_url_path is empty, it becomes /<path:filename>, which is a catch-all route. Some route sorting that Werkzeug does may alleviate this, but you're just asking for trouble.

There are better ways of serving SPAs than passing it through Flask. In development, SPAs have their own dev server that should be used, and either they or the WSGI app would set up a proxy to keep everything under one domain (Werkzeug has proxy middleware now). In production, Nginx would be configured to serve the static files and leave Flask to do backend things.

@jab

This comment has been minimized.

Copy link
Member

commented May 22, 2019

Yes, we have it set up so SPA dev servers work in development. And in production we use gunicorn with Whitenoise to serve static files, running behind Waiter.

Since the /<path:filename> static route has been working well for us (perhaps thanks to Werkzeug's route sorting), I'm wondering if there's a concrete example of where this wouldn't work well.

For example, we know that any route /foo we define precludes serving a static file named foo from the static folder, and haven't had any problems with that. (~All the API routes we define end up under a prefix like /api/ thanks to Flask-Rebar, which helps reduce potential for conflict.)

But eager to know if there are any compelling examples of nasty cases I'm missing!

@davidism

This comment has been minimized.

Copy link
Member

commented May 22, 2019

It sounds like you've only been generating the urls, not using Flask to actually serve the files. If you're using a proxy in dev and whitenoise in prod, routes that would be problematic aren't reaching Flask, so you won't see the issue.

@jab

This comment has been minimized.

Copy link
Member

commented May 22, 2019

Meant to add, this accepted StackOverflow answer and this post (which was referenced in #348 (comment)) recommend this, so if this recommendation actually comes with caveats it'd be good to have some docs about it – should I open a new issue?

@davidism

This comment has been minimized.

Copy link
Member

commented May 22, 2019

I don't actually have a reproducible example, just a general remembering of old issues and questions. I think there were some changes to Werkzeug that may have addressed it too, but it's been too long for me to remember. If people don't seem to be running into the issue, I wouldn't worry about it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.