From dfa9324966ce1a38346d15e35805d042848aabf1 Mon Sep 17 00:00:00 2001 From: Ramiro Morales Date: Tue, 12 Feb 2013 13:58:49 -0300 Subject: [PATCH] Don't use os.system() in compilemessages. Fixes #19584. This implies stop storing file path command line arguments in envvars as a security measure to start relying on with Popen's shell=False instead, and addition of an 'utils' module. Thanks kmichel_wgs for the report. --- .../management/commands/compilemessages.py | 24 +++++++++---------- django/core/management/utils.py | 14 +++++++++++ tests/i18n/commands/compilation.py | 19 +++++++++++++++ .../commands/locale/ja/LC_MESSAGES/django.po | 21 ++++++++++++++++ tests/i18n/tests.py | 3 ++- 5 files changed, 67 insertions(+), 14 deletions(-) create mode 100644 django/core/management/utils.py create mode 100644 tests/i18n/commands/locale/ja/LC_MESSAGES/django.po diff --git a/django/core/management/commands/compilemessages.py b/django/core/management/commands/compilemessages.py index 8f2c1ff771..2ca42d1c63 100644 --- a/django/core/management/commands/compilemessages.py +++ b/django/core/management/commands/compilemessages.py @@ -2,9 +2,10 @@ from __future__ import unicode_literals import codecs import os -import sys from optparse import make_option + from django.core.management.base import BaseCommand, CommandError +from django.core.management.utils import popen_wrapper from django.utils._os import npath def has_bom(fn): @@ -41,18 +42,15 @@ def compile_messages(stderr, locale=None): if has_bom(fn): raise CommandError("The %s file has a BOM (Byte Order Mark). Django only supports .po files encoded in UTF-8 and without any BOM." % fn) pf = os.path.splitext(fn)[0] - # Store the names of the .mo and .po files in an environment - # variable, rather than doing a string replacement into the - # command, so that we can take advantage of shell quoting, to - # quote any malicious characters/escaping. - # See http://cyberelk.net/tim/articles/cmdline/ar01s02.html - os.environ['djangocompilemo'] = npath(pf + '.mo') - os.environ['djangocompilepo'] = npath(pf + '.po') - if sys.platform == 'win32': # Different shell-variable syntax - cmd = 'msgfmt --check-format -o "%djangocompilemo%" "%djangocompilepo%"' - else: - cmd = 'msgfmt --check-format -o "$djangocompilemo" "$djangocompilepo"' - os.system(cmd) + program = 'msgfmt' + args = [program, '--check-format', '-o', npath(pf + '.mo'), npath(pf + '.po')] + output, errors, status = popen_wrapper(args) + if status: + if errors: + msg = "Execution of %s failed: %s" % (program, errors) + else: + msg = "Execution of %s failed" % program + raise CommandError(msg) class Command(BaseCommand): diff --git a/django/core/management/utils.py b/django/core/management/utils.py new file mode 100644 index 0000000000..4204a313bf --- /dev/null +++ b/django/core/management/utils.py @@ -0,0 +1,14 @@ +import os +from subprocess import PIPE, Popen + + +def popen_wrapper(args): + """ + Friendly wrapper around Popen. + + Returns stdout output, stderr output and OS status code. + """ + p = Popen(args, shell=False, stdout=PIPE, stderr=PIPE, + close_fds=os.name != 'nt', universal_newlines=True) + output, errors = p.communicate() + return output, errors, p.returncode diff --git a/tests/i18n/commands/compilation.py b/tests/i18n/commands/compilation.py index c15b95eb0e..705ed63de5 100644 --- a/tests/i18n/commands/compilation.py +++ b/tests/i18n/commands/compilation.py @@ -99,3 +99,22 @@ class MultipleLocaleCompilationTests(MessageCompilationTests): self.assertTrue(os.path.exists(self.MO_FILE_HR)) self.assertTrue(os.path.exists(self.MO_FILE_FR)) + + +class CompilationErrorHandling(MessageCompilationTests): + + LOCALE='ja' + MO_FILE='locale/%s/LC_MESSAGES/django.mo' % LOCALE + + def setUp(self): + super(CompilationErrorHandling, self).setUp() + self.addCleanup(self._rmfile, os.path.join(test_dir, self.MO_FILE)) + + def _rmfile(self, filepath): + if os.path.exists(filepath): + os.remove(filepath) + + def test_error_reported_by_msgfmt(self): + os.chdir(test_dir) + with self.assertRaises(CommandError): + call_command('compilemessages', locale=self.LOCALE, stderr=StringIO()) diff --git a/tests/i18n/commands/locale/ja/LC_MESSAGES/django.po b/tests/i18n/commands/locale/ja/LC_MESSAGES/django.po new file mode 100644 index 0000000000..08d6be1e63 --- /dev/null +++ b/tests/i18n/commands/locale/ja/LC_MESSAGES/django.po @@ -0,0 +1,21 @@ +# SOME DESCRIPTIVE TITLE. +# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER +# This file is distributed under the same license as the PACKAGE package. +# FIRST AUTHOR , YEAR. +# +msgid "" +msgstr "" +"Project-Id-Version: PACKAGE VERSION\n" +"Report-Msgid-Bugs-To: \n" +"POT-Creation-Date: 2011-12-04 04:59-0600\n" +"PO-Revision-Date: 2013-02-26 21:29-0300\n" +"Last-Translator: FULL NAME \n" +"Language-Team: LANGUAGE \n" +"Language: ja\n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: 8bit\n" +"Plural-Forms: nplurals=1; plural=0;\n" + +#, brainfuck-format +msgwhat!? "This is an invalid PO file. GNU msgfmt should reject it." diff --git a/tests/i18n/tests.py b/tests/i18n/tests.py index c9abe01f8c..4d06c85c97 100644 --- a/tests/i18n/tests.py +++ b/tests/i18n/tests.py @@ -41,7 +41,8 @@ if can_run_extraction_tests: MultipleLocaleExtractionTests) if can_run_compilation_tests: from .commands.compilation import (PoFileTests, PoFileContentsTests, - PercentRenderingTests, MultipleLocaleCompilationTests) + PercentRenderingTests, MultipleLocaleCompilationTests, + CompilationErrorHandling) from .contenttypes.tests import ContentTypeTests from .forms import I18nForm, SelectDateForm, SelectDateWidget, CompanyForm from .models import Company, TestModel