Sun, 11 Mar 2018 13:55:13 -0700 hgweb: clarify that apppath begins with a forward slash
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 13:55:13 -0700] rev 36899
hgweb: clarify that apppath begins with a forward slash Differential Revision: https://phab.mercurial-scm.org/D2821
Sun, 11 Mar 2018 13:38:56 -0700 hgweb: change how dispatch path is reported
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 13:38:56 -0700] rev 36898
hgweb: change how dispatch path is reported When I implemented the new request object, I carried forward some ugly hacks until I could figure out what was happening. One of those was the handling of PATH_INFO to determine how to route hgweb requests. Essentially, if we have PATH_INFO data, we route according to that. But if we don't, we route by the query string. I question if we still need to support query string routing. But that's for another day, I suppose. In this commit, we clean up the ugly "havepathinfo" hack and replace it with a "dispatchpath" attribute that can hold None or empty string to differentiate between the presence of PATH_INFO. This is still a bit hacky. But at least the request parsing and routing code is explicit about the meaning now. Differential Revision: https://phab.mercurial-scm.org/D2820
Sun, 11 Mar 2018 13:11:13 -0700 hgweb: refactor repository name URL parsing
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 13:11:13 -0700] rev 36897
hgweb: refactor repository name URL parsing The hgwebdir WSGI application detects when a requested URL is for a known repository and it effectively forwards the request to the hgweb WSGI application. The hgweb WSGI application needs to route the request based on the base URL for the repository. The way this normally works is SCRIPT_NAME is used to resolve the base URL and PATH_INFO contains the path after the script. But with hgwebdir, SCRIPT_NAME refers to hgwebdir, not the base URL for the repository. So, there was a hacky REPO_NAME environment variable being set to convey the part of the URL that represented the repository so hgweb could ignore this path component for routing purposes. The use of the environment variable for passing internal state is pretty hacky. Plus, it wasn't clear from the perspective of the URL parsing code what was going on. This commit improves matters by making the repository name an explicit argument to the request parser. The logic around handling of this value has been shored up. We add various checks that the argument is used properly - that the repository name does represent the prefix of the PATH_INFO. Differential Revision: https://phab.mercurial-scm.org/D2819
Sun, 11 Mar 2018 12:53:47 -0700 tests: add test coverage for parsing WSGI requests
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 12:53:47 -0700] rev 36896
tests: add test coverage for parsing WSGI requests A subsequent commit will need to make this code more complicated in order to support alternate base URLs. Let's establish some test coverage before we diverge too far from PEP 3333. As part of this, a minor bug related to a missing SCRIPT_NAME key has been squashed. Differential Revision: https://phab.mercurial-scm.org/D2818
Sun, 11 Mar 2018 10:51:14 -0700 hgweb: construct static URL like hgweb does
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:51:14 -0700] rev 36895
hgweb: construct static URL like hgweb does hgwebdir has a bit of code for constructing URLs. This reinvents wheels from our parsedrequest instance. And sometimes the behavior varies from what hgweb does. We'll want to converge that behavior. This commit changes hgwebdir so its staticurl template keyword is constructed the same way as hgweb's. There's probably room to factor this into a shared function. But let's solve the problem of divergence first. Differential Revision: https://phab.mercurial-scm.org/D2817
Sun, 11 Mar 2018 10:38:46 -0700 hgweb: remove unused **map argument
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:38:46 -0700] rev 36894
hgweb: remove unused **map argument It was unused before the recent code refactoring AFAICT. Differential Revision: https://phab.mercurial-scm.org/D2816
Sun, 11 Mar 2018 10:37:25 -0700 hgweb: extract entries() to standalone function
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:37:25 -0700] rev 36893
hgweb: extract entries() to standalone function There was some real wonkiness going on here. Essentially, the inline function was being executed with default arguments because a function reference was passed as-is into the templater. That seemed odd. So now we pass an explicit generator of the function result. Moving this code out of makeindex() makes makeindex() small enough to reason about. This makes it easier to see weird things, like the fact that we're calling self.refresh() twice. Why, I'm not sure. I'm also not sure why we need to call updatereqenv() to possibly update the SERVER_NAME, SERVER_PORT, and SCRIPT_NAME variables as part of rendering an index. I'll dig into these things in subsequent commits. Differential Revision: https://phab.mercurial-scm.org/D2815
Sun, 11 Mar 2018 10:24:46 -0700 hgweb: move rawentries() to a standalone function
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:24:46 -0700] rev 36892
hgweb: move rawentries() to a standalone function It was only accessing a few variables from the outer scope. Let's make it standalone so there is better clarity about what the inputs are. Differential Revision: https://phab.mercurial-scm.org/D2814
Sun, 11 Mar 2018 10:17:58 -0700 hgweb: move archivelist to standalone function
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:17:58 -0700] rev 36891
hgweb: move archivelist to standalone function This doesn't need to exist as an inline function in a method. Minor formatting changes were made as part of the move. Differential Revision: https://phab.mercurial-scm.org/D2813
Sun, 11 Mar 2018 10:15:33 -0700 hgweb: move readallowed to a standalone function
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 10:15:33 -0700] rev 36890
hgweb: move readallowed to a standalone function hgwebdir s kind of large. Let's make the class smaller by moving things that don't need to be there. Differential Revision: https://phab.mercurial-scm.org/D2812
Sun, 11 Mar 2018 15:51:13 -0700 hgweb: remove some use of wsgireq in hgwebdir
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 11 Mar 2018 15:51:13 -0700] rev 36889
hgweb: remove some use of wsgireq in hgwebdir While we're here, rename a method so abide by our style policy, since otherwise check-commit would complain. Differential Revision: https://phab.mercurial-scm.org/D2805
Sat, 10 Mar 2018 20:54:44 -0800 hgweb: fix a bug due to variable name typo
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:54:44 -0800] rev 36888
hgweb: fix a bug due to variable name typo It looks like the "sort" query string parameter was not being honored properly. Differential Revision: https://phab.mercurial-scm.org/D2804
Sat, 10 Mar 2018 20:51:46 -0800 hgweb: stop passing req and tmpl into @webcommand functions (API)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:51:46 -0800] rev 36887
hgweb: stop passing req and tmpl into @webcommand functions (API) We have effectively removed all consumers of the old wsgirequest type. The templater can be accessed on the requestcontext passed into the @webcommand function. For the most part, these arguments are unused. They only exist to provide backwards compatibility. And in the case of wsgirequest, use of that object could actively interfere with the new request object. So let's stop passing these objects to @webcommand functions. With this commit, wsgirequest is practically dead from the hgweb WSGI application. There are still some uses in hgwebdir though... .. api:: @webcommand functions now only receive a single argument. The request and templater instances can be accessed via the ``req`` and ``templater`` attributes of the first argument. Note that the request object is different from previous Mercurial releases and consumers of the previous ``req`` 2nd argument will need updating to use the new API. Differential Revision: https://phab.mercurial-scm.org/D2803
Sat, 10 Mar 2018 19:57:08 -0800 hgweb: pass modern request type into various webutil functions (API)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:57:08 -0800] rev 36886
hgweb: pass modern request type into various webutil functions (API) Our march towards killing wsgirequest continues. .. api:: Various functions in hgweb.webutil now take a modern request object instead of ``wsgirequest``. Differential Revision: https://phab.mercurial-scm.org/D2802
Sat, 10 Mar 2018 19:46:54 -0800 hgweb: don't redundantly pass templater with requestcontext (API)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:46:54 -0800] rev 36885
hgweb: don't redundantly pass templater with requestcontext (API) The requestcontenxt has a ``tmpl`` attribute to access the templater. We don't need to pass the templater explicitly when passing a requestcontext instance. .. api:: Various helper functions in hgweb.webutil no longer accept a templater instance. Access the templater through the ``web`` argument instead. Differential Revision: https://phab.mercurial-scm.org/D2801
Sat, 10 Mar 2018 20:38:28 -0800 hgweb: use templater on requestcontext instance
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:38:28 -0800] rev 36884
hgweb: use templater on requestcontext instance After this commit, all @webcommand function no longer use their "tmpl" argument. Instead, they use the templater attached to the requestcontext. This is the same exact object. So there should be no difference in behavior. Differential Revision: https://phab.mercurial-scm.org/D2800
Sat, 10 Mar 2018 19:41:18 -0800 hgweb: add a sendtemplate() helper function
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:41:18 -0800] rev 36883
hgweb: add a sendtemplate() helper function This pattern is common. Let's make a helper function to reduce boilerplate. We store the "global" template on the requestcontext instance and use it. The templater used by the helper function is the same templater that's passed in as an argument to the @webcommand functions. It needs to be this way because various commands are accessing and mutating the defaults on the templater instance. Differential Revision: https://phab.mercurial-scm.org/D2799
Sat, 10 Mar 2018 19:11:41 -0800 hgweb: use web.req instead of req.req
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:11:41 -0800] rev 36882
hgweb: use web.req instead of req.req We now have access to the modern request type on the requestcontext instance. Let's access it from there. While we're here, remove an unused argument from _search(). Differential Revision: https://phab.mercurial-scm.org/D2798
Sat, 10 Mar 2018 19:08:58 -0800 hgweb: stop setting headers on wsgirequest
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 19:08:58 -0800] rev 36881
hgweb: stop setting headers on wsgirequest All commands now go through the new response API. This is dead code. Differential Revision: https://phab.mercurial-scm.org/D2797
Sat, 10 Mar 2018 20:35:35 -0800 hgweb: always return iterable from @webcommand functions (API)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:35:35 -0800] rev 36880
hgweb: always return iterable from @webcommand functions (API) We had to hack up this function to support our transition to the new response API. Now that we're done with the transition (!!), we can return to returning an iterator of content chunks from these functions. It is tempting to return a normal object and not a generator. However, as the keyword extension demonstrates, extensions may wish to wrap commands and have a try..finally block around execution. Since there is a generator producing content and that generator could be executing code, the try..finally needs to live for as long as the generator is running. That means we have to return a generator so wrappers can consume the generator inside a try..finally. .. api:: hgweb @webcommand functions must use the new response object passed in via ``web.res`` to initiate sending of a response. The hgweb WSGI application will no longer start sending the response automatically. Differential Revision: https://phab.mercurial-scm.org/D2796
Sat, 10 Mar 2018 18:51:32 -0800 hgweb: send errors using new response API
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 18:51:32 -0800] rev 36879
hgweb: send errors using new response API Our slow march off of wsgirequest continues. Differential Revision: https://phab.mercurial-scm.org/D2795
Sat, 10 Mar 2018 18:42:00 -0800 hgweb: refactor 304 handling code
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 18:42:00 -0800] rev 36878
hgweb: refactor 304 handling code We had generic code in wsgirequest for handling HTTP 304 responses. We also had a special case for it in the catch all exception handler in the WSGI application. We only ever raise 304 in one place. So, we don't need to treat it specially in the catch all exception handler. But it is useful to validate behavior of 304 responses. We port the code that sends a 304 to use the new response API. We then move the code for screening 304 sanity into the new response API. As part of doing so, we discovered that we would send Content-Length: 0. This is not allowed. So, we fix our response code to not emit that header for empty response bodies. Differential Revision: https://phab.mercurial-scm.org/D2794
Sat, 10 Mar 2018 18:19:27 -0800 hgweb: transition permissions hooks to modern request type (API)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 18:19:27 -0800] rev 36877
hgweb: transition permissions hooks to modern request type (API) We're trying to remove ``wsgirequest``. The permissions hooks don't do anything they can't do with our new request type. So let's pass that in. This was the last use of ``wsgirequest`` in the wire protocol code! .. api:: hgweb.hgweb_mod.permhooks no longer take a ``wsgirequest`` instance as an argument. Differential Revision: https://phab.mercurial-scm.org/D2793
Sat, 10 Mar 2018 20:16:20 -0800 hgweb: port archive command to modern response API
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:16:20 -0800] rev 36876
hgweb: port archive command to modern response API Well, I tried to go with PEP 3333's recommendations and only allow our WSGI application to emit data via a response generator. Unfortunately, the "archive" command calls into the zipfile and tarfile modules and these operator on file objects and must send their data to an object with write(). There's no easy way turn these write() calls into a generator. So, we teach our response type how to expose a file object like object that can be used to write() output. We try to keep the API consistent with how things work currently: callers must call a setbody*(), then sendresponse() to trigger sending of headers, and only then can they get a handle on the object to perform writing. This required overloading the return value of @webcommand functions even more. Fortunately, we're almost completely ported off the legacy API. So we should be able to simplify matters in the near future. A test relying on this functionality has also been updated to use the new API. Differential Revision: https://phab.mercurial-scm.org/D2792
Sat, 10 Mar 2018 16:17:51 -0800 hgweb: refactor fake file object proxy for archiving
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 16:17:51 -0800] rev 36875
hgweb: refactor fake file object proxy for archiving Python's zip file writer operates on a file object. When doing work, it periodically calls write(), flush(), and tell() on that object. In WSGI contexts, the start_response function returns a write() function. That's a function to write data, not a full file object. So, when the archival code was first introduced in 2b03c6733efa in 2006, someone invented a proxy "tellable" type that wrapped a file object like object and kept track of write count so it could implement tell() and satisfy zipfile's needs. When our archival code runs, it attempts to tell() the destination and if that fails, converts it to a "tellable" instance. Our WSGI application passes the "wsgirequest" instance to the archival function. It fails the tell() test and is converted to a "tellable." It's worth noting that "wsgirequest" implements flush(), so "tellable" doesn't. This hackery all seems very specific to the WSGI code. So this commit moves the "tellable" type and the conversion of the destination file object into the WSGI code. There's a chance some other caller may be passing a file object like object that doesn't implement tell(). But I doubt it. As part of the refactor, our new type implements flush() and doesn't implement __getattr__. Given the intended limited use of this type, I want things to fail fast if there is an attempt to access attributes because I think it is important to document which attributes are being used for what purposes. Differential Revision: https://phab.mercurial-scm.org/D2791
Sat, 10 Mar 2018 16:27:01 -0800 tests: additional test coverage of archive web command
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 16:27:01 -0800] rev 36874
tests: additional test coverage of archive web command This command is special in a few ways. First, it is the only command using the write() function from WSGI's start_response() function. Second, it is setting a custom content-disposition header. We change the test so it prints out full details of the HTTP response. We also save the response body to a file so we can verify its size and hash. The hash check will help ensure that archive generation is deterministic. Differential Revision: https://phab.mercurial-scm.org/D2790
Sat, 10 Mar 2018 15:46:29 -0800 hgweb: port static file handling to new response API
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 15:46:29 -0800] rev 36873
hgweb: port static file handling to new response API hgwebdir_mod hasn't received as much porting effort. So we had to do some minor plumbing to get it to match hgweb_mod and to support the new response object. Differential Revision: https://phab.mercurial-scm.org/D2789
Sat, 10 Mar 2018 15:37:29 -0800 hgweb: remove one-off routing for file?style=raw
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 15:37:29 -0800] rev 36872
hgweb: remove one-off routing for file?style=raw Now that both functions are using the same API, we can unify how the command is called and perform command-specific behavior in the command itself instead of in the high-level router. Differential Revision: https://phab.mercurial-scm.org/D2788
Sat, 10 Mar 2018 20:36:34 -0800 hgweb: port most @webcommand to use modern response type
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 20:36:34 -0800] rev 36871
hgweb: port most @webcommand to use modern response type This only focused on porting the return value. raw file requests are wonky because they go through a separate code path at the dispatch layer. Now that everyone is using the same API, we could clean this up. It's worth noting that wsgirequest.respond() allows sending the Content-Disposition header, but the only user of that feature was removed as part of this change (with the setting of the header now being performed inline). A few @webcommand are not as straightforward as the others and they have not been ported yet. Differential Revision: https://phab.mercurial-scm.org/D2787
Sat, 10 Mar 2018 17:02:57 -0800 hgweb: support using new response object for web commands
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 17:02:57 -0800] rev 36870
hgweb: support using new response object for web commands We have a "requestcontext" type for holding state for the current request. Why we pass in the wsgirequest and templater instance to @webcommand functions, I don't know. I like the idea of standardizing on using "requestcontext" for passing all state to @webcommand functions because that scales well without API changes every time you want to pass a new piece of data. So, we add our new request and response instances to "requestcontext" so @webcommand functions can access them. We also teach our command dispatcher to recognize a new calling convention. Instead of returning content from the @webcommand function, we return our response object. This signals that this response object is to be used for sending output. The keyword extension was wrapping various @webcommand and assuming the output was iterable, so we had to teach it about the new calling convention. To prove everything works, we convert the "filelog" @webcommand to use the new convention. The new calling convention is a bit wonky. I intend to improve this once all commands are ported to use the new response object. Differential Revision: https://phab.mercurial-scm.org/D2786
Sat, 10 Mar 2018 14:19:27 -0800 hgweb: inline caching() and port to modern mechanisms
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 14:19:27 -0800] rev 36869
hgweb: inline caching() and port to modern mechanisms We only had one consumer of this simple function. While it could be a generic function, let's not over abstract the code. As part of inlining, we port it off wsgirequest, fix some Python 3 issues, and set a response header on our new response object so it is ready once we start using it to send responses. Differential Revision: https://phab.mercurial-scm.org/D2785
Sat, 10 Mar 2018 14:06:58 -0800 hgweb: expose repo name on parsedrequest
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 14:06:58 -0800] rev 36868
hgweb: expose repo name on parsedrequest I'm not a fan of doing this because I want to find a better solution to the REPO_NAME hack. But this change gets us a few steps closer to eliminating use of wsgirequest. We can worry about fixing REPO_NAME once wsgirequest is gone. Differential Revision: https://phab.mercurial-scm.org/D2784
Sat, 10 Mar 2018 14:00:40 -0800 hgweb: expose URL scheme and REMOTE_* attributes
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 14:00:40 -0800] rev 36867
hgweb: expose URL scheme and REMOTE_* attributes These are consulted by the HTTP wire protocol handler by reading from the env dict. Let's expose them as attributes instead. With the wire protocol handler updates to use the new attributes, we no longer have any consumers of the legacy wsgirequest type in the wire protocol code (outside of a proxied call to the permissions checker). So, we remove most references to it. Differential Revision: https://phab.mercurial-scm.org/D2783
Sat, 10 Mar 2018 12:31:11 -0800 hgweb: remove wsgirequest.form (API)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:31:11 -0800] rev 36866
hgweb: remove wsgirequest.form (API) Now that everything is ported to consume from parsedrequest.qsparams, we no longer have a need for wsgirequest.form. Let's remove all references to it. .. api:: The WSGI request object no longer exposes a ``form`` attribute containing parsed query string data. Use the ``qsparams`` attribute instead. Differential Revision: https://phab.mercurial-scm.org/D2782
Sat, 10 Mar 2018 12:36:36 -0800 hgweb: perform all parameter lookup via qsparams
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:36:36 -0800] rev 36865
hgweb: perform all parameter lookup via qsparams I think I managed to update all call sites using wsgirequest.form to use parsedrequest.qsparams. Since behavior of qsparams is to retrieve last value, behavior will change if a parameter was specified multiple times. But I think this is acceptable. I'm not a fan of the `req.req.qsparams` pattern. And some of the modified code could be written better. But I was aiming for a straight port with this change. Cleanup can come later. Differential Revision: https://phab.mercurial-scm.org/D2781
Sat, 10 Mar 2018 12:11:26 -0800 hgweb: set variables in qsparams
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:11:26 -0800] rev 36864
hgweb: set variables in qsparams We currently mutate wsgireq.form in a few places. Since it is independent from req.qsparams, we will need to make changes on req.qsparams as well before consumers can use qsparams. So let's do that. Eventually, we'll delete wsgireq.form and all references to it. Differential Revision: https://phab.mercurial-scm.org/D2780
Sat, 10 Mar 2018 11:46:52 -0800 hgweb: use our new request object for "style" parameter
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:46:52 -0800] rev 36863
hgweb: use our new request object for "style" parameter The "style" parameter is kind of wonky because it is explicitly set and has lookups in random locations. Let's port it to qsparams first because it isn't straightforward. There is subtle change in behavior. But I don't think it is worth calling out in a BC. Our multidict's __getitem__ returns the last set value for a key, not the first. So if the query string set a variable multiple times, before we would get the first value and now we would get the last value. It makes no sense to specify these things multiple times. And I think last write wins is more sensible than first write wins. Differential Revision: https://phab.mercurial-scm.org/D2779
Sat, 10 Mar 2018 12:35:38 -0800 hgweb: use a multidict for holding query string parameters
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 12:35:38 -0800] rev 36862
hgweb: use a multidict for holding query string parameters My intention with refactoring the WSGI code was to make it easier to read. I initially wanted to vendor and use WebOb, because it seems to be a pretty reasonable abstraction layer for WSGI. However, it isn't using relative imports and I didn't want to deal with the hassle of patching it. But that doesn't mean we can't use good ideas from WebOb. WebOb has a "multidict" data structure for holding parsed query string and POST form data. It quacks like a dict but allows you to store multiple values for each key. It offers mechanisms to return just one value, all values, or return 1 value asserting that only 1 value is set. I quite like its API. This commit implements a read-only "multidict" in the spirit of WebOb's multidict. We replace the query string attributes of our parsed request with an instance of it. Differential Revision: https://phab.mercurial-scm.org/D2776
Sat, 10 Mar 2018 11:23:05 -0800 hgweb: create dedicated type for WSGI responses
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:23:05 -0800] rev 36861
hgweb: create dedicated type for WSGI responses We have refactored the request side of WSGI processing into a dedicated type. Now let's do the same thing for the response side. We invent a ``wsgiresponse`` type. It takes an instance of a request (for consulation) and the WSGI application's "start_response" handler. The type basically allows setting the HTTP status line, response headers, and the response body. The WSGI application calls sendresponse() to start sending output. Output is emitted as a generator to be fed through the WSGI application. According to PEP 3333, this is the preferred way for output to be transmitted. (Our legacy ``wsgirequest`` exposed a write() to send data. We do not wish to support this API because it isn't recommended by PEP 3333.) The wire protocol code has been ported to use the new API. Differential Revision: https://phab.mercurial-scm.org/D2775
Sat, 10 Mar 2018 11:15:05 -0800 tests: add test for a wire protocol request to wrong base URL
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:15:05 -0800] rev 36860
tests: add test for a wire protocol request to wrong base URL We have code that validates that wire protocol commands (which are specified via query string) must occur at the base URL of a repo. But we have no test coverage for this behavior. Let's add some. Differential Revision: https://phab.mercurial-scm.org/D2778
Fri, 09 Mar 2018 17:10:36 -0800 hgweb: remove support for short query string based aliases (BC)
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 09 Mar 2018 17:10:36 -0800] rev 36859
hgweb: remove support for short query string based aliases (BC) Form data exposed by hgweb is post-processed to expand certain shortcuts. For example, URLs with "?cs=@" is essentially expanded to "?cmd=changeset&node=@". And the URL router treats this the same as "/changeset/@". These shortcuts were initially added in 2005 in 34cb3957d875 and 964baa35faf8. They have rarely been touched in the last decade (just moving code around a bit). We have almost no test coverage of this feature. AFAICT no templates reference URLs of this form. I even looked at the initial version of paper and coal from ~2008 and they use the "/command/params" URL form and not these shortcuts. Furthermore, I couldn't even get some shortcuts to work! For example, "?sl=@" attempts to do a revision search instead of showing shortlog starting at revision @. Maybe I'm just doing it wrong? Because this is ancient, mostly untested code, there is a migration path to something better, and because anyone passionate enough to preserve URLs can install URL redirects, let's nuke the feature. .. bc:: Query string shorts in hgweb like ``?cs=@`` have been removed. Use URLs of the form ``/:cmd`` instead. Differential Revision: https://phab.mercurial-scm.org/D2773
Sat, 10 Mar 2018 11:07:53 -0800 hgweb: remove support for POST form data (BC)
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:07:53 -0800] rev 36858
hgweb: remove support for POST form data (BC) Previously, we called out to cgi.parse(), which for POST requests parsed multipart/form-data and application/x-www-form-urlencoded Content-Type requests for form data, combined it with query string parameters, returned a union of the values. As far as I know, nothing in Mercurial actually uses this mechanism to submit data to the HTTP server. The wire protocol has its own mechanism for passing parameters. And the web interface only does GET requests. Removing support for parsing POST data doesn't break any tests. Another reason to not like this feature is that cgi.parse() may modify the QUERY_STRING environment variable as a side-effect. In addition, it merges both POST data and the query string into one data structure. This prevents consumers from knowing whether a variable came from the query string or POST data. That can matter for some operations. I suspect we use cgi.parse() because back when this code was initially implemented, it was the function that was readily available. In other words, I don't think there was conscious choice to support POST data: we just got it because cgi.parse() supported it. Since nothing uses the feature and it is untested, let's remove support for parsing POST form data. We can add it back in easily enough if we need it in the future. .. bc:: Hgweb no longer reads form data in POST requests from multipart/form-data and application/x-www-form-urlencoded requests. Arguments should be specified as URL path components or in the query string in the URL instead. Differential Revision: https://phab.mercurial-scm.org/D2774
Sat, 10 Mar 2018 11:06:13 -0800 hgweb: expose input stream on parsed WSGI request object
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 11:06:13 -0800] rev 36857
hgweb: expose input stream on parsed WSGI request object Our next step towards moving away from wsgirequest to our newer, friendlier parsedrequest type is input stream access. This commit exposes the input stream on the instance. Consumers in the HTTP protocol server switch to it. Because there were very few consumers of the input stream, we stopped storing a reference to the input stream on wsgirequest directly. All access now goes through parsedrequest. However, wsgirequest still may read from this stream as part of cgi.parse(). So we still need to create the stream from wsgirequest. Differential Revision: https://phab.mercurial-scm.org/D2771
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
Sat, 10 Mar 2018 10:47:30 -0800 hgweb: document continuereader
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:47:30 -0800] rev 36853
hgweb: document continuereader Differential Revision: https://phab.mercurial-scm.org/D2767
Thu, 08 Mar 2018 18:00:04 -0800 hgweb: remove wsgirequest.__iter__
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 18:00:04 -0800] rev 36852
hgweb: remove wsgirequest.__iter__ This was added in d0db3462d568 in 2006. I can't find a justification for this method in PEP 3333. I suspect we were originally intending to use this type as the WSGI application (which should be iterable)? The tests all pass without this method. So let's nuke it. Differential Revision: https://phab.mercurial-scm.org/D2749
Thu, 08 Mar 2018 17:57:07 -0800 hgweb: remove wsgirequest.read()
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 17:57:07 -0800] rev 36851
hgweb: remove wsgirequest.read() This was just a proxy to self.inp.read(). This method serves little value. Let's nuke it. Callers in the wire protocol server have been updated accordingly. Differential Revision: https://phab.mercurial-scm.org/D2748
Sat, 10 Mar 2018 10:46:08 -0800 hgweb: remove unused methods on wsgirequest
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:46:08 -0800] rev 36850
hgweb: remove unused methods on wsgirequest writelines() isn't used in our code base. close() was a no-op. It is an optional method per PEP 3333. My eventual goal is to kill the wsgirequest class, hence why I'm removing code. Differential Revision: https://phab.mercurial-scm.org/D2747
Thu, 08 Mar 2018 17:17:48 -0800 wireprotoserver: remove unused argument from _handlehttperror()
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 17:17:48 -0800] rev 36849
wireprotoserver: remove unused argument from _handlehttperror() Differential Revision: https://phab.mercurial-scm.org/D2746
Sat, 10 Mar 2018 10:44:56 -0800 hgweb: store and use request method on parsed request
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:44:56 -0800] rev 36848
hgweb: store and use request method on parsed request PEP 3333 says that REQUEST_METHOD is always defined. Differential Revision: https://phab.mercurial-scm.org/D2745
Sat, 10 Mar 2018 10:45:12 -0800 hgweb: handle CONTENT_LENGTH
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:45:12 -0800] rev 36847
hgweb: handle CONTENT_LENGTH PEP 3333 says CONTENT_LENGTH may be set. I /think/ WSGI servers are allowed to invent this key even if the client didn't send it. We had code in wireprotoserver looking for this key. So let's just automagically convert this key to an HTTP request header when parsing the request. Differential Revision: https://phab.mercurial-scm.org/D2744
Thu, 08 Mar 2018 16:38:01 -0800 wireprotoserver: access headers through parsed request
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 08 Mar 2018 16:38:01 -0800] rev 36846
wireprotoserver: access headers through parsed request Now that we can access headers via the parsed request object, let's do that. Since the new object uses bytes, hyphens, and is case-insensitive, a bit of code around normalizing values has been removed. I think the new code is much more intuitive because it more closely matches what is going out over the wire. Differential Revision: https://phab.mercurial-scm.org/D2743
Mon, 12 Mar 2018 13:15:00 -0700 hgweb: garbage collect on every request stable
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 12 Mar 2018 13:15:00 -0700] rev 36845
hgweb: garbage collect on every request There appears to be a cycle in localrepository or hgweb that is preventing repositories from being garbage collected when hgwebdir dispatches to hgweb. Every request creates a new repository instance and then leaks that object and other referenced objects. A periodic GC to find cycles will eventually collect the old repositories. But these don't run reliably and rapid requests to hgwebdir can result in rapidly increasing memory consumption. With the Firefox repository, repeated requests to raw-file URLs leak ~100 MB per hgwebdir request (most of this appears to be cached manifest data structures). WSGI processes quickly grow to >1 GB RSS. Breaking the cycles in localrepository is going to be a bit of work. Because we know that hgwebdir leaks localrepository instances, let's put a band aid on the problem in the form of an explicit gc.collect() on every hgwebdir request. As the inline comment states, ideally we'd do this in a finally block for the current request iff it dispatches to hgweb. But _runwsgi() returns an explicit value. We need the finally to run after generator exhaustion. So we'd need to refactor _runwsgi() to "yield" instead of "return." That's too much change for a patch to stable. So we implement this hack one function above and run it on every request. The performance impact of this change should be minimal. Any impact should be offset by benefits from not having hgwebdir processes leak memory.
Sun, 11 Mar 2018 20:10:38 +0900 amend: abort if unresolved merge conflicts found (issue5805) stable
Yuya Nishihara <yuya@tcha.org> [Sun, 11 Mar 2018 20:10:38 +0900] rev 36844
amend: abort if unresolved merge conflicts found (issue5805) It was checked by repo.commit() before e8a7c1a0565a "cmdutil: remove the redundant commit during amend."
Mon, 12 Mar 2018 22:47:33 +0900 debugwireproto: close the write end before consuming all available data
Yuya Nishihara <yuya@tcha.org> [Mon, 12 Mar 2018 22:47:33 +0900] rev 36843
debugwireproto: close the write end before consuming all available data And make it read all available data deterministically. Otherwise util.poll() may deadlock because both stdout and stderr could have no data. Spotted by the next patch which removes stderr from the fds.
Fri, 09 Mar 2018 15:57:16 +0100 graft: check for missing revision first before scanning working copy
Joerg Sonnenberger <joerg@bec.de> [Fri, 09 Mar 2018 15:57:16 +0100] rev 36842
graft: check for missing revision first before scanning working copy Differential Revision: https://phab.mercurial-scm.org/D2753
Sat, 10 Mar 2018 22:02:58 -0500 hook: ensure stderr is flushed when an exception is raised, for test stability
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Mar 2018 22:02:58 -0500] rev 36841
hook: ensure stderr is flushed when an exception is raised, for test stability Windows has had issues with output order in test-ssh-proto-unbundle.t[1] since it was created a few weeks ago. Each of the problems occurred when an exception was thrown out of the hook. Now the only thing blocking D2720 is the fact that the "abort: ..." lines on stderr are totally AWOL. I have no idea where there are. [1] https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/541/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio
Sat, 10 Mar 2018 10:27:56 -0800 wireproto: raise ProgrammingError instead of Abort
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 10 Mar 2018 10:27:56 -0800] rev 36840
wireproto: raise ProgrammingError instead of Abort This isn't a user-facing error and can only be caused by bad Python code. Thanks to Yuya for spotting this. Differential Revision: https://phab.mercurial-scm.org/D2777
(0) -30000 -10000 -3000 -1000 -300 -100 -60 +60 +100 +300 +1000 +3000 +10000 tip