Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12316 closed defect (fixed)

TypeError: coercing to Unicode: need string or buffer, NoneType found

Reported by: Ryan J Ollos Owned by: Jun Omae
Priority: normal Milestone: 1.0.11
Component: general Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Avoid TypeError in invalid request that has form data with an unnamed value.

API Changes:
Internal Changes:

Description

2015-12-24 02:53:30,390 Trac[main] ERROR: Internal Server Error: <RequestWithSession "POST '/'">, referrer 'http://bitten.edgewall.org'
Traceback (most recent call last):
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/main.py", line 554, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/main.py", line 237, in dispatch
    req.args.get('__FORM_TOKEN') != req.form_token:
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 355, in __getattr__
    value = self.callbacks[name](self)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 338, in <lambda>
    'args': lambda req: arg_list_to_args(req.arg_list),
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 355, in __getattr__
    value = self.callbacks[name](self)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 732, in _parse_arg_list
    name = unicode(value.name, 'utf-8')
TypeError: coercing to Unicode: need string or buffer, NoneType found

Attachments (0)

Change History (8)

comment:1 by Ryan J Ollos, 5 years ago

I'm unsure which form submit leads to the error. The only POST form at http://bitten.edgewall.org/ is the logout form.

According to the documentation None can occur as a field name. We may want to account for that: tags/trac-1.0.9/trac/web/api.py@:725,732#L705.

Maybe the following patch would handle the issue:

  • trac/web/api.py

    diff --git a/trac/web/api.py b/trac/web/api.py
    index 0ea3e05..ab38827 100644
    a b class Request(object):  
    728728
    729729        args = []
    730730        for value in fs.list or ():
     731            name = None
    731732            try:
    732                 name = unicode(value.name, 'utf-8')
     733                if value.name is not None:
     734                    name = unicode(value.name, 'utf-8')
    733735                if not value.filename:
    734736                    value = unicode(value.value, 'utf-8')
    735737            except UnicodeDecodeError, e:

Or would we want to use an empty string as the name when value.name is None?

in reply to:  1 ; comment:2 by Jun Omae, 5 years ago

That issue can be reproduced with form-data with unnamed value. It's needed to retrieve the value from req.arg_list instead of req.args. However, I don't think that is valid request.

  • trac/web/api.py

    diff --git a/trac/web/api.py b/trac/web/api.py
    index 0ea3e05af..b2934318a 100644
    a b class Request(object):  
    729729        args = []
    730730        for value in fs.list or ():
    731731            try:
    732                 name = unicode(value.name, 'utf-8')
     732                name = value.name
     733                if name is not None:
     734                    name = unicode(name, 'utf-8')
    733735                if not value.filename:
    734736                    value = unicode(value.value, 'utf-8')
    735737            except UnicodeDecodeError, e:
  • trac/web/tests/api.py

    diff --git a/trac/web/tests/api.py b/trac/web/tests/api.py
    index 7070ae615..b87612233 100644
    a b class RequestTestCase(unittest.TestCase):  
    331331        req = Request(environ, None)
    332332        self.assertRaises(HTTPBadRequest, lambda: req.arg_list)
    333333
     334    def test_post_with_unnamed_value(self):
     335        boundary = '_BOUNDARY_'
     336        data = (
     337            '--%(boundary)s\r\n'
     338            'Content-Disposition: form-data; name="foo"\r\n'
     339            '\r\n'
     340            'named value\r\n'
     341            '--%(boundary)s\r\n'
     342            'Content-Disposition: form-data; name=""\r\n'
     343            '\r\n'
     344            'name is empty\r\n'
     345            '--%(boundary)s\r\n'
     346            'Content-Disposition: form-data\r\n'
     347            '\r\n'
     348            'unnamed value\r\n'
     349            '--%(boundary)s--\r\n'
     350        )
     351        data %= {'boundary': boundary}
     352        content_type = 'multipart/form-data; boundary="%s"' % boundary
     353        environ = self._make_environ(method='POST',
     354                                     **{'wsgi.input': StringIO(data),
     355                                        'CONTENT_LENGTH': str(len(data)),
     356                                        'CONTENT_TYPE': content_type})
     357        req = Request(environ, None)
     358        self.assertEqual('named value', req.args['foo'])
     359        self.assertEqual([('foo', 'named value'), ('', 'name is empty'),
     360                          (None, 'unnamed value')],
     361                         req.arg_list)
     362
    334363
    335364class RequestSendFileTestCase(unittest.TestCase):
    336365

comment:3 by Ryan J Ollos, 5 years ago

Here's another issue which touches similar areas of the code:

2016-01-13 20:39:12,290 Trac[main] ERROR: Internal Server Error: <RequestWithSession "POST '/attachment/wiki/Documentation/configure.html/'">, referrer 'http://bitten.edgewall.org/attachment/wiki/Documentation/configure.html/?action=new'
Traceback (most recent call last):
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/main.py", line 554, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/main.py", line 203, in dispatch
    if handler.match_request(req):
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/attachment.py", line 481, in match_request
    req.args['realm'] = realm
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 355, in __getattr__
    value = self.callbacks[name](self)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 338, in <lambda>
    'args': lambda req: arg_list_to_args(req.arg_list),
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 355, in __getattr__
    value = self.callbacks[name](self)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 725, in _parse_arg_list
    fs = _FieldStorage(fp, environ=self.environ, keep_blank_values=True)
  File "/usr/lib/python2.7/cgi.py", line 508, in __init__
    self.read_multi(environ, keep_blank_values, strict_parsing)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/web/api.py", line 208, in read_multi
    cgi.FieldStorage.read_multi(self, *args, **kwargs)
  File "/usr/lib/python2.7/cgi.py", line 637, in read_multi
    environ, keep_blank_values, strict_parsing)
  File "/usr/lib/python2.7/cgi.py", line 510, in __init__
    self.read_single()
  File "/usr/lib/python2.7/cgi.py", line 647, in read_single
    self.read_lines()
  File "/usr/lib/python2.7/cgi.py", line 669, in read_lines
    self.read_lines_to_outerboundary()
  File "/usr/lib/python2.7/cgi.py", line 697, in read_lines_to_outerboundary
    line = self.fp.readline(1<<16)
IOError: request data read error

in reply to:  2 comment:4 by Ryan J Ollos, 5 years ago

Replying to Jun Omae:

That issue can be reproduced with form-data with unnamed value. It's needed to retrieve the value from req.arg_list instead of req.args. However, I don't think that is valid request.

Is that a patch that you think we should consider committing, or is it just a proof of concept to demonstrate?

comment:5 by Ryan J Ollos, 5 years ago

Milestone: 1.0.101.0.11

comment:6 by Ryan J Ollos, 5 years ago

I'll push this change after #12403 is committed.

comment:7 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to 1.0-stable [14622], merged to trunk in [14623].

comment:8 by Ryan J Ollos, 5 years ago

Owner: changed from Ryan J Ollos to Jun Omae

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.