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'

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?

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

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.

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.

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.

Turns out self.db.config.WEB_SECRET_KEY is different each time for the
wsgi implementation while staying constant for the standalone server.

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.

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-/.

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

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

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

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

Hi Ralf:

It's the 

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

does your wsgi server use chunked encoding?

-- rouilj

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

Hi Ralf:

I have a working wsgi setup. I am testing with uwsgi.
I also have gnuicorn working without chunked encoding issues.

In testing, it looks like GET works.

I think I have fixed crash issues with OPTIONS and DELETE.

POST, PUT and PATCH work if I use: application/x-www-form-urlencoded
If I use application/json I get crashes as the form passed into
the rest code isn't constructed right and the call to self.handle_csrf()
attempts to run:

  "@csrf" in self.form

and crashes as self.form.list is None which gives type error not indexable.

I think some of the logic that parses the input data and allows json to pass through needs to be duplicated in the wsgi handler to pass the json.

I'm checking it the changes to the wsgi handler now. Can you update and see
what else is still broken.

On Wed, Oct 23, 2019 at 03:31:45AM +0000, John Rouillard wrote:
> 
> I have a working wsgi setup. I am testing with uwsgi.
> I also have gnuicorn working without chunked encoding issues.
> 
> In testing, it looks like GET works.
> 
> I think I have fixed crash issues with OPTIONS and DELETE.
> 
> POST, PUT and PATCH work if I use: application/x-www-form-urlencoded
> If I use application/json I get crashes as the form passed into
> the rest code isn't constructed right and the call to self.handle_csrf()
> attempts to run:
> 
>   "@csrf" in self.form
> 
> and crashes as self.form.list is None which gives type error not indexable.
> 
> I think some of the logic that parses the input data and allows json
> to pass through needs to be duplicated in the wsgi handler to pass the
> json.
> 
> I'm checking it the changes to the wsgi handler now. Can you update and see
> what else is still broken.

Hmm, I'm using json with post and put all the time.
Works for me.
I'll now do an update and test if one of my scripts using both, post and
put, still works.

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

On Wed, Oct 23, 2019 at 10:00:49AM +0000, Ralf Schlatterbeck wrote:
> Ralf Schlatterbeck added the comment:

> Hmm, I'm using json with post and put all the time.
> Works for me.
> I'll now do an update and test if one of my scripts using both, post and
> put, still works.

Also after your changes everything on my side kept working.
I'm using both, POST and PUT methods with JSON.
So maybe there is still something different with uwsgi vs. wsgi.

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

This looks like a PEBKAC issue.

I just tested again. Using application/json I can use:

  POST to create a new issue
  PUT to modify issue's title and status
  DELETE to retire an issue
  PATCH to retire and restore an issue
  POST with X-HTTP-METHOD-OVERRIDE DELETE to retire an issue

This is under uwsgi.

I did one fix. With PATCH and a json payload, the self.form.list was set to None. This caused a crash in csrf nonce checking. So the check now
validates that self.form.list is not None before it tries: '@csrf' in self.form.

If this passes CI, I will release a 2.0.0alpha0.

2.0.0alpha0 is out.

I don't know of any outstanding issues with wsgi.

Closing.