Roundup Tracker - Issues

Issue 2551046

Title: Attempts to attach file or create large message fail under python2
Type: crash Severity: urgent
Components: Web interface Versions: devel
Status: fixed Resolution: fixed
Dependencies: Superseder:
Assigned To: rouilj Nosy List: ezio.melotti, joseph_myers, rouilj
Priority: high Keywords: Blocker, python2

Created on 2019-06-07 01:38 by rouilj, last changed 2019-10-11 00:57 by rouilj.

File name Uploaded Description Edit Remove rouilj, 2019-06-09 01:23
msg6531 Author: [hidden] (rouilj) Date: 2019-06-07 01:38
Hi Joseph,

Sorry to bother you but I bisected the code and it looks like the 
commit rev5671:f60c44563c3a makes image file uploads and large (967 
works, 1303 fails) messages fail when running under python2:

changeset:   5671:f60c44563c3a
user:        Joseph Myers <>
date:        Sun Mar 24 21:49:17 2019 +0000
files:       roundup/cgi/
Adjust make_file override to use binary files only when needed.

Previous version would produce errors for large text fields in form
submissions with Python 3, so inherit from the default make_file
implementation and only force binary files in the specific case
(self.length >= 0) identified in <>.

So it looks like this error is now happening under python2 post commit.

Running python2 nuke and attaching a file or adding a largish 
message while creating a new issue (or adding to an existing issue) 
causes the following crash:

EXCEPTION AT Fri Jun  7 01:26:07 2019
Traceback (most recent call last):
  File "roundup.hg/roundup/scripts/", line 222, in 
  File "roundup.hg/roundup/scripts/", line 459, in 
    tracker.Client(tracker, self, env).main()
  File "roundup.hg/roundup/cgi/", line 395, in __init__
    self.form = BinaryFieldStorage(fp=request.rfile, environ=env)
  File "/usr/lib/python2.7/", line 507, in __init__
    self.read_multi(environ, keep_blank_values, strict_parsing)
  File "/usr/lib/python2.7/", line 636, in read_multi
    environ, keep_blank_values, strict_parsing)
  File "/usr/lib/python2.7/", line 509, in __init__
  File "/usr/lib/python2.7/", line 646, in read_single
  File "/usr/lib/python2.7/", line 668, in read_lines
  File "/usr/lib/python2.7/", line 728, in 
    self.__write(odelim + line)
  File "/usr/lib/python2.7/", line 675, in __write
    self.file = self.make_file('')
  File "roundup.hg/roundup/cgi/", line 246, in make_file
    return super().make_file()
TypeError: super() takes at least 1 argument (0 given)

Not sure what to do here. Should it be super().make_file(self)?

At the very least it looks like we have no test for this code path.

msg6532 Author: [hidden] (rouilj) Date: 2019-06-07 01:44
Joseph this crash is happening on the commit you did to handle:

Previous version would produce errors for large text fields in form
submissions with Python 3,

See the initial email in this issue for details.

I had hoped assigning it to you would have put you on the nosy list and
got the initial message sent to you, but it looks like it didn't
work as I intended.
msg6533 Author: [hidden] (joseph_myers) Date: 2019-06-07 02:02
I'm guessing this is an issue to do with super() in old-style classes with 
Python 2.  Try adding "or sys.version_info[0] < 3" to the condition for 
using tempfile.TemporaryFile directly (the inheritance is only really 
relevant with Python 3)?
msg6534 Author: [hidden] (rouilj) Date: 2019-06-07 11:02
Hi Joseph:

 In message <>,
Joseph Myers writes:
>I'm guessing this is an issue to do with super() in old-style classes with 
>Python 2.  Try adding "or sys.version_info[0] < 3" to the condition for 
>using tempfile.TemporaryFile directly (the inheritance is only really 
>relevant with Python 3)?

I'll give that a shot and see if it works. Do you have any idea about
how to generate a test case that will call super()?

Have a great weekend.
msg6535 Author: [hidden] (joseph_myers) Date: 2019-06-07 11:37
I don't know the right way to add tests (of large messages / uploads) that 
go through this code and so demonstrate both this issue and the previous 
one with Python 3.
msg6536 Author: [hidden] (rouilj) Date: 2019-06-07 15:20
Hi Joseph:

Yup it looks like you are correct its' a super mispmatch between
py2 and py3.

The form of the super() call isn't python 2 compatible.
Also it looks like the cgi.FieldStorage class under python2
is an old style class. So Ezio's suggestion of changing
it to:

   python2 you need to use super(SubClass, self), on python3
   super() is enough, but super(SubClass, self) is also accepted

doesn't work either as super(BinaryFieldStorage, self) throws:

   File "roundup/cgi/", line 246, in make_file
      return super(BinaryFieldStorage,self).make_file()
   TypeError: super() argument 1 must be type, not classobj

Your suggestion to check python version does work, but it seems

Since there is only single inheritance on the class, I changed it to use:

  class BinaryFieldStorage(cgi.FieldStorage):
    def make_file(self, mode=None):
        import tempfile
        if self.length >= 0:
            return tempfile.TemporaryFile("wb+")
        return cgi.FieldStorage().make_file()

This seems to work in manual testing with both p3 and p2.
Do you know of any reason it wouldn't work?

Also there are a number of places where super(Subclass,self) is
being called. Do you know if any of those were touched
with the python 3 patches? If so they need to be audited and
invoked by tests to make sure we don't have old style class issues.
Unless codecov already reports they are covered.

Also I guess somebody has to figure out how to test this patch
and the other super calls too.

-- rouilj
msg6537 Author: [hidden] (joseph_myers) Date: 2019-06-07 16:41
I don't think cgi.FieldStorage().make_file() is right; the point of using 
super() is that make_file (in Python 3's cgi module) checks 
self._binary_file, which means it needs to be run on the *correct* 
FieldStorage object - with the correct _binary_file value - not on a newly 
constructed object that might have the wrong _binary_file value.
msg6540 Author: [hidden] (rouilj) Date: 2019-06-07 22:02
Tested all the classes passed to super() with issubclass(..., object)
and all classes return true. So that's good.
msg6541 Author: [hidden] (rouilj) Date: 2019-06-09 01:23
I have committed rev5775:17e110426ad7 preserving the super call.

The diff is:

diff -r 765f8c0e99ef -r 17e110426ad7 roundup/cgi/
--- a/roundup/cgi/     Fri Jun 07 21:53:55 2019 -0400
+++ b/roundup/cgi/     Sat Jun 08 21:10:39 2019 -0400
@@ -230,7 +230,8 @@
         if set_cookie:
             self.client.add_cookie(self.cookie_name, self._sid, 
-class BinaryFieldStorage(cgi.FieldStorage):
+# import from object as well so it's a new style object and I can use 
+class BinaryFieldStorage(cgi.FieldStorage, object):
     '''This class works around the bug
        cgi.FieldStorage must save all data as binary/bytes. This is
@@ -243,7 +244,7 @@
         import tempfile
         if self.length >= 0:
             return tempfile.TemporaryFile("wb+")
-        return super().make_file()
+        return super(BinaryFieldStorage, self).make_file()
 class Client:
     """Instantiate to handle one CGI request.

I arrived at this thanks to modifying an example from Ezio.
I have attached the modified example as

There were three ways to handle this:

  1) make sure super is never called on python 2 using the 'or ...'
     clause Joseph initially suggested. Workable but seemed like it
     wasn't in the python spirit.

  2) Use my suggested rewrite to call the base class directly. Joseph
     was correct that it shouldn't be:

         return cgi.FieldStorage().make_file()

     Ezio produced an earlier version of that showed the 
     call should be:

                 return cgi.FieldStorage().make_file(self)

    which addresses Joseph's issue if I were to go this way.

  3) The patch above which is also shown in makes
     BinaryFieldStorage inherit from object as well which makes
     super() when called using python2 and python 3 compatible
     semantics as super(BaseFieldStorage, self).... work. when run under py2 and py3 produces:


showing that super works as expected in this case.

I still can't figure out how to trigger this test case by calling
Client. But I am able to trigger both code paths manually using binary
and text data.


-- rouilj
msg6724 Author: [hidden] (rouilj) Date: 2019-10-11 00:57
I would like to get a test on this case, but I have struggled and failed to find something that hits the second code path. Closing as it is not
a blocker anymore.
Date User Action Args
2019-10-11 00:57:09rouiljsetstatus: open -> fixed
resolution: fixed
messages: + msg6724
2019-06-09 01:23:26rouiljsetfiles: +
messages: + msg6541
2019-06-07 22:02:42rouiljsetmessages: + msg6540
2019-06-07 16:41:12joseph_myerssetmessages: + msg6537
2019-06-07 15:20:42rouiljsetstatus: new -> open
assignee: joseph_myers -> rouilj
messages: + msg6536
nosy: + ezio.melotti
2019-06-07 11:37:35joseph_myerssetmessages: + msg6535
2019-06-07 11:02:53rouiljsetmessages: + msg6534
2019-06-07 02:02:11joseph_myerssetmessages: + msg6533
2019-06-07 01:44:09rouiljsetnosy: + joseph_myers
messages: + msg6532
2019-06-07 01:38:44rouiljcreate