-
-
Notifications
You must be signed in to change notification settings - Fork 34.7k
gh-42550: Add 'Expect: 100-Continue' support to httplib #133276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
3c37a5e
532026d
7146d5b
d902673
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ The module provides the following classes: | |
|
|
||
|
|
||
| .. class:: HTTPConnection(host, port=None[, timeout], source_address=None, \ | ||
| blocksize=8192) | ||
| blocksize=8192, continue_timeout=2.5) | ||
|
|
||
| An :class:`HTTPConnection` instance represents one transaction with an HTTP | ||
| server. It should be instantiated by passing it a host and optional port | ||
|
|
@@ -46,7 +46,10 @@ The module provides the following classes: | |
| The optional *source_address* parameter may be a tuple of a (host, port) | ||
| to use as the source address the HTTP connection is made from. | ||
| The optional *blocksize* parameter sets the buffer size in bytes for | ||
| sending a file-like message body. | ||
| sending a file-like message body. Finally, the optional *continue_timeout* | ||
| parameter controls how long the connection will wait for a ``100 Continue`` | ||
| response from the server before sending the body, if the request included | ||
| an ``Expect: 100-Continue`` header. | ||
|
|
||
| For example, the following calls all create instances that connect to the server | ||
| at the same host and port:: | ||
|
|
@@ -66,6 +69,9 @@ The module provides the following classes: | |
| .. versionchanged:: 3.7 | ||
| *blocksize* parameter was added. | ||
|
|
||
| .. versionchanged:: next | ||
| *continue_timeout* parameter was added. | ||
|
|
||
|
|
||
| .. class:: HTTPSConnection(host, port=None, *[, timeout], \ | ||
| source_address=None, context=None, \ | ||
|
|
@@ -321,11 +327,24 @@ HTTPConnection Objects | |
| No attempt is made to determine the Content-Length for file | ||
| objects. | ||
|
|
||
| .. method:: HTTPConnection.getresponse() | ||
| .. versionchanged:: next | ||
| If the headers include ``Expect: 100-Continue`` and *body* is set, | ||
| the body will not be sent until either a ``100 Continue`` response is | ||
| received from the server or the :class:`HTTPConnection`'s *continue_timeout* | ||
| is reached; if a response code other than 100 is received, the body | ||
| will not be sent at all. | ||
|
|
||
| .. method:: HTTPConnection.getresponse(ignore_100_continue=True) | ||
|
|
||
| Should be called after a request is sent to get the response from the server. | ||
| Returns an :class:`HTTPResponse` instance. | ||
|
|
||
| By default, a server response of ``100 Continue`` will be ignored and the | ||
| call will not return until a response other than code 100 is received. | ||
| Setting *ignore_100_continue* to ``False`` will allow a 100 response to be | ||
| returned; you will then need to call :meth:`getresponse` a second time | ||
| (after transmitting the body) to get the final response. | ||
|
|
||
| .. note:: | ||
|
|
||
| Note that you must have read the whole response before you can send a new | ||
|
|
@@ -336,6 +355,10 @@ HTTPConnection Objects | |
| :class:`HTTPConnection` object will be ready to reconnect when | ||
| a new request is sent. | ||
|
|
||
| .. versionchanged:: next | ||
| Added the *ignore_100_continue* parameter. (In prior versions | ||
| a ``Continue`` response was always ignored.) | ||
|
|
||
|
|
||
| .. method:: HTTPConnection.set_debuglevel(level) | ||
|
|
||
|
|
@@ -439,11 +462,17 @@ also send your request step by step, by using the four functions below. | |
| an argument. | ||
|
|
||
|
|
||
| .. method:: HTTPConnection.endheaders(message_body=None, *, encode_chunked=False) | ||
| .. method:: HTTPConnection.endheaders(message_body=None, *, encode_chunked=False, \ | ||
| expect_continue=False) | ||
|
|
||
| Send a blank line to the server, signalling the end of the headers. The | ||
| optional *message_body* argument can be used to pass a message body | ||
| associated with the request. | ||
| associated with the request. If a body is provided, setting | ||
| *expect_continue* to ``True`` will wait for a ``100 Continue`` response | ||
| from the server before sending the body. (This should generally be | ||
| used only when an ``Expect: 100-Continue`` header has been sent.) If no | ||
| response is received within the :class:`HTTPConnection`'s *continue_timeout*, | ||
| the body will be sent regardless. | ||
|
|
||
| If *encode_chunked* is ``True``, the result of each iteration of | ||
| *message_body* will be chunk-encoded as specified in :rfc:`7230`, | ||
|
|
@@ -464,6 +493,9 @@ also send your request step by step, by using the four functions below. | |
| .. versionchanged:: 3.6 | ||
| Added chunked encoding support and the *encode_chunked* parameter. | ||
|
|
||
| .. versionadded:: next | ||
| The *expect_continue* parameter was added. | ||
|
|
||
|
|
||
| .. method:: HTTPConnection.send(data) | ||
|
|
||
|
|
@@ -642,6 +674,48 @@ method attribute. Here is an example session that uses the ``PUT`` method:: | |
| >>> print(response.status, response.reason) | ||
| 200, OK | ||
|
|
||
| Since version 3.15, conditional transmission of the body is supported when an | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't mention the version here. think of docs as being read in the future, version specific changes are normally footnotes via the version added and version changed markers. (this is a feature so it won't be in 3.15) |
||
| ``Expect: 100-Continue`` header is set. To use this in a simple case, just | ||
| set the header, and optionally the time for which the client should wait for | ||
| a ``100 Continue`` response before sending the body regardless:: | ||
|
|
||
| >>> import http.client | ||
| >>> BODY = "***filecontents***" | ||
| >>> conn = http.client.HTTPConnection("localhost", 8080, continue_timeout=1.0) | ||
| >>> conn.request("PUT", "/file", BODY, headers={'Expect': '100-Continue'}) | ||
| >>> response = conn.getresponse() | ||
| >>> # You will not see the '100' response, as it is handled internally | ||
| >>> print(response.status, response.reason) | ||
| 200, OK | ||
|
|
||
| Here is a more complex example in which we manually check the response and | ||
| decide whether to send the body. This may be useful if the body must be | ||
| generated by some resource-intensive process which should be skipped if the | ||
| server will not accept it. :: | ||
|
|
||
| >>> import http.client | ||
| >>> conn = http.client.HTTPConnection("localhost", 8080) | ||
| >>> conn.putrequest("PUT", "/file") | ||
| >>> conn.putheader('Expect', '100-Continue') | ||
| >>> # Assuming you know in advance what the length will be | ||
| >>> # If not, you will need to encode it as chunked | ||
| >>> conn.putheader('Content-Length', '42') | ||
| >>> conn.endheaders() | ||
| >>> response = conn.getresponse(ignore_100_continue=False) | ||
| >>> print(response.status, response.reason) | ||
| 100, Continue | ||
| >>> BODY = resource_intensive_calculation() | ||
| >>> conn.send(BODY) | ||
| >>> response = conn.getresponse() | ||
| >>> print(response.status, response.reason) | ||
| 200, OK | ||
|
|
||
| .. note:: | ||
|
|
||
| The *continue_timeout* setting does not apply when directly using | ||
| :meth:`HTTPConnection.getresponse`, so use the above example only if you are confident | ||
| that the server respects the ``Expect: 100-Continue`` header. | ||
|
|
||
| .. _httpmessage-objects: | ||
|
|
||
| HTTPMessage Objects | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,7 @@ | |
| import http | ||
| import io | ||
| import re | ||
| import select | ||
| import socket | ||
| import sys | ||
| import collections.abc | ||
|
|
@@ -320,15 +321,16 @@ def _read_status(self): | |
| raise BadStatusLine(line) | ||
| return version, status, reason | ||
|
|
||
| def begin(self): | ||
| def begin(self, ignore_100_continue=True): | ||
| if self.headers is not None: | ||
| # we've already started reading the response | ||
| return | ||
|
|
||
| # read until we get a non-100 response | ||
| # (unless caller has requested return of 100 responses) | ||
| while True: | ||
| version, status, reason = self._read_status() | ||
| if status != CONTINUE: | ||
| if not (ignore_100_continue and status == CONTINUE): | ||
| break | ||
| # skip the header from the 100 response | ||
| skipped_headers = _read_headers(self.fp) | ||
|
|
@@ -864,13 +866,15 @@ def _get_content_length(body, method): | |
| return None | ||
|
|
||
| def __init__(self, host, port=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, | ||
| source_address=None, blocksize=8192): | ||
| source_address=None, blocksize=8192, continue_timeout=2.5): | ||
| self.timeout = timeout | ||
| self.continue_timeout = continue_timeout | ||
| self.source_address = source_address | ||
| self.blocksize = blocksize | ||
| self.sock = None | ||
| self._buffer = [] | ||
| self.__response = None | ||
| self.__pending_response = None | ||
| self.__state = _CS_IDLE | ||
| self._method = None | ||
| self._tunnel_host = None | ||
|
|
@@ -882,9 +886,10 @@ def __init__(self, host, port=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, | |
|
|
||
| self._validate_host(self.host) | ||
|
|
||
| # This is stored as an instance variable to allow unit | ||
| # tests to replace it with a suitable mockup | ||
| # These are stored as instance variables to allow unit | ||
| # tests to replace them with a suitable mockup | ||
| self._create_connection = socket.create_connection | ||
| self._select = select.select | ||
|
|
||
| def set_tunnel(self, host, port=None, headers=None): | ||
| """Set up host and port for HTTP CONNECT tunnelling. | ||
|
|
@@ -1020,6 +1025,10 @@ def close(self): | |
| self.sock = None | ||
| sock.close() # close it manually... there may be other refs | ||
| finally: | ||
| pending_response = self.__pending_response | ||
| if pending_response: | ||
| self.__pending_response = None | ||
| pending_response.close() | ||
| response = self.__response | ||
| if response: | ||
| self.__response = None | ||
|
|
@@ -1080,7 +1089,8 @@ def _read_readable(self, readable): | |
| datablock = datablock.encode("iso-8859-1") | ||
| yield datablock | ||
|
|
||
| def _send_output(self, message_body=None, encode_chunked=False): | ||
| def _send_output(self, message_body=None, encode_chunked=False, | ||
| expect_continue=False): | ||
| """Send the currently buffered request and clear the buffer. | ||
|
|
||
| Appends an extra \\r\\n to the buffer. | ||
|
|
@@ -1092,6 +1102,23 @@ def _send_output(self, message_body=None, encode_chunked=False): | |
| self.send(msg) | ||
|
|
||
| if message_body is not None: | ||
| if expect_continue and not self.__response: | ||
| read_ready, _, _ = self._select([self.sock], [], [], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. select won't work properly on a HTTPSConnection. There's a .pending method on SSLSocket that could be used in this scenario if read_ready. Investigate what is plausible and add some tests covering the HTTPSConnection test case. |
||
| self.continue_timeout) | ||
| if read_ready: | ||
| if self.debuglevel > 0: | ||
| response = self.response_class(self.sock, | ||
| self.debuglevel, | ||
| method=self._method) | ||
| else: | ||
| response = self.response_class(self.sock, | ||
| method=self._method) | ||
| response.begin(ignore_100_continue=False) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _max_headers= should also be passed here. |
||
| if response.code != CONTINUE: | ||
| # Break without sending the body | ||
| self.__pending_response = response | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wherever self.__response is cleared (= None) such as putrequest, self.__pending_response needs to cleared as well. tests should cover that this happens by running through a scenario that'd lead to this such as maybe a non-100 early response that gets ignored before another request is sent on the same connection? |
||
| return | ||
| response.close() | ||
|
|
||
| # create a consistent interface to message_body | ||
| if hasattr(message_body, 'read'): | ||
|
|
@@ -1318,7 +1345,8 @@ def putheader(self, header, *values): | |
| header = header + b': ' + value | ||
| self._output(header) | ||
|
|
||
| def endheaders(self, message_body=None, *, encode_chunked=False): | ||
| def endheaders(self, message_body=None, *, encode_chunked=False, | ||
| expect_continue=False): | ||
| """Indicate that the last header line has been sent to the server. | ||
|
|
||
| This method sends the request to the server. The optional message_body | ||
|
|
@@ -1329,7 +1357,8 @@ def endheaders(self, message_body=None, *, encode_chunked=False): | |
| self.__state = _CS_REQ_SENT | ||
| else: | ||
| raise CannotSendHeader() | ||
| self._send_output(message_body, encode_chunked=encode_chunked) | ||
| self._send_output(message_body, encode_chunked=encode_chunked, | ||
| expect_continue=expect_continue) | ||
|
|
||
| def request(self, method, url, body=None, headers={}, *, | ||
| encode_chunked=False): | ||
|
|
@@ -1374,20 +1403,33 @@ def _send_request(self, method, url, body, headers, encode_chunked): | |
| else: | ||
| encode_chunked = False | ||
|
|
||
| # If the Expect: 100-continue header is set, we will try to honor it | ||
| # (if possible). We can only do so if 1) the request has a body, and | ||
| # 2) there is no current incomplete response (since we need to read | ||
| # the response stream to check if the code is 100 or not) | ||
| expect_continue = ( | ||
| body and not self.__response | ||
| and 'expect' in header_names | ||
| and '100-continue' in {v.lower() for k, v in headers.items() | ||
| if k.lower() == 'expect'} | ||
| ) | ||
|
|
||
| for hdr, value in headers.items(): | ||
| self.putheader(hdr, value) | ||
| if isinstance(body, str): | ||
| # RFC 2616 Section 3.7.1 says that text default has a | ||
| # default charset of iso-8859-1. | ||
| body = _encode(body, 'body') | ||
| self.endheaders(body, encode_chunked=encode_chunked) | ||
| self.endheaders(body, encode_chunked=encode_chunked, | ||
| expect_continue=expect_continue) | ||
|
|
||
| def getresponse(self): | ||
| def getresponse(self, ignore_100_continue=True): | ||
| """Get the response from the server. | ||
|
|
||
| If the HTTPConnection is in the correct state, returns an | ||
| instance of HTTPResponse or of whatever object is returned by | ||
| the response_class variable. | ||
| instance of HTTPResponse or of whatever object is returned by the | ||
| response_class variable. The connection will wait for a response other | ||
| than code 100 ('Continue') unless ignore_100_continue is set to False. | ||
|
Comment on lines
+1452
to
+1458
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "ignore..." is a somewhat of a negative term in English for use in an API which leads to awkward descriptions of the behavior. it defaults to True and people have to turn the negative off with False leading to double negatives to reason about. As this defaults to not supporting 100 continue behavior, why not call it (apply this everywhere) also we should make it keyword only so that it has to be spelled out when anyone wants to use it for better code readability. ( I also think eliding the number from the name is a good idea. 100 is defined as continue so no need to mention that internal http detail. |
||
|
|
||
| If a request has not been sent or if a previous response has | ||
| not be handled, ResponseNotReady is raised. If the HTTP | ||
|
|
@@ -1418,27 +1460,32 @@ def getresponse(self): | |
| if self.__state != _CS_REQ_SENT or self.__response: | ||
| raise ResponseNotReady(self.__state) | ||
|
|
||
| if self.debuglevel > 0: | ||
| if self.__pending_response: | ||
| response = self.__pending_response | ||
| self.__pending_response = None | ||
| elif self.debuglevel > 0: | ||
| response = self.response_class(self.sock, self.debuglevel, | ||
| method=self._method) | ||
| else: | ||
| response = self.response_class(self.sock, method=self._method) | ||
|
|
||
| try: | ||
| try: | ||
| response.begin() | ||
| response.begin(ignore_100_continue=ignore_100_continue) | ||
| except ConnectionError: | ||
| self.close() | ||
| raise | ||
| assert response.will_close != _UNKNOWN | ||
| self.__state = _CS_IDLE | ||
| if response.code != 100: | ||
| # Code 100 is effectively 'not a response' for this purpose | ||
| self.__state = _CS_IDLE | ||
|
|
||
| if response.will_close: | ||
| # this effectively passes the connection to the response | ||
| self.close() | ||
| else: | ||
| # remember this, so we can tell when it is complete | ||
| self.__response = response | ||
| if response.will_close: | ||
| # this effectively passes the connection to the response | ||
| self.close() | ||
| else: | ||
| # remember this, so we can tell when it is complete | ||
| self.__response = response | ||
|
|
||
| return response | ||
| except: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a constructor parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that since it is a property of the server (how quickly it responds with 100 Continue, or whether it understands Expect at all), it should be in the constructor, not the individual request. The other per-request parameters (expect_continue, ignore_100_continue) could vary per-request. LMK if that makes sense or we need to change this and I can update the PR.