amol- / depot

Toolkit for storing files and attachments in web applications
MIT License
161 stars 41 forks source link

Middleware mountpoint break routing in some case #54

Closed slav0nic closed 5 years ago

slav0nic commented 5 years ago

Midleware mountpoint really check only first part of url. For example if i have something like

app = DepotManager.make_middleware(app, mountpoint='/storage', replace_wsgi_filewrapper=True)

It will also serve files by url like /storage123/default/... not only /storage/. If i set mountpoint to /storage/ url property https://github.com/amol-/depot/blob/30b90af4190a8e31c263d58d9f8c13d8581fe5f0/depot/fields/upload.py#L61 return wrong url with double /

amol- commented 5 years ago

Thanks for spotting this, for performance reasons (depot tries to be as quick as possible at deciding it should forward the request to the app) the check on path is very naive and is just a startswith. Which leads to the issue you just mentioned that /storage123 is served too when /storage is the mountpoint.

I think this can easily be fixed by appending a / in the check itself ( https://github.com/amol-/depot/blob/master/depot/middleware.py#L180 )

amol- commented 5 years ago

https://github.com/amol-/depot/commit/a4520a96c575e2a106dd4541db8b78ff9a62a8b8 should fix this