Opened 5 years ago

Last modified 5 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 Changed 5 years 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 5 years ago by Simon Charette (previous) (diff)

comment:2 Changed 5 years ago by Simon Charette

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

comment:3 Changed 5 years 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 5 years 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 5 years 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 5 years ago by Carlton Gibson

Type: Bug → Cleanup/optimization

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

comment:7 Changed 5 years ago by Carlton Gibson

Triage Stage: Unreviewed → Accepted

comment:8 Changed 5 years ago by Pascal Wichmann

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