#11919 drop dep on deprecated cgi (removed in 3.13)#11918
Conversation
graingert
changed the title
graingert
force-pushed
the
drop-deprecated-cgi
branch
from
d0282dd to
60b5ed5
Compare
|
Many thanks for the PR. I think that vendoring is the only way to keep backward compatibilty. We can replace parse_multipart with I think that it's ok to merge this. We should find a better API for twisted form parsing...and once we have that new API we can deprecate this. |
…y backporting parse_qsl
twm
left a comment
twm
left a comment
There was a problem hiding this comment.
I am hesitant to vendor so much code without also vendoring its tests. It looks like using the older parse_multipart implementation would lower the burden in both ways.
| def parse_multipart(fp, pdict, encoding="utf-8", errors="replace", separator="&"): | ||
| """Parse multipart input. | ||
|
|
||
| Arguments: | ||
| fp : input file | ||
| pdict: dictionary containing other parameters of content-type header | ||
| encoding, errors: request encoding and error handler, passed to | ||
| FieldStorage | ||
|
|
||
| Returns a dictionary just like parse_qs(): keys are the field names, each | ||
| value is a list of values for that field. For non-file fields, the value | ||
| is a list of strings. | ||
| """ | ||
| # RFC 2046, Section 5.1 : The "multipart" boundary delimiters are always | ||
| # represented as 7bit US-ASCII. | ||
| boundary = pdict["boundary"].decode("ascii") | ||
| ctype = "multipart/form-data; boundary={}".format(boundary) | ||
| headers = Message() | ||
| headers.set_type(ctype) | ||
| try: | ||
| headers["Content-Length"] = pdict["CONTENT-LENGTH"] | ||
| except KeyError: | ||
| pass | ||
| fs = FieldStorage( | ||
| fp, | ||
| headers=headers, | ||
| encoding=encoding, | ||
| errors=errors, | ||
| environ={"REQUEST_METHOD": "POST"}, | ||
| separator=separator, | ||
| ) | ||
| return {k: fs.getlist(k) for k in fs} |
There was a problem hiding this comment.
I think that we should use the version of cgi.parse_multipart from before this commit. That commit regressed on the type of headers (from bytes to str) and introduced the encoding stuff, which is (a) a questionable design choice at this layer and (b) actively counterproductive for Twisted, since we need bytes.
| from twisted.web import _cgi | ||
| fs = _cgi.FieldStorage() |
There was a problem hiding this comment.
If you take my suggestion above I think that we can avoid vendoring FieldStorage entirely by replace this with something that echoes os.environ['QUERY_STRING'] (RFC). FieldStorage appears to have been used purely for convenience, and as the cgi module is going away I don't think there's any sense verifying compatibility with it.
HEADER_OUTPUT_CGI looks like it'd work for this.
5 participants
Scope and purpose
Fixes #11919