Opened 6 years ago

Last modified 6 years 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 by Simon Charette, 6 years ago

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 6 years ago by Simon Charette (previous) (diff)

comment:2 by Simon Charette, 6 years ago

Cc: Florian Apolloner added
Component: Testing framework → HTTP handling
Type: Uncategorized → Bug

comment:3 by Xavier Fernandez, 6 years ago

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 by Simon Charette, 6 years ago

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 by Florian Apolloner, 6 years ago

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 by Carlton Gibson, 6 years ago

Type: Bug → Cleanup/optimization

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

comment:7 by Carlton Gibson, 6 years ago

Triage Stage: Unreviewed → Accepted

comment:8 by Pascal Wichmann, 6 years ago

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