Opened 11 years ago

Closed 10 years ago

#21451 closed Bug (fixed)

LiveServerTestCase returns a 404 on all URLs when MEDIA_URL = '' and DEBUG = True

Reported by: Aymeric Augustin Owned by: Greg Chapple
Component: Testing framework Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is a fairly obscure bug and it was interesting to diagnose ;-)

I ran into it while working on the Django Debug Toolbar. Since the DDT isn't displayed when DEBUG = False, its tests run with DEBUG = True. I admit that this isn't a common use case. (Of course, I could change the function that controls whether the toolbar is shown to display it during tests regardless of DEBUG. But this isn't the point.) Since the toolbar doesn't handle any uploaded files, MEDIA_URL isn't set and defaults to the empty string.

In these circumstances, _MediaFilesHandler.should_handle(path) will always return True because any path starts with the empty string. Then _MediaFilesHandler.get_response(request) will fail to find the media file corresponding to path and catch the resulting Http404. When DEBUG = False, get_response will continue after the try/except block and call the parent get_response, which will return the expected result. But when DEBUG = True, get_response will happily return the 404 debug page! NB: all the code I've described is actually in StaticFilesHandler, which _MediaFilesHandler inherits.

I believe there are actually two bugs here:

  • _MediaFilesHandler shouldn't be applied when MEDIA_URL isn't set — and while we're there, StaticFilesHandler shouldn't be applied either when STATIC_URL isn't set.
  • get_response shouldn't swallow Http404 exceptions when DEBUG = False — this behavior has hidden the previous problem until now.

Attachments (1)

21451-patch-Django-1.6.diff (2.4 KB ) - added by Aymeric Augustin 11 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 by Aymeric Augustin, 11 years ago

I'm attaching a patch against Django 1.6 with three changes:

  • a regression test
  • a change in StaticFilesHandler.get_response that makes the test fail when DEBUG = False. I tested by commenting out the settings override.
  • a change in LiveServerThread.run that makes the test pass regardless of the value of DEBUG.

I didn't spend too much time on this and I'm not sure this is the best solution.

by Aymeric Augustin, 11 years ago

Attachment: 21451-patch-Django-1.6.diff added

comment:2 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted

comment:3 by hatem@…, 11 years ago

I have just ran into this issue.

comment:4 by Greg Chapple, 10 years ago

Has patch: set
Owner: changed from nobody to Greg Chapple
Status: newassigned

I submitted this patch in https://github.com/django/django/pull/2759 - and just added a couple of small comments.

I did some testing around not applying StaticFilesHandler when STATIC_URL is not set. I don't think this is actually possible however, as LiveServerTestCase in Django 1.6 used the staticfiles app, which in turn required the STATIC_URL setting to be set.

comment:5 by Tim Graham, 10 years ago

Resolution: fixed
Status: assignedclosed

On the PR, Greg noted that this was fixed in 1.7 as part of the removal the static files dependencies in LiveServerTestCase [e909ceae].

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