Sat, 10 Mar 2018 10:56:10 -0800 hgweb: make parsedrequest part of wsgirequest
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:56:10 -0800] rev 36856
hgweb: make parsedrequest part of wsgirequest This is kind of ugly. But an upcoming commit will teach parsedrequest about the input stream. Because the input stream is global state and can't be accessed without side-effects, we need to take actions to ensure that multiple consumers don't read from it independently. The easiest way to do this is for one object to hold a reference to both items having access to the input stream so that when a copy is made, we can remove the attribute from the other instance. So we create our parsed request instance from the wsgirequest constructor and hold a reference to it there. This is better than our new type holding a reference to wsgirequest because all the code for managing access will be temporary and we shouldn't pollute parsedrequest with this ugly history. Differential Revision: https://phab.mercurial-scm.org/D2770
Sat, 10 Mar 2018 11:03:45 -0800 hgweb: refactor the request draining code
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:03:45 -0800] rev 36855
hgweb: refactor the request draining code The previous code for draining was only invoked in a few places in the wire protocol. Behavior wasn't consist. Furthermore, it was difficult to reason about. With us converting the input stream to a capped reader, it is now safe to always drain the input stream when its size is known because we can never overrun the input and read into the next HTTP request. The only question is "should we?" This commit changes the draining code so every request is examined. Draining now kicks in for a few requests where it wouldn't before. But I think the code is sufficiently restricted so the behavior is safe. Possibly the most dangerous part of this code is the issuing of Connection: close for POST and PUT requests that don't have a Content-Length. I don't think there are any such uses in our WSGI application, so this should be safe. In the near future, I plan to significantly refactor the WSGI response handling. I anticipate this code evolving a bit. So any minor regressions around draining or connection closing behavior might be fixed as a result of that work. All tests pass with this change. That scares me a bit because it means we are lacking low-level tests for the HTTP protocol. Differential Revision: https://phab.mercurial-scm.org/D2769
Sat, 10 Mar 2018 10:48:34 -0800 hgweb: use a capped reader for WSGI input stream
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:48:34 -0800] rev 36854
hgweb: use a capped reader for WSGI input stream Per PEP 3333, the input stream from WSGI should respect EOF and prevent reads past the end of the request body. However, not all WSGI servers guarantee this. Notably, our BaseHTTPServer based built-in HTTP server doesn't. Instead, it exposes the raw socket and you can read() from it all you want, getting the connection in a bad state by doing so. We have a "cappedreader" utility class that proxies a file object and prevents reading past a limit. This commit converts the WSGI input stream into a capped reader when the input length is advertised via Content-Length headers. "cappedreader" only exposes a read() method. PEP 3333 states that the input stream MUST also support readline(), readlines(hint), and __iter__(). However, since our WSGI application code only calls read() and since we're not manipulating the stream exposed by the WSGI server, we're not violating the spec here. Differential Revision: https://phab.mercurial-scm.org/D2768
(0) -30000 -10000 -3000 -1000 -300 -100 -30 -10 -3 +3 +10 +30 +100 +300 +1000 +3000 +10000 tip