r47313 - more cleanups
hawkowl-TA+aISz0psMTMxyoc4vAAJOcrHinNvQL0E9HWUfgJXw@public.gmane.org Sun, 24 Apr 2016 07:07:10 -0600 (MDT)
| Newsgroups | gmane.comp.python.twisted.commits |
|---|---|
| Message-ID | <[email protected]> |
Author: hawkowl
Date: Sun Apr 24 07:07:04 2016
New Revision: 47313
Modified:
branches/proper-upgrade-8301-2/twisted/web/http.py
branches/proper-upgrade-8301-2/twisted/web/test/test_http.py
branches/proper-upgrade-8301-2/twisted/web/test/test_web.py
Log:
more cleanups
Modified: branches/proper-upgrade-8301-2/twisted/web/http.py
==============================================================================
--- branches/proper-upgrade-8301-2/twisted/web/http.py (original)
+++ branches/proper-upgrade-8301-2/twisted/web/http.py Sun Apr 24 07:07:04 2016
@@ -91,7 +91,7 @@
# twisted imports
from twisted import copyright
from twisted.python.compat import (
- _PY3, unicode, intToBytes, networkString, nativeString)
+ _PY3, unicode, intToBytes, networkString, nativeString, iterbytes)
from twisted.python.deprecate import deprecated
from twisted.python import log
from twisted.python.versions import Version
@@ -2083,48 +2083,46 @@
def _upgrade(self):
"""
- Look at the headers and determine if they want us to upgrade.
+ Look at the headers and determine if the client wants us to upgrade.
"""
content = self._buffer.split(b"\r\n\r\n", 1)[0].split(b"\r\n")
- headers = {}
+ headers = {}
requestLine = content[0].split(b" ")
+ # Should match "VERB PATH HTTP/VER"
if not len(requestLine) == 3:
_respondToBadRequestAndDisconnect(self._channel.transport)
return None
if requestLine[2].lower() == b"http/1.0":
- # It's HTTP/1.0, return that
- print("http1.0")
+ # It's HTTP/1.0, return that.
return b"http/1.0"
if not requestLine[2].lower() == b"http/1.1":
# If it's not HTTP/1.0 or HTTP/1.1, we don't really know what to
# do!
- print("not 1.1")
_respondToBadRequestAndDisconnect(self._channel.transport)
return None
+ # Get the header lines (that is, every line except the first, which is
+ # the request line
for line in content[1:]:
key, val = line.split(b":", 1)
- headers[key.lower()] = val.lstrip()
+ headers[key.lower()] = val.strip()
verb = requestLine[0]
path = requestLine[1]
- if b"connection" not in headers or b"upgrade" not in headers:
- # Regular HTTP/1.1, no "Upgrade" or "Connection" (we need both)
- return b"http/1.1"
-
- if not b"upgrade" in headers[b"connection"].lower().split(b", "):
- # "Connection" is there and "Upgrade" is there but "Connection"
- # does not say to upgrade
- print("no upgrade")
+ if b"upgrade" not in headers:
+ # Regular HTTP/1.1, no "Upgrade"
return b"http/1.1"
- for upgrade in headers[b"upgrade"].split(b", "):
- # See if we can upgrade to this. If we can't, try the next,
+ for upgrade in headers[b"upgrade"].split(b","):
+ upgrade = upgrade.strip()
+ # See if we can upgrade to this. If we can't, try the next one, and
+ # so on, until we either get one we can upgrade to, or we just
+ # don't upgrade the connection.
upgrader = self.factory._upgradeables.get(upgrade.lower())
if upgrader:
@@ -2148,21 +2146,23 @@
def _bufferData(self, data):
"""
- Add data to the buffer.
+ Add data to the internal temporary buffer, and determine if we have
+ enough of it to see if the client wants to perform upgrade negotiation.
"""
- self._buffer += (data)
+ self._buffer += data
self._bufferLen += len(data)
if self._bufferLen > self._maxHeadersLength:
- _respondToBadRequestAndDisconnect(self._channel)
+ _respondToBadRequestAndDisconnect(self._channel.transport)
if b"\r\n\r\n" in self._buffer:
self._negotiatedProtocol = self._upgrade()
-
- if self._negotiatedProtocol == b"http/1.1":
- self._channel.dataReceived(self._buffer)
self._buffering = False
- self._buffer = []
+
+ if self._negotiatedProtocol in [b"http/1.1", b"http/1.0"]:
+ res = self.dataReceived(self._buffer)
+
+ del self._buffer
def dataReceived(self, data):
@@ -2170,28 +2170,32 @@
A override of L{IProtocol.dataReceived} that checks what protocol we're
using.
"""
- if not self._buffering and self._negotiatedProtocol is None:
+ if self._buffering:
+ return self._bufferData(data)
+
+ elif self._negotiatedProtocol is None:
try:
negotiatedProtocol = self._channel.transport.negotiatedProtocol
except AttributeError:
+ # The transport didn't negotiate the protocol (e.g. it's
+ # plaintext non-ALPN), so we should investigate the content.
self._buffering = True
return self._bufferData(data)
- if negotiatedProtocol is None:
- negotiatedProtocol = b'http/1.1'
-
if negotiatedProtocol == b'h2':
- assert H2_ENABLED, "Cannot negotiate HTTP/2 without support."
+ return _respondToBadRequestAndDisconnect(
+ self._channel.transport)
+ elif negotiatedProtocol in [b"http/1.1", None]:
+ # If it's HTTP/1.1 (which may be an upgrade) or we don't know
+ # yet, look at the request.
+ self._buffering = True
+ return self._bufferData(data)
else:
- # Only HTTP/2 and HTTP/1.1 are supported right now.
- assert negotiatedProtocol == b'http/1.1', \
- "Unsupported protocol negotiated"
+ return _respondToBadRequestAndDisconnect(
+ self._channel.transport)
self._negotiatedProtocol = negotiatedProtocol
- elif self._buffering:
- self._bufferData(data)
-
else:
return self._channel.dataReceived(data)
Modified: branches/proper-upgrade-8301-2/twisted/web/test/test_http.py
==============================================================================
--- branches/proper-upgrade-8301-2/twisted/web/test/test_http.py (original)
+++ branches/proper-upgrade-8301-2/twisted/web/test/test_http.py Sun Apr 24 07:07:04 2016
@@ -265,7 +265,8 @@
a.makeConnection(t)
# one byte at a time, to stress it.
for byte in iterbytes(self.requests):
- a.dataReceived(byte)
+ if not t.disconnecting:
+ a.dataReceived(byte)
a.connectionLost(IOError("all done"))
return a._negotiatedProtocol
@@ -311,11 +312,9 @@
"""
b = StringTransport()
b.negotiatedProtocol = b'h2'
- self.assertRaises(
- AssertionError,
- self._negotiatedProtocolForTransportInstance,
- b,
- )
+ negotiatedProtocol = self._negotiatedProtocolForTransportInstance(b)
+ self.assertEqual(negotiatedProtocol, None)
+ self.assertEqual(b.value(), b"HTTP/1.1 400 Bad Request\r\n\r\n")
def test_unknownProtocol(self):
@@ -325,11 +324,9 @@
"""
b = StringTransport()
b.negotiatedProtocol = b'smtp'
- self.assertRaises(
- AssertionError,
- self._negotiatedProtocolForTransportInstance,
- b,
- )
+ negotiatedProtocol = self._negotiatedProtocolForTransportInstance(b)
+ self.assertEqual(negotiatedProtocol, None)
+ self.assertEqual(b.value(), b"HTTP/1.1 400 Bad Request\r\n\r\n")
def test_factory(self):
@@ -2539,6 +2536,32 @@
sub(["category", "message"], warnings[0]))
+class Pitocol(Protocol):
+ """
+ A protocol that writes pi to the transport.
+ """
+ def dataReceived(protoself, data):
+ """
+ A C{dataReceived} that expects "GO" and will then write out
+ "3.14" * C{piTimes}. If there's any other data, that won't
+ """
+ if not protoself.connected:
+ self.fail("dataReceived called when disconnected!")
+ if data == b"GO":
+ for i in range(piTimes):
+ protoself.transport.write(b"3.14")
+ protoself.transport.loseConnection()
+
+
+
+
+piTimes = 100
+piFactory = Factory()
+piFactory.protocol = Pitocol
+piFactory.startFactory()
+
+
+
class HTTPUpgradeTests(unittest.TestCase):
"""
Tests for HTTP/1.1 protocol upgrade.
@@ -2561,40 +2584,62 @@
and an "Upgrade" header that lists a protocol we support will be
upgraded to that protocol.
"""
- piTimes = 10
- class Pitocol(Protocol):
- """
- A protocol that writes pi to the transport.
- """
- def dataReceived(protoself, data):
- """
- A C{dataReceived} that expects "GO" and will then write out
- "3.14" * C{piTimes}. If there's any other data, that won't
- """
- if not protoself.connected:
- self.fail("dataReceived called when disconnected!")
- if data == b"GO":
- for i in range(piTimes):
- protoself.transport.write(b"3.14")
- protoself.transport.loseConnection()
+ class PiUpgrader(object):
- piFactory = Factory()
- piFactory.protocol = Pitocol
+ def upgrade(self, verb, path, headers):
+ pi = piFactory.buildProtocol(None)
+ return pi, False, {b"beep": b"boop"}
+
+ factory = self._makeFactory()
+ factory._addUpgrader(b"pitocol", PiUpgrader())
+
+ protocol = factory.buildProtocol(None)
+
+ trans = StringTransport()
+ protocol.makeConnection(trans)
+ val = [
+ b"GET / HTTP/1.1\r\n"
+ b"Connection: keep-alive, Upgrade\r\n",
+ b"Upgrade: pitocol\r\n\r\n",
+ ]
+
+ for x in iterbytes(b"".join(val)):
+ protocol.dataReceived(x)
+
+ expectedValue = b"".join([
+ b"HTTP/1.1 101 Switching Protocols\r\nServer: ",
+ _version, b"\r\nUpgrade: pitocol\r\nConnection: Upgrade\r\n",
+ b"beep: boop\r\n\r\n"])
+
+ self.assertEqual(trans.value(), expectedValue)
+ trans.clear()
+
+ protocol.dataReceived(b"GO")
+
+ self.assertEqual(trans.value(), b"3.14" * piTimes)
+ self.assertTrue(trans.disconnecting)
+
+
+ def test_upgradeNegotiatedHTTP11(self):
+ """
+ A ALPN-negotiated HTTP/1.1 protocol needs to be read and checked for
+ any upgrade requests.
+ """
class PiUpgrader(object):
def upgrade(self, verb, path, headers):
pi = piFactory.buildProtocol(None)
return pi, False, {b"beep": b"boop"}
-
factory = self._makeFactory()
factory._addUpgrader(b"pitocol", PiUpgrader())
protocol = factory.buildProtocol(None)
trans = StringTransport()
+ trans.negotiatedProtocol = b'http/1.1'
protocol.makeConnection(trans)
val = [
@@ -2616,13 +2661,13 @@
protocol.dataReceived(b"GO")
- self.assertEqual(trans.value(), b"3.14" * 10)
+ self.assertEqual(trans.value(), b"3.14" * piTimes)
self.assertTrue(trans.disconnecting)
def test_notHTTP(self):
"""
- A non-HTTP request should return with a "bad request" error.
+ A non-HTTP request returns with a "bad request" error.
"""
factory = self._makeFactory()
protocol = factory.buildProtocol(None)
@@ -2641,6 +2686,31 @@
self.assertEqual(trans.value(), expectedValue)
+ def test_tooMuchData(self):
+ """
+ Too much data being sent before completed headers will cause a "bad
+ request" error.
+ """
+ factory = self._makeFactory()
+ protocol = factory.buildProtocol(None)
+
+ trans = StringTransport()
+ protocol.makeConnection(trans)
+
+ val = [
+ b"GET / HTTP/1.1\r\n",
+ b"a" * http._MAX_HEADERS_SIZE
+ ]
+
+ for x in iterbytes(b"".join(val)):
+ if not trans.disconnecting:
+ protocol.dataReceived(x)
+
+ expectedValue = b"HTTP/1.1 400 Bad Request\r\n\r\n"
+ self.assertEqual(trans.value(), expectedValue)
+ self.assertEqual(trans.disconnecting, True)
+
+
def test_mangledStatusLine(self):
"""
A non-HTTP first-line (or a mangled one) will return with a
Modified: branches/proper-upgrade-8301-2/twisted/web/test/test_web.py
==============================================================================
--- branches/proper-upgrade-8301-2/twisted/web/test/test_web.py (original)
+++ branches/proper-upgrade-8301-2/twisted/web/test/test_web.py Sun Apr 24 07:07:04 2016
@@ -298,7 +298,7 @@
self.channel = self.site.buildProtocol(None)
self.transport = http.StringTransport()
self.transport.close = lambda *a, **kw: None
- self.transport.disconnecting = lambda *a, **kw: 0
+ self.transport.disconnecting = 0
self.transport.getPeer = lambda *a, **kw: "peer"
self.transport.getHost = lambda *a, **kw: "host"
self.channel.makeConnection(self.transport)
@@ -319,7 +319,7 @@
validator = b"If-Modified-Since: " + modifiedSince
else:
validator = b"If-Not-Match: " + etag
- for line in [b"GET / HTTP/1.1", validator, b""]:
+ for line in [b"GET / HTTP/1.1", validator, b"\r\n"]:
self.channel.dataReceived(line + b'\r\n')
result = self.transport.getvalue()
self.assertEqual(httpCode(result), http.OK)