I'm getting a traceback when creating a file via apache:

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/wsgi_handler.py", line 127, in __call__
    client.main()
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py", line 514, in main
    self.handle_rest()
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py", line 652, in handle_rest
    self.write(output)
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py", line 2186, in write
    self.header()
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py", line 2532, in header
    self._socket_op(self.request.start_response, headers, response)
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/client.py", line 2050, in _socket_op
    call(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/wsgi_handler.py", line 72, in start_response
    message), headers)
TypeError: expected byte string object, value of type NoneType found

Note that I've instrumented the code, the real line where this occurs in wsgi_handler.py is 68.

It *looks* like this is not in start_response but in the _start_response method called which seems to be a method of the mod_wsgi.Adapter object which is probably written in C.
When I'm logging what is passed to the _start_response method, several headers have None as a value.

When I'm logging response_code, message, headers:

201 Created [('Content-Length', '107'), ('Access-Control-Allow-Headers', 'Content-Type, Authorization, X-Requested-With, X-HTTP-Method-Override'), ('Content-Encoding', 'gzip'), ('Vary', 'Accept-Encoding'), ('Location', 'http://bee.priv.zoo:7070/qso/rest/data/file/2898'), ('Allow', None), ('Access-Control-Allow-Origin', '*'), ('Access-Control-Allow-Methods', None), ('Content-Type', 'application/json')]

So the 'Allow' and the 'Access-Control-Allow-Methods' headers have None as a value.

Any Idea why this happens? The behaviour is quite new but I've not yet pinpointed since when this occurs.

OK found the change that introduced the None values:

changeset:   6525:c505c774a94d
user:        John Rouillard <rouilj@ieee.org>
date:        Sun Nov 07 01:04:43 2021 -0500
files:       CHANGES.txt roundup/rest.py test/rest_common.py test/test_liveserver.py
description:
Mutiple changes to REST code.


John, would you agree that setting these headers to '' instead of None would still fulfill the original intention? I've tested this will not produce a traceback.

On Tue, Dec 07, 2021 at 08:02:30AM +0000, Ralf Schlatterbeck wrote:
> 
> John, would you agree that setting these headers to '' instead of None
> would still fulfill the original intention? I've tested this will not
> produce a traceback.

I've pushed a fix: 6543:982460ed0a23

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com

Hi Ralf:

In message <1638864150.27.0.0128258511087.issue2551178@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>OK found the change that introduced the None values:
>
>changeset:   6525:c505c774a94d
>user:        John Rouillard <rouilj@ieee.org>
>date:        Sun Nov 07 01:04:43 2021 -0500
>files:       CHANGES.txt roundup/rest.py test/rest_common.py test/test_liveserver.py
>description:
>Mutiple changes to REST code.

Well that's a helpful message 8-(. Fortunately the checked in comments
are better.

>John, would you agree that setting these headers to '' instead of
>None would still fulfill the original intention? I've tested this
>will not produce a traceback.

According to what I remember, these headers should not be emitted at
all.

Does this change to client.py solve the issue?

===
@@ -2492,9 +2502,15 @@
         self.write(content)

     def setHeader(self, header, value):
-        """Override a header to be returned to the user's browser.
+        """Override or delete a header to be returned to the user's browser.
         """
-        self.additional_headers[header] = value
+        if value is None:
+            try:
+                del(self.additional_headers[header])
+            except KeyError:
+                pass
+        else:
+            self.additional_headers[header] = value

     def header(self, headers=None, response=None):
         """Put up the appropriate header.
===

Also shouldn't test*Post*() in test/rest_common.py have shown the
issue? Since this appears unique to wsgi, I wonder if a file POST test
case test_liveserver.py can trigger it.

On Tue, Dec 07, 2021 at 03:18:11PM +0000, John Rouillard wrote:
> 
> >John, would you agree that setting these headers to '' instead of
> >None would still fulfill the original intention? I've tested this
> >will not produce a traceback.
> 
> According to what I remember, these headers should not be emitted at
> all.
> 
> Does this change to client.py solve the issue?

Hmm, can't test immediately: This is a "production" (sort-of) system
I'm ham-radio operator and I'm keeping my logs in a roundup tracker.
There are external QSL services (QSL is a report to another ham for a
successful connection called a QSO) from which I'm downloading QSL-Cards
(a picture with some text on it) and attach this to the QSL in Roundup.

The thing fails *after* the picture has been sucessfully created in
Roundup but *before* the QSL item is modified to include the Link to the
picture.

So this isn't the typical json request but a file upload with another
content-type.

>      def setHeader(self, header, value):
> -        """Override a header to be returned to the user's browser.
> +        """Override or delete a header to be returned to the user's browser.
>          """
> -        self.additional_headers[header] = value
> +        if value is None:
> +            try:
> +                del(self.additional_headers[header])
> +            except KeyError:
> +                pass
> +        else:
> +            self.additional_headers[header] = value

On inspection that patch looks like it should work.
Feel free to commit & push and I'll close the issue if it works for me
or tell you when I download pictures next time ;-)

> Also shouldn't test*Post*() in test/rest_common.py have shown the
> issue? Since this appears unique to wsgi, I wonder if a file POST test
> case test_liveserver.py can trigger it.

If that test checks for file-uploads, yes. But I doubt it does (haven't
looked).

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com

Hi Ralf:

In message <20211207155349.mz6kgvfevhjht36e@runtux.com>,
Ralf Schlatterbeck writes:
>On Tue, Dec 07, 2021 at 03:18:11PM +0000, John Rouillard wrote:
>> >John, would you agree that setting these headers to '' instead of
>> >None would still fulfill the original intention? I've tested this
>> >will not produce a traceback.
>> 
>> According to what I remember, these headers should not be emitted at
>> all.
>> 
>> Does this change to client.py solve the issue?
>
>Hmm, can't test immediately: This is a "production" (sort-of) system

Fair enough.

Neat idea about keeping your logs in Roundup. Wish I had known about it
would have made an interesting use case for my article.

>So this isn't the typical json request but a file upload with another
>content-type.

That may be an important point.

>On inspection that patch looks like it should work.
>Feel free to commit & push and I'll close the issue if it works for me
>or tell you when I download pictures next time ;-)

Done changeset:   6544:9aa8df0b4426.

>> Also shouldn't test*Post*() in test/rest_common.py have shown the
>> issue? Since this appears unique to wsgi, I wonder if a file POST test
>> case test_liveserver.py can trigger it.
>
>If that test checks for file-uploads, yes. But I doubt it does (haven't
>looked).

test/test_anydbm.py::anydbmRestTest::testPostFile 

The name is confusing if it doesn't. However it is posting text.

        form.list = [
            cgi.MiniFieldStorage('content', 'hello\r\nthere')
        ]

Maybe binary is a different deal?

On Tue, Dec 07, 2021 at 04:16:26PM +0000, John Rouillard wrote:
> >If that test checks for file-uploads, yes. But I doubt it does (haven't
> >looked).
> 
> test/test_anydbm.py::anydbmRestTest::testPostFile 
> 
> The name is confusing if it doesn't. However it is posting text.
> 
>         form.list = [
>             cgi.MiniFieldStorage('content', 'hello\r\nthere')
>         ]
> 
> Maybe binary is a different deal?

No, I *think* this is unique to WSGI. The traceback came from there. So
it may well be that pythons built-in webserver is more tolerant with
problematic headers. And I'm quite sure the test above doesn't go so
deep to test exchanged headers at html-level. So there is probably no
webserver and headers involved. And if we could test this against
pythons built-in server we could still have it working and wsgi
crashing... I think that level can only be tested with a web-testing
framework like Selenium.

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com

I've just received another QSL. Your new code works!
Should we keep this open? I don't think the tests can be made to test the headers without a bigger web testing framework as said in previous msg.
Thanks
Ralf

In message <1638897716.59.0.162942634814.issue2551178@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>
>
>Ralf Schlatterbeck added the comment:
>
>I've just received another QSL. Your new code works!
>Should we keep this open? I don't think the tests can be made to
>test the headers without a bigger web testing framework as said
>in previous msg.

Well test_liveserver runs roundup as a WSGI. Then it connects to it
using requests. I derived it from some of jerrykan's code back in
April 2021.

Current tests include testing for proper OPTIONS response and
compression/content-encoding.

So requests should be able to post a file and download it. That should
be a reasonable test right?

Test that uploads a file via the rest interface added.
It does crash the server if the fix for this issue is reverted.

changeset:   6567:34199d2fef48

Closing.