Page MenuHomePhabricator

templatefilters: defend against evil unicode strs in json filter
ClosedPublic

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

Details

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

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?

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.

This revision was automatically updated to reflect the committed changes.