Opened 10 months ago

Last modified 7 months ago

#30018 new Cleanup/optimization

Regression for selenium tests & inaccurate Content-Length

Reported by: Xavier Fernandez Owned by: nobody
Component: HTTP handling Version: 2.1
Severity: Normal Keywords:
Cc: Florian Apolloner, Pascal Wichmann Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Not sure it qualifies as a bug/regression but:

diff --git a/tests/view_tests/tests/test_specials.py b/tests/view_tests/tests/test_specials.py
index 70ffb1d23e..873e65a478 100644
--- a/tests/view_tests/tests/test_specials.py
+++ b/tests/view_tests/tests/test_specials.py
@@ -1,4 +1,5 @@
 from django.test import SimpleTestCase, override_settings
+from django.test.selenium import SeleniumTestCase
 
 
 @override_settings(ROOT_URLCONF='view_tests.generic_urls')
@@ -21,3 +22,12 @@ class URLHandling(SimpleTestCase):
         """
         response = self.client.get('/permanent_nonascii_redirect/')
         self.assertRedirects(response, self.redirect_target, status_code=301)
+
+
+@override_settings(ROOT_URLCONF='view_tests.urls')
+class JSSeleniumTests(SeleniumTestCase):
+
+    available_apps = ['view_tests']
+
+    def test_wrong_length(self):
+        self.selenium.get(self.live_server_url + '/error_view/')
diff --git a/tests/view_tests/urls.py b/tests/view_tests/urls.py
index c487dd7cb9..974a59ff7c 100644
--- a/tests/view_tests/urls.py
+++ b/tests/view_tests/urls.py
@@ -31,6 +31,9 @@ urlpatterns = [
     url(r'technical404/$', views.technical404, name="my404"),
     url(r'classbased404/$', views.Http404View.as_view()),
 
+    url(r'error_view/$', views.error_view),
+    url(r'wrong_length/$', views.wrong_length),
+
     # i18n views
     url(r'^i18n/', include('django.conf.urls.i18n')),
     url(r'^jsi18n/$', i18n.JavaScriptCatalog.as_view(packages=['view_tests'])),
diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py
index ce0079a355..fe0b397a00 100644
--- a/tests/view_tests/views.py
+++ b/tests/view_tests/views.py
@@ -22,6 +22,16 @@ def index_page(request):
     return HttpResponse('<html><body>Dummy page</body></html>')
 
 
+def wrong_length(request):
+    response = HttpResponse('')
+    response["Content-Length"] = 42
+    return response
+
+
+def error_view(request):
+    return HttpResponse('<html><body><img src="/wrong_length/"></body></html>')
+
+
 def with_parameter(request, parameter):
     return HttpResponse('ok')

did pass in 2.1.3 and now hangs on 2.1.4 due to https://github.com/django/django/commit/e1721ece485b35ab5543f134203a8a8ce9f31a7c

I acknowledge it might be a case of "Don't do that" but in my case, wrong_length also returns a X-Accel-Redirect header for nginx, hence the empty response with a non-null Content-Length.

Change History (8)

comment:1 Changed 10 months ago by Simon Charette

Hello Xavier,

Wouldn't NGINX take care off adding Content-Lenght when dealing with X-Accel-Redirect? I feel like the WSGI layer should be expected to block if provided a content smaller than Content-Lenght.

Any thoughts Florian?

Last edited 10 months ago by Simon Charette (previous) (diff)

comment:2 Changed 10 months ago by Simon Charette

Cc: Florian Apolloner added
Component: Testing frameworkHTTP handling
Type: UncategorizedBug

comment:3 Changed 10 months ago by Xavier Fernandez

Hello Simon,

Wouldn't NGINX take care off adding Content-Lenght when dealing with X-Accel-Redirect?

From my tests, it does and I've taken care of this issue by avoiding it: I removed this header from the response for GET requests (but kept it for HEAD requests).

But indeed, the WSGI layer could maybe warn when seeing inconsistent responses.

I'm also fine with closing this issue since it is clearly a very specific (and bad) case.

comment:4 Changed 10 months ago by Simon Charette

I'm fine with either resolutions as well but I'll let Florian chime in as he has a bit more context here.

comment:5 Changed 10 months ago by Florian Apolloner

I think this behavior is to be expected with keep-alive support. What we could do (if it is easy to implement) is to close the connection if we detect such a situation (Literally close the socket, not just a "Connection: close" header, because the client would still wait for the bytes to come).

comment:6 Changed 10 months ago by Carlton Gibson

Type: BugCleanup/optimization

Let's accept this on the "if it is easy to implement" basis. Otherwise we can wontfix.

comment:7 Changed 10 months ago by Carlton Gibson

Triage Stage: UnreviewedAccepted

comment:8 Changed 7 months ago by Pascal Wichmann

Cc: Pascal Wichmann added
Note: See TracTickets for help on using tickets.
Back to Top