httplib simply ignores CONTINUE

[cdwave]

BPO	1346874
Nosy	@orsenthil, @vadmium, @gsauthof, @tbartlett0
PRs	gh-42550: Add 'Expect: 100-Continue' support to httplib #14880
Files	issue1346874.patch: adding 100-continue to httplib issue1346874-273.patch: Updated patch against 2.7.3

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/orsenthil'
closed_at = None
created_at = <Date 2005-11-03.12:23:56.000>
labels = ['type-bug', 'library', '3.9']
title = 'httplib simply ignores CONTINUE'
updated_at = <Date 2020-11-04.17:34:38.738>
user = 'https://bugs.python.org/cdwave'

bugs.python.org fields:

activity = <Date 2020-11-04.17:34:38.738>
actor = 'Mattias Amnefelt'
assignee = 'orsenthil'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2005-11-03.12:23:56.000>
creator = 'cdwave'
dependencies = []
files = ['11073', '26357']
hgrepos = []
issue_num = 1346874
keywords = ['patch']
message_count = 21.0
messages = ['60830', '60831', '60832', '70848', '134337', '134403', '165195', '165196', '165205', '165207', '165209', '165211', '165220', '165252', '241188', '262236', '262801', '262818', '262827', '262832', '348250']
nosy_count = 13.0
nosy_names = ['nnorwitz', 'jjlee', 'orsenthil', 'cdwave', 'rharris', 'edevil', 'nikratio', 'Carl.Nobile', 'martin.panter', 'gms', 'IcicleSpider', 'Mattias Amnefelt', 'tbartlett0']
pr_nums = ['14880']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1346874'
versions = ['Python 3.9']

Linked PRs

• gh-42550: Add 'Expect: 100-Continue' support to httplib #133275
• gh-42550: Add 'Expect: 100-Continue' support to httplib #133276

[cdwave]

I bumped into this code in httplib.py HTTPConnection,
begin():

        # read until we get a non-100 response
        while True:
            version, status, reason = self._read_status()
            if status != CONTINUE:
                break
            # skip the header from the 100 response
            while True:
                skip = self.fp.readline().strip()
                if not skip:
                    break
                if self.debuglevel > 0:
                    print "header:", skip

This basically silently eats any 100-continue response
that the server may send to us.
This is not according to the spec - the client should
WAIT for the 100-continue, and then post the data.
Because of this code snippet, it is impossible for a
client to wait for a 100-continue response, since it is
silently eaten by this code.

A correct implementation would be:

• Check the outgoing headers for "Expect: 100-continue"
• If that is present, set an "expectcontinue" flag.
• If the client tries to send data to the connection,
  or if the data member was set in the request, wait for
  the server to send the 100 response BEFORE sending out
  any data at all, if the flag is set.
• If the server fails to send it, the connection will
  eventually time out.

I'd be happy to provide an implementation myself, as it
doesn't seem hard to implement and would really help my
project.

[nnorwitz]

Logged In: YES
user_id=33168

It's much easier to get a patch integrated since there
should be less work than a bug. I encourage you to work on
a patch.

Don't forget that a patch, must do many things: 1) fix the
code, 2) fix (or add!) tests, 3) fix the documentation with
an appropriate \versionchanged (or \versionadded) tag, and
finally 4) update Misc/NEWS.

The old behaviour should generally be backwards compatible
by default when adding new functionality. If it's a bug, it
may still be desirable to maintain backwards compatibility.
I don't know enough about HTTP to provide any guidance here.

[jjlee]

Logged In: YES
user_id=261020

I don't see any violation of RFC 2616 here. Which part of
the spec., precisely, do you claim is violated?

Still, implementing better 100 Continue handling would be
useful, if done correctly, so go ahead and write that patch!

[rharris]

I'm implemented the behavior described by Mike above with a patch to
2.6. The patch will raise an ExpectationFailed before sending the body
if the server responds with a 417 (Expectation Failed).

This patch should only modify behavior for requests that specify
"Expect: 100-continue" in the request's headers.

Note: The spec says that the client should send the body if the server
does not respond within some reasonable period of time. This is not
currently supported. Anyone have any thoughts on how that could be done?

[CarlNobile]

I have run into this same issue. It does violate RFC2616 in section 4.3 "All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a message-body. All other responses do include a message-body, although it MAY be of zero length."

The embedded while loop is looking for entity data coming back from the server which will never be seen. In my tests the code dies with an exception. I don't see why anything is being done special for a 100 CONTINUE at all. My fix was to eliminate the code previously quoted and replace it with a single line of code so that it would now look like the code snippet below.

   def begin(self):
        if self.msg is not None:
            # we've already started reading the response
            return
        version, status, reason = self._read_status()
        self.status = status
        self.reason = reason.strip()

Note on providing a patch as stated previously.

Having this restriction on providing a patch is a large deterrent to people. I spent a lot of time myself finding the cause of the issues I was having. I don't really have the time to fix tests and documentation also. I understand the reason for asking, but it certainly is a discouragement to helping when bugs are found.

[terryjreedy]

Carl, anyone is free to submit an incomplete patch, and people often do, but if people who are affected by an issue do not think it worth their time to complete it, or even move it along, there is no reason to expect people who are not affected by it to think so either. All of us volunteers are busy doing whatever it is we do.

[edevil]

Can anyone confirm what is missing for this patch to be committed?

Is it just test and documentation changes or is something wrong with the code changes as well?

[CarlNobile]

I was told some time ago that it was documentation changes. And, if I
remember correctly CONTINUE (100) was not ignored, it was actually broken.
Data was being read from stdin when a CONTINUE was received and this should
never happen based on RFC 2616, because there will never be any data to
read.

~Carl

On Tue, Jul 10, 2012 at 12:15 PM, André Cruz <report@bugs.python.org> wrote:

André Cruz <andre@cabine.org> added the comment:

Can anyone confirm what is missing for this patch to be committed?

Is it just test and documentation changes or is something wrong with the
code changes as well?

----------
nosy: +edevil

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue1346874\>

---

[edevil]

As far as I can see, the patch does add some documentation changes. What exactly is missing?

As for the bug, if I understood correctly, what you are saying is that when "ignore_continue" is True, and the server sends a 100 Continue response, the code will continue to try to read data from the server even though none is expected, is that right?