ValueError: invalid literal for int() with base 10: ')(,.",\'",[--'

[Ryan J Ollos]

2016-01-17 16:55:07,918 Trac[main] ERROR: Internal Server Error: <RequestWithSession "GET '/report?asc=%29%28%2C.%22%2C%27%22%2C%5B--&format=csv'">, referrer None
Traceback (most recent call last):
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/web/main.py", line 554, in _dispatch_request
    dispatcher.dispatch(req)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/web/main.py", line 247, in dispatch
    resp = chosen_handler.process_request(req)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/ticket/report.py", line 176, in process_request
    template, data, content_type = self._render_list(req)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/ticket/report.py", line 296, in _render_list
    asc = bool(int(req.args.get('asc', 1)))
ValueError: invalid literal for int() with base 10: ')(,.",\'",[--'

Proposed fix:

trac/ticket/report.py

@@ -30 +30 @@
 from trac.perm import IPermissionRequestor
 from trac.resource import Resource, ResourceNotFound
 from trac.ticket.api import TicketSystem
-from trac.util import as_int, content_disposition
+from trac.util import as_bool, as_int, content_disposition
 from trac.util.datefmt import format_datetime, format_time, from_utimestamp
 from trac.util.presentation import Paginator
 from trac.util.text import exception_to_unicode, to_unicode, quote_query_string
@@ -293 +293 @@
     def _render_list(self, req):
         """Render the list of available reports."""
         sort = req.args.get('sort', 'report')
-        asc = bool(int(req.args.get('asc', 1)))
+        asc = as_bool(req.args.get('asc', 1))
         format = req.args.get('format')
 
         rows = self.env.db_query("""

[Ryan J Ollos]

Fixed in [14485], merged in [14486].

Looks like some test failures, possibly due to Pygments 2.1. I'll investigate and open a new ticket if necessary.

[Jun Omae]

report.py seems to still have the same issue.

$ git grep -n '\<bool(int('
trac/ticket/query.py:341:                            val = bool(int(val))
trac/ticket/report.py:398:        asc = bool(int(asc)) # string '0' or '1' to int/boolean
trac/wiki/web_api.py:45:            options['escape_newlines'] = bool(int(req.args['escape_newlines']
trac/wiki/web_api.py:48:            options['shorten'] = bool(int(req.args['shorten'] or 0))

The following lines might have the same issue.

$ git grep -n '\<\(bool\|int\)(req\.args\>'
trac/admin/web_ui.py:469:                anchor = '#no%d' % (int(req.args.get('plugin')) + 1)
trac/search/web_ui.py:160:        noquickjump = int(req.args.get('noquickjump', '0'))
trac/ticket/admin.py:689:                                   str(int(req.args.get(key)))) for key
trac/ticket/report.py:153:        id = int(req.args.get('id', -1))
trac/ticket/report.py:389:        page = int(req.args.get('page', '1'))
trac/ticket/web_ui.py:518:        id = int(req.args.get('id'))
trac/ticket/web_ui.py:551:                cnum = int(req.args.get('cnum'))
trac/ticket/web_ui.py:557:                cnum = int(req.args.get('cnum'))
trac/ticket/web_ui.py:572:                cnum = int(req.args['cnum_edit'])
trac/ticket/web_ui.py:837:        new_version = int(req.args.get('version', 1))
trac/ticket/web_ui.py:838:        old_version = int(req.args.get('old_version', new_version))
trac/ticket/web_ui.py:1034:        new_version = int(req.args.get('version', 1))
trac/ticket/web_ui.py:1035:        old_version = int(req.args.get('old_version', new_version))
trac/timeline/web_ui.py:94:        maxrows = int(req.args.get('max', 50 if format == 'rss' else 0))
trac/versioncontrol/diff.py:335:        context = int(req.args.get('contextlines', pref))
trac/versioncontrol/diff.py:342:    arg = int(req.args.get('contextall', 0))
trac/versioncontrol/web_ui/log.py:87:        limit = int(req.args.get('limit') or self.default_log_limit)
trac/wiki/web_api.py:45:            options['escape_newlines'] = bool(int(req.args['escape_newlines']
trac/wiki/web_api.py:48:            options['shorten'] = bool(int(req.args['shorten'] or 0))
trac/wiki/web_ui.py:266:        version = int(req.args.get('version', 0)) or None
trac/wiki/web_ui.py:267:        old_version = int(req.args.get('old_version', 0)) or version
trac/wiki/web_ui.py:374:            version = int(req.args.get('version', 0))
trac/wiki/web_ui.py:375:        old_version = int(req.args.get('old_version') or 0) or version
trac/wiki/web_ui.py:516:                                version=int(req.args['version']))
tracopt/ticket/deleter.py:126:        id = int(req.args.get('id'))

[Ryan J Ollos]

I didn't review the entire module, but it seems like a good idea to fix all possible instances.

[Ryan J Ollos]

Modified test case in [14505], merged to trunk in [14506].

I used as_int or as_bool for all the cases in which I could produce a traceback. Proposed changes in log:rjollos.git:t12325_value_error and log:rjollos.git:t12325_value_error_trunk.

[Christian Boos]

Looks good. However, I wonder if we shouldn't rather go a step further and have:

• req.args.as_int('blah') → raise TracError(_("Invalid request arguments.")) if not OK
• req.args.as_int('blah', default) → pick default if not OK

Same for .as_bool.

[Ryan J Ollos]

That sounds like a good idea. I'll look into implementing the changes on the trunk.

[Ryan J Ollos]

Need to check if this issue is fixed, and if not, add to proposed changes:

[pid 8711 140573411886848] 2016-02-09 10:36:57,402 Trac[main] ERROR: Internal Server Error: <RequestWithSession "GET '/wiki/WikiStart?action=diff&version=103&old_version=101.'">, referrer None
Traceback (most recent call last):
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/web/main.py", line 607, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/web/main.py", line 256, in dispatch
    resp = chosen_handler.process_request(req)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/wiki/web_ui.py", line 180, in process_request
    return self._render_diff(req, versioned_page)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/wiki/web_ui.py", line 431, in _render_diff
    old_version = int(old_version)
ValueError: invalid literal for int() with base 10: '101.'

[Ryan J Ollos]

This issue is probably fixed by proposed changes:

[pid 19077 140573411886848] 2016-02-10 21:04:11,539 Trac[main] ERROR: Internal Server Error: <RequestWithSession "GET '/ticket/2182?action=diff&version=93.\xea\xb0\x84\xed\x8a\xb8\xec\xb0\xa8\xed\x8a\xb8\xec\x9a\xa9\xec\x96\xb4\xec\xa0\x95\xeb\xa6\xac\xeb\xb0\x8f\xec\xba\x98\xeb\xa6\xb0\xeb\x8d\x94mainnav\xec\x9e\x98\xeb\xaa\xbb\xed\x91\x9c\xec\x8b\x9c\xeb\x90\x98\xeb\x8a\x94\xeb\xac\xb8\xec\xa0\x9c'">, referrer None
Traceback (most recent call last):
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/web/main.py", line 607, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/web/main.py", line 256, in dispatch
    resp = chosen_handler.process_request(req)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/ticket/web_ui.py", line 177, in process_request
    return self._process_ticket_request(req)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/ticket/web_ui.py", line 592, in _process_ticket_request
    return self._render_diff(req, ticket, data, text_fields)
  File "/usr/local/virtualenv/1.1dev/lib/python2.7/site-packages/trac/ticket/web_ui.py", line 896, in _render_diff
    new_version = int(req.args.get('version', 1))
UnicodeEncodeError: 'decimal' codec can't encode characters in position 3-14: invalid decimal Unicode string

[Ryan J Ollos]

This issue will be fixed with the proposed changes:

2016-03-04 17:15:37,497 Trac[main] ERROR: Internal Server Error: <RequestWithSession "GET "/search?q=leaks&noquickjump=1'A=0&ticket=on"">, referrer "https://dev.jquery.com/search?q=leaks&noquickjump=1'A=0&ticket=on"
Traceback (most recent call last):
  File "/var/www/bugs.jquery.com/private/pve/local/lib/python2.7/site-packages/trac/web/main.py", line 554, in _dispatch_request
    dispatcher.dispatch(req)
  File "/var/www/bugs.jquery.com/private/pve/local/lib/python2.7/site-packages/trac/web/main.py", line 247, in dispatch
    resp = chosen_handler.process_request(req)
  File "/var/www/bugs.jquery.com/private/pve/local/lib/python2.7/site-packages/trac/search/web_ui.py", line 100, in process_request
    data['quickjump'] = self._check_quickjump(req, query)
  File "/var/www/bugs.jquery.com/private/pve/local/lib/python2.7/site-packages/trac/search/web_ui.py", line 160, in _check_quickjump
    noquickjump = int(req.args.get('noquickjump', '0'))
ValueError: invalid literal for int() with base 10: "1'A=0"

[Ryan J Ollos]

Previously we used the pattern as_int(req.get('key', default1), default2)), where default1 is returned if 'key' is not in the dict, and default2 is returned if an exception is raised converting the value of 'key'.

req.as_int('key', default) doesn't quite have the same flexibility. default will be returned when the key is not in the dict and if an exception is raised converting the value of 'key'. If you don't specify a default you'll get an exception for both scenarios.

The only way I can see the reconcile the difference would be to have an on_error argument, and have HTTPBadRequest raised if on_error isn't specified.

Return '1' if exception is raised converting value of arg1:

>>> req.args.as_int('arg1', default='1', on_error='1')

Raise HTTPBadRequest if exception is raised converting value of arg1:

>>> req.args.as_int('arg1', default='1')

[Ryan J Ollos]

Replying to Ryan J Ollos:

If you don't specify a default you'll get an exception for both scenarios.

On second thought, this doesn't necessarily need to be true. We could return the default when the key is not in dict.

I'm thinking that as_int and as_bool should just raise an HTTPBadRequest whenever an exception is raised converting the input parameter.

Initial changes committed to 1.0-stable in [14663], merged to trunk in [14664].

[Ryan J Ollos]

Replying to Ryan J Ollos:

Replying to Ryan J Ollos:

If you don't specify a default you'll get an exception for both scenarios.

On second thought, this doesn't necessarily need to be true. We could return the default when the key is not in dict.

I think this is the key point that should be further considered. There are scenarios when we may want to return default when the value can't be converted, and there are scenarios when we may want to raise HTTPBadRequest when the value can't be converted. In both scenarios we probably want the typical behavior of returning default when the key is not in the dict. I created #12426 to discuss.