Rest-API hanging on DELETE and OPTIONS not supported.

Hi all:

Since I opened the original REST ticket, I figured I should kick the
tires.

I set up the swagger-ui interface for testing from:

  https://bitbucket.org/kinggreedy1991/restfulapidocumentation

Using the OPTIONS method returns 501 (note the swagger-ui actually
hangs until I ^C'ed the roundup-server). I then used their demo curl
command to see what was happening. This patch fixes that:

diff -r 5fc476d4e34c roundup/scripts/roundup_server.py
--- a/roundup/scripts/roundup_server.py Wed Jan 30 18:11:02 2019 +0100
+++ b/roundup/scripts/roundup_server.py Sat Feb 02 15:21:42 2019 -0500
@@ -255,7 +255,7 @@
                     print('EXCEPTION AT', ts)
		                          traceback.print_exc()

-    do_GET = do_POST = do_HEAD = do_PUT = do_DELETE = do_PATCH = run_cgi
+    do_GET = do_POST = do_HEAD = do_PUT = do_DELETE = do_PATCH = do_OPTIONS = r

     def index(self):
              ''' Print up an index of the available trackers
	      
However with this patch it brings up a new issue.  Trying to use the
new OPTIONS verb support or the DELETE or PATCH verbs hang. E.G.

    curl -vv -X DELETE -u admin -p --header 'Accept: application/json' \
        'https://rouilj.dynamic-dns.net/demo/rest/data/issue/22'

hangs and never comes back. The logs from the roundup-server instance report:

   127.0.0.1 - - [02/Feb/2019 15:05:39] timeout

I should see:

  {"data": {"status": "ok"}}

returned from curl.

The code put out by the swagger-ui (originally version 2.1.2 and then
upgraded to 2.2.10) adds a content-type header:

  --header 'Content-Type: application/x-www-form-urlencoded'

and initially I thought it was the content-type header (with no
content-length header) triggering the problem. But it happens without
the content* headers as well.

I tracked the hang to the standard cgi.py python module in the
FieldStorage.__init__ method. The read_single() method gets called and
hangs forever since there is no data to read. I think it is trying to
read data as though the verb was POST. It looks like any verb other
than GET is treated like POST and tries to read data even if there is
no data.

This patch:

diff -r 5fc476d4e34c roundup/cgi/client.py
--- a/roundup/cgi/client.py     Wed Jan 30 18:11:02 2019 +0100
+++ b/roundup/cgi/client.py     Sat Feb 02 15:35:15 2019 -0500
@@ -363,6 +363,18 @@
 
         # see if we need to re-parse the environment for the form (eg Zope)
         if form is None:
+            # cgi.FieldStorage doesn't special case OPTIONS, DELETE or
+            # PATCH verbs. They are processed like POST. So FieldStorage
+            # hangs on these verbs trying to read posted data that
+            # will never arrive.
+            # If not defined, set CONTENT_LENGTH to 0 so it doesn't
+            # hang reading the data.
+            if self.env['REQUEST_METHOD'] in ['OPTIONS', 'DELETE', 'PATCH']:
+                if 'CONTENT_LENGTH' not in self.env:
+                    self.env['CONTENT_LENGTH'] = 0
+                    logger.debug("Setting CONTENT_LENGTH to 0 for method: %s",
+                                self.env['REQUEST_METHOD'])
+
             self.form = cgi.FieldStorage(fp=request.rfile, environ=env)
             # In some case (e.g. content-type application/xml), cgi
             # will not parse anything. Fake a list property in this case

seems to fix it and I think is safe as it:

  1) applies only to the problem methods (so GET/POST are not changed)
  2) respects any existing CONTENT_LENGTH (so a PATCH with payload
     will work correctly)

with a CONTENT_LENGTH of 0, read_single no longer hangs.

Does anybody have any comments on this issue or my patch?

If not I'll commit it later this week.

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.