Permalink
Browse files

Merge not-modified-not-content-type-4156

Author: exarkun
Reviewer: mbp, jesstess
Fixes: #4156

Change `twisted.web.server.Request` to not set a default Content-Type header in
the response when the response code is NOT MODIFIED.  Previously the always-set
text/html default might have been incorrect and confused the client (possibly a
cache).


git-svn-id: svn://svn.twistedmatrix.com/svn/Twisted/trunk@32443 bbbe8e31-12d6-0310-92fd-ac37d47ddeeb
  • Loading branch information...
1 parent a2538e6 commit 89e4033f48f705e65712f65c477595f430bca806 @exarkun exarkun committed Aug 9, 2011
Showing with 114 additions and 46 deletions.
  1. +21 −1 twisted/web/server.py
  2. +78 −34 twisted/web/test/test_web.py
  3. +1 −0 twisted/web/topfiles/4156.bugfix
  4. +8 −7 twisted/web/twcgi.py
  5. +6 −4 twisted/web/wsgi.py
View
@@ -53,9 +53,15 @@ def _addressToTuple(addr):
class Request(pb.Copyable, http.Request, components.Componentized):
"""
An HTTP request.
+
+ @ivar defaultContentType: A C{str} giving the default I{Content-Type} value
+ to send in responses if no other value is set. C{None} disables the
+ default.
"""
implements(iweb.IRequest)
+ defaultContentType = "text/html"
+
site = None
appRootURL = None
__pychecker__ = 'unusednames=issuer'
@@ -117,7 +123,6 @@ def process(self):
# set various default headers
self.setHeader('server', version)
self.setHeader('date', http.datetimeToString())
- self.setHeader('content-type', "text/html")
# Resource Identification
self.prepath = []
@@ -134,9 +139,24 @@ def write(self, data):
@param data: A string to write to the response.
"""
+ if not self.startedWriting:
+ # Before doing the first write, check to see if a default
+ # Content-Type header should be supplied.
+ modified = self.code != http.NOT_MODIFIED
+ contentType = self.responseHeaders.getRawHeaders('content-type')
+ if modified and contentType is None and self.defaultContentType is not None:
+ self.responseHeaders.setRawHeaders(
+ 'content-type', [self.defaultContentType])
+
+ # Only let the write happen if we're not generating a HEAD response by
+ # faking out the request method. Note, if we are doing that,
+ # startedWriting will never be true, and the above logic may run
+ # multiple times. It will only actually change the responseHeaders once
+ # though, so it's still okay.
if not self._inFakeHead:
http.Request.write(self, data)
+
def render(self, resrc):
"""
Ask a resource to render itself.
@@ -196,7 +196,21 @@ def testListEntities(self):
class SimpleResource(resource.Resource):
+ """
+ @ivar _contentType: C{None} or a C{str} giving the value of the
+ I{Content-Type} header in the response this resource will render. If it
+ is C{None}, no I{Content-Type} header will be set in the response.
+ """
+ def __init__(self, contentType=None):
+ resource.Resource.__init__(self)
+ self._contentType = contentType
+
+
def render(self, request):
+ if self._contentType is not None:
+ request.responseHeaders.setRawHeaders(
+ "content-type", [self._contentType])
+
if http.CACHED in (request.setLastModified(10),
request.setETag('MatchingTag')):
return ''
@@ -420,13 +434,10 @@ class ConditionalTest(unittest.TestCase):
"""
web.server's handling of conditional requests for cache validation.
"""
-
- # XXX: test web.distrib.
-
def setUp(self):
self.resrc = SimpleResource()
self.resrc.putChild('', self.resrc)
- self.site = server.Site(self.resrc)
+ self.resrc.putChild('with-content-type', SimpleResource('image/jpeg'))
self.site = server.Site(self.resrc)
self.site.logFile = log.logfile
@@ -438,25 +449,29 @@ def setUp(self):
self.transport.getPeer = lambda *a, **kw: "peer"
self.transport.getHost = lambda *a, **kw: "host"
self.channel.makeConnection(self.transport)
- for l in ["GET / HTTP/1.1",
- "Accept: text/html"]:
- self.channel.lineReceived(l)
+
def tearDown(self):
self.channel.connectionLost(None)
- def _modifiedTest(self, modifiedSince):
+ def _modifiedTest(self, modifiedSince=None, etag=None):
"""
- Given the value C{modifiedSince} for the I{If-Modified-Since}
- header, verify that a response with a 200 code and the resource as
- the body is returned.
+ Given the value C{modifiedSince} for the I{If-Modified-Since} header or
+ the value C{etag} for the I{If-Not-Match} header, verify that a response
+ with a 200 code, a default Content-Type, and the resource as the body is
+ returned.
"""
- self.channel.lineReceived("If-Modified-Since: " + modifiedSince)
- self.channel.lineReceived('')
+ if modifiedSince is not None:
+ validator = "If-Modified-Since: " + modifiedSince
+ else:
+ validator = "If-Not-Match: " + etag
+ for line in ["GET / HTTP/1.1", validator, ""]:
+ self.channel.lineReceived(line)
result = self.transport.getvalue()
self.assertEqual(httpCode(result), http.OK)
self.assertEqual(httpBody(result), "correct")
+ self.assertEqual(httpHeader(result, "Content-Type"), "text/html")
def test_modified(self):
@@ -466,22 +481,26 @@ def test_modified(self):
requested resource, a 200 response is returned along with a response
body containing the resource.
"""
- self._modifiedTest(http.datetimeToString(1))
+ self._modifiedTest(modifiedSince=http.datetimeToString(1))
def test_unmodified(self):
"""
- If a request is made with an I{If-Modified-Since} header value with
- a timestamp indicating a time after the last modification of the
- request resource, a 304 response is returned along with an empty
- response body.
+ If a request is made with an I{If-Modified-Since} header value with a
+ timestamp indicating a time after the last modification of the request
+ resource, a 304 response is returned along with an empty response body
+ and no Content-Type header if the application does not set one.
"""
- self.channel.lineReceived("If-Modified-Since: %s"
- % http.datetimeToString(100))
- self.channel.lineReceived('')
+ for line in ["GET / HTTP/1.1",
+ "If-Modified-Since: " + http.datetimeToString(100), ""]:
+ self.channel.lineReceived(line)
result = self.transport.getvalue()
self.assertEqual(httpCode(result), http.NOT_MODIFIED)
self.assertEqual(httpBody(result), "")
+ # Since there SHOULD NOT (RFC 2616, section 10.3.5) be any
+ # entity-headers, the Content-Type is not set if the application does
+ # not explicitly set it.
+ self.assertEqual(httpHeader(result, "Content-Type"), None)
def test_invalidTimestamp(self):
@@ -491,7 +510,7 @@ def test_invalidTimestamp(self):
and a normal 200 response is returned with a response body
containing the resource.
"""
- self._modifiedTest("like, maybe a week ago, I guess?")
+ self._modifiedTest(modifiedSince="like, maybe a week ago, I guess?")
def test_invalidTimestampYear(self):
@@ -501,7 +520,7 @@ def test_invalidTimestampYear(self):
header is treated as not having been present and a normal 200
response is returned with a response body containing the resource.
"""
- self._modifiedTest("Thu, 01 Jan blah 00:00:10 GMT")
+ self._modifiedTest(modifiedSince="Thu, 01 Jan blah 00:00:10 GMT")
def test_invalidTimestampTooLongAgo(self):
@@ -511,7 +530,7 @@ def test_invalidTimestampTooLongAgo(self):
having been present and a normal 200 response is returned with a
response body containing the resource.
"""
- self._modifiedTest("Thu, 01 Jan 1899 00:00:10 GMT")
+ self._modifiedTest(modifiedSince="Thu, 01 Jan 1899 00:00:10 GMT")
def test_invalidTimestampMonth(self):
@@ -522,27 +541,52 @@ def test_invalidTimestampMonth(self):
and a normal 200 response is returned with a response body
containing the resource.
"""
- self._modifiedTest("Thu, 01 Blah 1970 00:00:10 GMT")
+ self._modifiedTest(modifiedSince="Thu, 01 Blah 1970 00:00:10 GMT")
def test_etagMatchedNot(self):
- """If-None-Match ETag cache validator (positive)"""
- self.channel.lineReceived("If-None-Match: unmatchedTag")
- self.channel.lineReceived('')
- result = self.transport.getvalue()
- self.assertEqual(httpCode(result), http.OK)
- self.assertEqual(httpBody(result), "correct")
+ """
+ If a request is made with an I{If-None-Match} ETag which does not match
+ the current ETag of the requested resource, the header is treated as not
+ having been present and a normal 200 response is returned with a
+ response body containing the resource.
+ """
+ self._modifiedTest(etag="unmatchedTag")
+
def test_etagMatched(self):
- """If-None-Match ETag cache validator (negative)"""
- self.channel.lineReceived("If-None-Match: MatchingTag")
- self.channel.lineReceived('')
+ """
+ If a request is made with an I{If-None-Match} ETag which does match the
+ current ETag of the requested resource, a 304 response is returned along
+ with an empty response body.
+ """
+ for line in ["GET / HTTP/1.1", "If-None-Match: MatchingTag", ""]:
+ self.channel.lineReceived(line)
result = self.transport.getvalue()
self.assertEqual(httpHeader(result, "ETag"), "MatchingTag")
self.assertEqual(httpCode(result), http.NOT_MODIFIED)
self.assertEqual(httpBody(result), "")
+ def test_unmodifiedWithContentType(self):
+ """
+ Similar to L{test_etagMatched}, but the response should include a
+ I{Content-Type} header if the application explicitly sets one.
+
+ This I{Content-Type} header SHOULD NOT be present according to RFC 2616,
+ section 10.3.5. It will only be present if the application explicitly
+ sets it.
+ """
+ for line in ["GET /with-content-type HTTP/1.1",
+ "If-None-Match: MatchingTag", ""]:
+ self.channel.lineReceived(line)
+ result = self.transport.getvalue()
+ self.assertEqual(httpCode(result), http.NOT_MODIFIED)
+ self.assertEqual(httpBody(result), "")
+ self.assertEqual(httpHeader(result, "Content-Type"), "image/jpeg")
+
+
+
from twisted.web import google
class GoogleTestCase(unittest.TestCase):
@@ -0,0 +1 @@
+twisted.web.server now omits the default Content-Type header from NOT MODIFIED responses.
View
@@ -276,26 +276,27 @@ def outReceived(self, output):
text = self.headertext + output
headerEnds = []
for delimiter in '\n\n','\r\n\r\n','\r\r', '\n\r\n':
- headerend = string.find(text,delimiter)
+ headerend = text.find(delimiter)
if headerend != -1:
headerEnds.append((headerend, delimiter))
if headerEnds:
- # twisted.web.server.Request.process always addes a content-type
- # response header. That's not appropriate for us.
- self.request.responseHeaders.removeHeader('content-type')
+ # The script is entirely in control of response headers; disable the
+ # default Content-Type value normally provided by
+ # twisted.web.server.Request.
+ self.request.defaultContentType = None
headerEnds.sort()
headerend, delimiter = headerEnds[0]
self.headertext = text[:headerend]
# This is a final version of the header text.
linebreak = delimiter[:len(delimiter)/2]
- headers = string.split(self.headertext, linebreak)
+ headers = self.headertext.split(linebreak)
for header in headers:
- br = string.find(header,': ')
+ br = header.find(': ')
if br == -1:
log.msg( 'ignoring malformed CGI header: %s' % header )
else:
- headerName = string.lower(header[:br])
+ headerName = header[:br].lower()
headerText = header[br+2:]
if headerName == 'location':
self.request.setResponseCode(http.FOUND)
View
@@ -201,6 +201,12 @@ def __init__(self, reactor, threadpool, application, request):
'SERVER_PORT': str(request.getHost().port),
'SERVER_PROTOCOL': request.clientproto}
+
+ # The application object is entirely in control of response headers;
+ # disable the default Content-Type value normally provided by
+ # twisted.web.server.Request.
+ self.request.defaultContentType = None
+
for name, values in request.requestHeaders.getAllRawHeaders():
name = 'HTTP_' + name.upper().replace('-', '_')
# It might be preferable for http.HTTPChannel to clear out
@@ -283,10 +289,6 @@ def _sendResponseHeaders(self):
code = int(code)
self.request.setResponseCode(code, message)
- # twisted.web.server.Request.process always addes a content-type
- # response header. That's not appropriate for us.
- self.request.responseHeaders.removeHeader('content-type')
-
for name, value in self.headers:
# Don't allow the application to control these required headers.
if name.lower() not in ('server', 'date'):

0 comments on commit 89e4033

Please sign in to comment.