This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1136

templatefilters: defend against evil unicode strs in json filter
ClosedPublic

Authored by durin42 on Oct 16 2017, 10:56 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG4fdc4adbc838: templatefilters: defend against evil unicode strs in json filter
Summary

We only want to do I/O in terms of bytes, so lets explode early
instead of recursing forever.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Oct 16 2017, 10:56 PM
ryanmce added a subscriber: ryanmce.Oct 17 2017, 8:23 AM

I'm not a python 3 expert by any means, but I'm not sold on this change.

I think we need perhaps a programming error so it's clear its a bug and not something the user needs to fix in their command inputs.

mercurial/templatefilters.py
242–243

I fear that this is is an non-actionable error. How might I see it as a user? If I see it, what do I do?

Also, why is this a TypeError? Should this be a ProgrammingError instead? Or is there a way user input could trigger this?

ryanmce added a comment.Oct 17 2017, 8:39 AM

I'll queue this series except this patch so we can have more discussion on what we should do here.

yuja requested changes to this revision.Oct 17 2017, 9:50 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/templatefilters.py
242–243

It's okay to be non actionable error because it's plain wrong
to pass unicode to Mercurial functions. But I agree it should be
a ProgrammingError. TypeError in filter functions is caught
at templater.runfilter().

This revision now requires changes to proceed.Oct 17 2017, 9:50 AM
durin42 updated this revision to Diff 2917.Oct 17 2017, 10:26 AM
yuja accepted this revision.Oct 18 2017, 8:21 AM
This revision is now accepted and ready to land.Oct 18 2017, 8:21 AM
yuja added a comment.Oct 18 2017, 8:22 AM

Queued. I've added missed import and dropped r'' from the error message.

Closed by commit rHG4fdc4adbc838: templatefilters: defend against evil unicode strs in json filter (authored by durin42). · Explain WhyOct 18 2017, 8:46 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/templatefilters.py (8 lines)

Diff 2966

mercurial/templatefilters.py

# templatefilters.py - common template expansion filters # templatefilters.py - common template expansion filters
# #
# Copyright 2005-2008 Matt Mackall <mpm@selenic.com> # Copyright 2005-2008 Matt Mackall <mpm@selenic.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import os import os
import re import re
import time import time
from . import ( from . import (
encoding, encoding,
error,
hbisect, hbisect,
node, node,
pycompat, pycompat,
registrar, registrar,
templatekw, templatekw,
url, url,
util, util,
) )
elif obj is False: elif obj is False:
return 'false' return 'false'
elif obj is True: elif obj is True:
return 'true' return 'true'
elif isinstance(obj, (int, long, float)): elif isinstance(obj, (int, long, float)):
return pycompat.bytestr(obj) return pycompat.bytestr(obj)
elif isinstance(obj, bytes): elif isinstance(obj, bytes):
return '"%s"' % encoding.jsonescape(obj, paranoid=paranoid) return '"%s"' % encoding.jsonescape(obj, paranoid=paranoid)
elif isinstance(obj, str):
# This branch is unreachable on Python 2, because bytes == str
# and we'll return in the next-earlier block in the elif
# ladder. On Python 3, this helps us catch bugs before they
# hurt someone.
raise error.ProgrammingError(
'Mercurial only does output with bytes on Python 3: %r' % obj)
ryanmceUnsubmitted
Not Done

I fear that this is is an non-actionable error. How might I see it as a user? If I see it, what do I do?

Also, why is this a TypeError? Should this be a ProgrammingError instead? Or is there a way user input could trigger this?

ryanmce: I fear that this is is an non-actionable error. How might I see it as a user? If I see it, what…
yujaUnsubmitted
Not Done

It's okay to be non actionable error because it's plain wrong
to pass unicode to Mercurial functions. But I agree it should be
a ProgrammingError. TypeError in filter functions is caught
at templater.runfilter().

yuja: It's okay to be non actionable error because it's plain wrong to pass unicode to Mercurial…
elif util.safehasattr(obj, 'keys'): elif util.safehasattr(obj, 'keys'):
out = ['"%s": %s' % (encoding.jsonescape(k, paranoid=paranoid), out = ['"%s": %s' % (encoding.jsonescape(k, paranoid=paranoid),
json(v, paranoid)) json(v, paranoid))
for k, v in sorted(obj.iteritems())] for k, v in sorted(obj.iteritems())]
return '{' + ', '.join(out) + '}' return '{' + ', '.join(out) + '}'
elif util.safehasattr(obj, '__iter__'): elif util.safehasattr(obj, '__iter__'):
out = [json(i, paranoid) for i in obj] out = [json(i, paranoid) for i in obj]
return '[' + ', '.join(out) + ']' return '[' + ', '.join(out) + ']'