Roundup Tracker - Issues

Issue 2551047

classification
Title: REST-API not working with wsgi
Type: crash Severity: normal
Components: API Versions: 2.0.0alpha
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: schlatterbeck Nosy List: rouilj, schlatterbeck
Priority: high Keywords: rest

Created on 2019-06-19 17:57 by schlatterbeck, last changed 2019-07-23 09:01 by schlatterbeck.

Messages
msg6551 Author: [hidden] (schlatterbeck) Date: 2019-06-19 17:57
The rest implementation uses several calls to client.request.headers.
In WSGI 'request' is a RequestDispatcher object from
roundup/cgi/wsgi_handler.py
This object doesn't have a 'headers' attribute.

My naive implementation of a headers object would be to take the header
name, convert it to uppercase and replace '-' with '_', prefix it with
HTTP_ and look it up in the CGI environment. Is this an acceptable
solution? Alternatives, better ideas?

I found a google groups article that goes into the same direction:
https://groups.google.com/forum/#!topic/modwsgi/swJmEP79Pds

Traceback when calling rest uri with WSGI:
Traceback (most recent call last):
  File
"/usr/local/lib/python2.7/dist-packages/roundup/cgi/wsgi_handler.py",
line 74, in __call__
    client.main()
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py",
line 471, in main
    self.handle_rest()
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py",
line 572, in handle_rest
    self.path, self.form)
  File "/usr/local/lib/python2.7/dist-packages/roundup/rest.py", line
1723, in dispatch
    headers = self.client.request.headers
AttributeError: 'RequestDispatcher' object has no attribute 'headers'
msg6552 Author: [hidden] (rouilj) Date: 2019-06-19 21:46
Hi Ralf:

In message <1560967042.68.0.358271823986.issue2551047@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>The rest implementation uses several calls to client.request.headers.
>In WSGI 'request' is a RequestDispatcher object from
>roundup/cgi/wsgi_handler.py
>This object doesn't have a 'headers' attribute.
>
>My naive implementation of a headers object would be to take the header
>name, convert it to uppercase and replace '-' with '_', prefix it with
>HTTP_ and look it up in the CGI environment. Is this an acceptable
>solution? Alternatives, better ideas?

The headers are checked in cgi/client.py::handle_csrf using:

       header_names = [ "ORIGIN", "REFERER", "X-FORWARDED-HOST",
            "HOST" ]
       ...
        for header in header_names:
            if (config["WEB_CSRF_ENFORCE_HEADER_%s"%header] == 'required'
                    and "HTTP_%s" % header.replace('-', '_') not in self.env):
                logger.error(self._("csrf header %s required but missing for user%s."), header, current_user)
                raise Unauthorised(self._("Missing header: %s")%header)

so it looks like the env array is being used here as well.

Also in _serve_file:

       # see if there's an if-modified-since...
        # XXX see which interfaces set this
        #if hasattr(self.request, 'headers'):
            #ims = self.request.headers.getheader('if-modified-since')
        if 'HTTP_IF_MODIFIED_SINCE' in self.env:
            # cgi will put the header in the env var

looks like they chose the same solution. Maybe some rationale
for/against this change can be found in the hg repo?

>I found a google groups article that goes into the same direction:
>https://groups.google.com/forum/#!topic/modwsgi/swJmEP79Pds

More support for your suggestion.

>    headers = self.client.request.headers
>AttributeError: 'RequestDispatcher' object has no attribute 'headers'

Are you planning on doing something like:

class HttpHeaders:

  def __init__(self, dict):

  for header, value in items(dict):
     if header.startswith("HTTP"):
       self[header] = value

  return self

then in wsgi_handler:

  RequestDispatcher.headers = HttpHeaders(self.env)

so that RequestDispatcher.headers is a dict?
msg6553 Author: [hidden] (schlatterbeck) Date: 2019-06-21 16:24
Thanks, John, for the quick reply.
So my idea of fixing this looks right.

On Wed, Jun 19, 2019 at 09:46:45PM +0000, John Rouillard wrote:
> Are you planning on doing something like:
> 
> class HttpHeaders:
[...]

Yes, similar. I've not copied the dict but pass the environ into
the constructor and mangle incoming names on lookup.

I'm not entirely sure if we only need to support get / getheader
or if all the other dictionary methods should also be supported.
At least rest.py only uses get.

I've just pushed the change, it now works for me.

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg6554 Author: [hidden] (rouilj) Date: 2019-06-21 21:20
Hi Ralf:

In message <20190621162359.a2emrxlvxrjef5v7@runtux.com>,
Ralf Schlatterbeck writes:
>Thanks, John, for the quick reply.
>So my idea of fixing this looks right.
>
>On Wed, Jun 19, 2019 at 09:46:45PM +0000, John Rouillard wrote:
>> Are you planning on doing something like:
>> 
>> class HttpHeaders:
>[...]
>
>Yes, similar. I've not copied the dict but pass the environ into
>the constructor and mangle incoming names on lookup.

Gotcha.

>I'm not entirely sure if we only need to support get / getheader
>or if all the other dictionary methods should also be supported.
>At least rest.py only uses get.

Might as well just go with these two dictionary methods for now.
We can add more as needed.

>I've just pushed the change, it now works for me.

Cool. Any thought's on how we add tests for these?  All the front ends
need testing and I am at a loss as to how to actually test them.

We could try installing seomthing like:

  https://docs.pylonsproject.org/projects/webtest/en/latest/webtest.html

and I think that could work with cgi, roundup-server, wsgi but I would
prefer something that works without having to install a new library.
msg6555 Author: [hidden] (schlatterbeck) Date: 2019-06-23 11:40
OK, looks like not everything is working yet.
GET requests work and I can navigate the classes.

In the following I'm running an instance of roundup-server and an
instance of WSGI behind apache against the same database. So everything
in the following is tested against the standalone roundup-server where
it works.

But for wsgi
- For application/url-encoded requests to update a class, the computed
  etag and the etag sent do not match, although for the same class the
  etag I'm sending was returned previously, I've not yet investigated
  further
- For application/json it seems like the "input" property containing
  the dictionary is not an istance of SimulateFieldStorageFromJson, so
  consequently several method fail, including trying to retrieve @pretty
  and @apiver from input.

So, in short, updates do not work at all due to possibly multiple reasons.
msg6556 Author: [hidden] (schlatterbeck) Date: 2019-06-23 11:51
OK, the problem with the SimulateFieldStorageFromJson turns out to be
related to a lookup problem of Content-Type: This doesn't have a HTTP_
prefix in CGI, the environment variable is "CONTENT_TYPE" not
"HTTP_CONTENT_TYPE". After special-casing this I'm now also getting a
failed If-Match header verification.
msg6557 Author: [hidden] (schlatterbeck) Date: 2019-06-23 12:04
Turns out self.db.config.WEB_SECRET_KEY is different each time for the
wsgi implementation while staying constant for the standalone server.
msg6559 Author: [hidden] (schlatterbeck) Date: 2019-06-23 12:55
I've committed a fix for the Content-Type variable and made the new
issue2551048 for the changing secret_key value.

John, regarding your question about testability: I also don't have a
good answer, I think testing WSGI can only be done (with meaningful
results) against an application running inside apache. And implementing
(and automating) this is quite some effort.
msg6561 Author: [hidden] (rouilj) Date: 2019-06-23 17:14
In message <1561290709.88.0.990405351233.issue2551047@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>
>
>Ralf Schlatterbeck added the comment:
>
>OK, the problem with the SimulateFieldStorageFromJson turns out to be
>related to a lookup problem of Content-Type: This doesn't have a HTTP_
>prefix in CGI, the environment variable is "CONTENT_TYPE" not
>"HTTP_CONTENT_TYPE". After special-casing this I'm now also getting a
>failed If-Match header verification.

By special casing do you mean given CONTENT_TYPE look up either
HTTP_CONTENT_TYPE or CONTENT_TYPE in the env array?

I wonder if this is also an issue for the cgi interface 8-/.
msg6562 Author: [hidden] (schlatterbeck) Date: 2019-06-23 18:22
On Sun, Jun 23, 2019 at 05:14:04PM +0000, John Rouillard wrote:
> By special casing do you mean given CONTENT_TYPE look up either
> HTTP_CONTENT_TYPE or CONTENT_TYPE in the env array?

I'm doing name-mangling during lookup in the env dict, so this is a
spacial case during name-mangling, I'm simply not prepending HTTP_ in
that case. See code :-)

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg6568 Author: [hidden] (rouilj) Date: 2019-06-27 02:00
Hi Ralf:

I have been playing around with gunicorn and the following wsgi 
wrapper:

===
import sys
sys.path.append('/path/to/roundup/hg/checkout/directory')

# obtain the WSGI request dispatcher                                            
from roundup.cgi.wsgi_handler import RequestDispatcher
tracker_home = 'demo'

def app(environ, start_response):
    RequestDispatcher(tracker_home)(environ, start_response)
===

I invoke it as:

  SCRIPT_NAME=/sysadmin /usr/local/bin/gunicorn --bind 127.0.0.1:8917 -
-log-file=- --timeout=10 wsgi:app

(it is accessed as /sysadmin under my http server).

I am having issues with chrome:

 .../sysadmin/issue188 net::ERR_INCOMPLETE_CHUNKED_ENCODING 200 (OK)

are you seeing anything similar where the web page displays fully but 
seems to keep trying to download?

Also do we need to remove:

          if environ ['REQUEST_METHOD'] == 'OPTIONS':
            code = 501

from roundup/cgi/wsgi_handler.py:RequestDispatcher::__call__
as options is a valid method for the rest call.

-- rouilj
msg6578 Author: [hidden] (rouilj) Date: 2019-07-12 00:36
Hi Ralf:

Any update on this? Is it working as expected now? I tried setting
up a quick WSGI config with gunicorn and documented an issue. I am
wondering if you see similar issues with apache.

-- rouilj
msg6580 Author: [hidden] (schlatterbeck) Date: 2019-07-22 14:23
On Fri, Jul 12, 2019 at 12:36:34AM +0000, John Rouillard wrote:
> 
> Any update on this? Is it working as expected now? I tried setting
> up a quick WSGI config with gunicorn and documented an issue. I am
> wondering if you see similar issues with apache.

Yes, works for me. Running a production config for almost two weeks now
(but with not much REST-use yet) on Python-2.7.

What problems are you seeing?

[Sorry for late reply, just back from 2-week vacation]

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg6584 Author: [hidden] (rouilj) Date: 2019-07-23 00:27
Hi Ralf:

It's the 

  .../sysadmin/issue188 net::ERR_INCOMPLETE_CHUNKED_ENCODING 200 (OK)

does your wsgi server use chunked encoding?

-- rouilj
msg6585 Author: [hidden] (schlatterbeck) Date: 2019-07-23 09:01
On Tue, Jul 23, 2019 at 12:27:24AM +0000, John Rouillard wrote:
> 
> John Rouillard added the comment:
> 
> Hi Ralf:
> 
> It's the 
> 
>   .../sysadmin/issue188 net::ERR_INCOMPLETE_CHUNKED_ENCODING 200 (OK)
> 
> does your wsgi server use chunked encoding?

How do I find out?
I have the standard apache wsgi module (not uwsgi) on debian 9 (stretch,
not yet buster) but I would have to check the production config, it's in
a container and I don't know what OS is inside the container.

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
History
Date User Action Args
2019-07-23 09:01:58schlatterbecksetmessages: + msg6585
2019-07-23 00:27:24rouiljsetmessages: + msg6584
2019-07-22 14:23:45schlatterbecksetmessages: + msg6580
2019-07-12 00:36:34rouiljsetmessages: + msg6578
2019-06-27 02:09:59rouiljsettype: crash
2019-06-27 02:00:52rouiljsetstatus: new -> open
assignee: schlatterbeck
messages: + msg6568
2019-06-24 00:40:39rouiljsetpriority: high
2019-06-24 00:40:13rouiljsetkeywords: + rest
2019-06-23 18:22:00schlatterbecksetmessages: + msg6562
2019-06-23 17:14:04rouiljsetmessages: + msg6561
2019-06-23 12:55:50schlatterbecksetmessages: + msg6559
2019-06-23 12:04:14schlatterbecksetmessages: + msg6557
2019-06-23 11:51:49schlatterbecksetmessages: + msg6556
2019-06-23 11:40:12schlatterbecksetmessages: + msg6555
2019-06-21 21:20:44rouiljsetmessages: + msg6554
2019-06-21 16:24:03schlatterbecksetmessages: + msg6553
2019-06-19 21:46:45rouiljsetmessages: + msg6552
2019-06-19 17:57:22schlatterbeckcreate