Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#4515 closed (fixed)

django.views.static.serve() redirects (HTTP code 302) to the wrong location when the path to the static file is "normalized"

Reported by: Chris Wagner <cw264701@…> Owned by: nobody
Component: Uncategorized Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

For instance, if I have a URL hook for static files set up at /collection/, and I enter a URL like /collection//path/to/file.mp3, django.views.static.serve() will normalize the /path/to/file.mp3 path down to path/to/file.mp3 and return an HttpResponseRedirect with the redirect path set to the normalized path path/to/file.mp3; that is, it's missing the /collection/ prefix that needs to be passed back to the Web browser.

Unfortunately, I see no clean way to fix this. The most obvious solution would require passing the URL prefix (/collection/, in this case) to the django.views.static.serve() function. That would be pretty ugly. It might be best to disable the redirect feature (?).

For whatever reason, when using Firefox (I didn't try any other browsers), this leads to some odd loop, which causes Django to send 302 forward messages with continuously longer paths (e.g., /collection//path/to/path/to/path/to/...). ...Actually, I think I just figured out why: It's because Firefox is trying to do the forward, and it tacks on the path/to/file.mp3 relative to the current location, so you end up with path/to/path/to/file.mp3; then, Django normalizes that again and sends another 302. Actually, this problem of infinite redirections (it eventually stops, because of some limit, but anyway) may only occur when you're dealing with filenames that have spaces; I'm not sure, but it may be the case because the spaces will be %20's in the original path and then normalized to spaces... Yeesh!

Oh, and, I believe this is the problem that the folks on #1291 were seeing. I opened a new ticket because I thought that'd be easier to understand.

By the way, I'm not sure but, I believe I read, somewhere, that HTTP Location headers should always contain a full URL, rather than a simple path. So, instead of saying Location: /path/to, Django should say Location: http://appserver/path/to. It doesn't seem to include the full URL, judging from my Firefox Live HTTP Headers extension; I see something like Location: path/to (exactly what is specified in the creation of the HttpResponseRedirect object, I believe). Maybe I should file another bug about this (?).

Change History (7)

comment:1 Changed 8 years ago by Chris Wagner <cw264701@…>

Oh -- I don't think the spaces (i.e., %20's) have to do with the looping. If you look at the code...

    path = posixpath.normpath(urllib.unquote(path))
    newpath = ''
    # ... (do the normalization stuff)
    if newpath and path != newpath:
        return HttpResponseRedirect(newpath)

you can see that the posixpath.normpath() and urllib.unquote() calls do not affect the path/newpath comparison.

comment:2 Changed 8 years ago by mtredinnick

  • Resolution set to duplicate
  • Status changed from new to closed

This gets fixed when #987 gets fixed properly (using absolute URLs in HttpRedirect calls). It's really another manifestation of that problem.

Fixing #987 is on my list to do "soon-ish".

comment:3 Changed 8 years ago by Chris Wagner <cw264701@…>

I'm not sure, but I don't believe that this problem will be fixed when #987 is fixed...

The problem, here, is not simply that the redirect is missing a scheme and server prefix (e.g., http://server-name), but that it's actually missing a portion of the path prefix -- that is, it's missing something like http://server-name/path/to/static-stuff/. I'm guessing that, once #987 is resolved, it will attempt to do a redirect to http://server-name/sub-dir/my-static-file.txt (as an example) in a case when it should actually redirect to http://server-name/path/to/static-stuff/sub-dir/my-static-file.txt. I.e., the bug is not merely a concern with the way Django handles HttpResponseRedirect in general; it's a problem that is also specific to the way this particular HttpResponseRedirect is used.

comment:4 follow-up: Changed 8 years ago by mtredinnick

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Possibly correct. If you don't think it's a dupe, not reopening the ticket won't ever get it fixed, so let's fix that.

comment:5 in reply to: ↑ 4 Changed 8 years ago by Chris Wagner <cw264701@…>

Replying to mtredinnick:

If you don't think it's a dupe, not reopening the ticket won't ever get it fixed, so let's fix that.

Didn't want to be rude. ;)

comment:6 Changed 7 years ago by anonymous

  • Resolution set to fixed
  • Status changed from reopened to closed

Hi Chris, do you want to test this out again? From your description, I think that #987 actually may have fixed this - it not only adds the server prefix, but also adds the path prefix if a relative URL is passed.

Feel free to reopen again if you can verify this - we won't think it's rude ;)

comment:7 Changed 7 years ago by Chris Wagner <cw264701@…>

It seems to throw an IOError ("No such file or directory"), now, which makes sense. I'm satisfied; thanks. :)

Note: See TracTickets for help on using tickets.
Back to Top