CGIHTTPRequestHandler.run_cgi() HTTP_ACCEPT improperly parsed

[mwatkins]

BPO	5054
Nosy	@terryjreedy, @orsenthil, @akheron, @vadmium, @demianbrecht, @miss-islington
PRs	bpo-5054: CGIHTTPRequestHandler.run_cgi() HTTP_ACCEPT improperly parsed #23638 [3.9] bpo-5054: CGIHTTPRequestHandler.run_cgi() HTTP_ACCEPT improperly parsed (GH-23638) #23657
Files	cgi-accept.patch

^{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 = <Date 2020-12-05.15:59:51.277>
created_at = <Date 2009-01-25.15:30:02.235>
labels = ['type-bug', 'library', '3.9', '3.10']
title = 'CGIHTTPRequestHandler.run_cgi() HTTP_ACCEPT improperly parsed'
updated_at = <Date 2020-12-05.16:00:36.000>
user = 'https://bugs.python.org/mwatkins'

bugs.python.org fields:

activity = <Date 2020-12-05.16:00:36.000>
actor = 'orsenthil'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2020-12-05.15:59:51.277>
closer = 'orsenthil'
components = ['Library (Lib)']
creation = <Date 2009-01-25.15:30:02.235>
creator = 'mwatkins'
dependencies = []
files = ['38142']
hgrepos = []
issue_num = 5054
keywords = ['patch']
message_count = 6.0
messages = ['80510', '108657', '236019', '236020', '382569', '382570']
nosy_count = 8.0
nosy_names = ['terry.reedy', 'orsenthil', 'mwatkins', 'catalin.iacob', 'petri.lehtinen', 'martin.panter', 'demian.brecht', 'miss-islington']
pr_nums = ['23638', '23657']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue5054'
versions = ['Python 3.9', 'Python 3.10']

[mwatkins]

There appears to have been a bug in how HTTP_ACCEPT is parsed living in
run_cgi() for eons, perhaps from the time it was written. Perhaps not
many are using this code (I'm not either) but recent (post 3.0 Release)
Python 3.x appear to have broken something in getallmatchingheaders()
(which originates in Message) and I happened to stumble upon this
condition while searching through the stdlib code.

From Line 980 of http.server

        accept = []
        for line in self.headers.getallmatchingheaders('accept'):
            if line[:1] in "\t\n\r ":
                accept.append(line.strip())
            else:
                accept = accept + line[7:].split(',')
        env['HTTP_ACCEPT'] = ','.join(accept)

line[:1] in '\t\n\r' clearly was meant to to be line[-1].

However that doesn't fix completely this chunk of code as it makes some
assumptions about what getallmatchingheaders() delivers which aren't
accurate. The following behaves as expected and feels safer:

        accept = []
        for line in self.headers.getallmatchingheaders('accept'):
            if line.lower().startswith("accept:"):
                line = line[7:]
            for part in line.split(','):
                part = part.strip()
                if part:
                    accept.append(part)
        env['HTTP_ACCEPT'] = ','.join(accept)

Note that post Python 3.0 release,
http.client.HTTPMessage.getallmatchingheaders() was broken. I've
reported this just now and proposed a fix in bpo-5053.

[terryjreedy]

I hope that someone who knows more than me on this subject takes a look at this.

[vadmium]

Posting a patch for this so that we can get rid of the broken HTTPMessage.getallmatchingheaders() method in bpo-5053.

[vadmium]

BTW in the original code, I think line[:1] in "\t\n\r " might have been correct. It looks like the getallmatchinheaders() method was actually meant to return continued lines separately, prefixed with whitespace. My patch is probably only appropriate for Python 3; maybe Mike’s code will work for Python 2.

[miss-islington]

New changeset b630ca7 by Miss Islington (bot) in branch '3.9':
[3.9] bpo-5054: CGIHTTPRequestHandler.run_cgi() HTTP_ACCEPT improperly parsed (GH-23638) (GH-23657)
b630ca7

[orsenthil]

This was also resolved in 3.10 #23638