[Piglit] [PATCH] framework: Don't use eval in PiglitTest.interpret_result()

[Ilia Mirkin]

On Mon, Apr 21, 2014 at 7:04 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> Python's eval() is not recommended for use anywhere at anytime, because
> it allows arbitrary code to be executed. Not using it is minimally
> invasive and is just a good idea.

FWIW for an even more minimally invasive strategy might be to make the
output json-compliant (s/'/"/) and using json.loads instead of eval.
Although both require changing the format... my concern is that there
exist tests that manually create these output strings. Did you check
for that? Perhaps the json module can be coaxed into not being idiotic
and accepting the single quotes (which, admittedly, are not legal
JSON, but... come on.) Or you could try replacing the ' with "
wholesale in the strings. It would mess up someone encoding 'that\'s'
into "that\"s" but... I can't really bring myself to care about that
either.

>
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> ---
>  framework/exectest.py    | 9 +++++----
>  tests/util/piglit-util.c | 6 +++---
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/framework/exectest.py b/framework/exectest.py
> index 4410115..efc7796 100644
> --- a/framework/exectest.py
> +++ b/framework/exectest.py
> @@ -271,14 +271,15 @@ class PiglitTest(Test):
>
>      def interpret_result(self):
>          outlines = self.result['out'].split('\n')
> -        outpiglit = (s[7:] for s in outlines if s.startswith('PIGLIT:'))
> +        outpiglit = (s[8:] for s in outlines if s.startswith('PIGLIT:'))
>
>          for piglit in outpiglit:
> -            if piglit.startswith('subtest'):
> +            if piglit.startswith('subtest:'):
>                  if not 'subtest' in self.result:
>                      self.result['subtest'] = {}
> -                self.result['subtest'].update(eval(piglit[7:]))
> +                name, value = shlex.split(piglit[9:])
> +                self.result['subtest'][name] = value
>              else:
> -                self.result.update(eval(piglit))
> +                self.result['result'] = piglit
>          self.result['out'] = '\n'.join(
>              s for s in outlines if not s.startswith('PIGLIT:'))
> diff --git a/tests/util/piglit-util.c b/tests/util/piglit-util.c
> index cefc303..8cee65b 100644
> --- a/tests/util/piglit-util.c
> +++ b/tests/util/piglit-util.c
> @@ -220,7 +220,7 @@ piglit_report_result(enum piglit_result result)
>
>         fflush(stderr);
>
> -       printf("PIGLIT: {'result': '%s' }\n", result_str);
> +       printf("PIGLIT: %s\n", result_str);
>         fflush(stdout);
>
>         switch(result) {
> @@ -241,9 +241,9 @@ piglit_report_subtest_result(enum piglit_result result, const char *format, ...)
>
>         va_start(ap, format);
>
> -       printf("PIGLIT:subtest {'");
> +       printf("PIGLIT: subtest: '");
>         vprintf(format, ap);
> -       printf("' : '%s'}\n", result_str);
> +       printf("' '%s'\n", result_str);
>         fflush(stdout);
>
>         va_end(ap);
> --
> 1.9.2
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit