Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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.
commit dfa9324966ce1a38346d15e35805d042848aabf1 1 parent 5c51d71
Ramiro Morales authored February 12, 2013
24  django/core/management/commands/compilemessages.py
@@ -2,9 +2,10 @@
2 2
 
3 3
 import codecs
4 4
 import os
5  
-import sys
6 5
 from optparse import make_option
  6
+
7 7
 from django.core.management.base import BaseCommand, CommandError
  8
+from django.core.management.utils import popen_wrapper
8 9
 from django.utils._os import npath
9 10
 
10 11
 def has_bom(fn):
@@ -41,18 +42,15 @@ def compile_messages(stderr, locale=None):
41 42
                     if has_bom(fn):
42 43
                         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)
43 44
                     pf = os.path.splitext(fn)[0]
44  
-                    # Store the names of the .mo and .po files in an environment
45  
-                    # variable, rather than doing a string replacement into the
46  
-                    # command, so that we can take advantage of shell quoting, to
47  
-                    # quote any malicious characters/escaping.
48  
-                    # See http://cyberelk.net/tim/articles/cmdline/ar01s02.html
49  
-                    os.environ['djangocompilemo'] = npath(pf + '.mo')
50  
-                    os.environ['djangocompilepo'] = npath(pf + '.po')
51  
-                    if sys.platform == 'win32': # Different shell-variable syntax
52  
-                        cmd = 'msgfmt --check-format -o "%djangocompilemo%" "%djangocompilepo%"'
53  
-                    else:
54  
-                        cmd = 'msgfmt --check-format -o "$djangocompilemo" "$djangocompilepo"'
55  
-                    os.system(cmd)
  45
+                    program = 'msgfmt'
  46
+                    args = [program, '--check-format', '-o', npath(pf + '.mo'), npath(pf + '.po')]
  47
+                    output, errors, status = popen_wrapper(args)
  48
+                    if status:
  49
+                        if errors:
  50
+                            msg = "Execution of %s failed: %s" % (program, errors)
  51
+                        else:
  52
+                            msg = "Execution of %s failed" % program
  53
+                        raise CommandError(msg)
56 54
 
57 55
 
58 56
 class Command(BaseCommand):
14  django/core/management/utils.py
... ...
@@ -0,0 +1,14 @@
  1
+import os
  2
+from subprocess import PIPE, Popen
  3
+
  4
+
  5
+def popen_wrapper(args):
  6
+    """
  7
+    Friendly wrapper around Popen.
  8
+
  9
+    Returns stdout output, stderr output and OS status code.
  10
+    """
  11
+    p = Popen(args, shell=False, stdout=PIPE, stderr=PIPE,
  12
+              close_fds=os.name != 'nt', universal_newlines=True)
  13
+    output, errors = p.communicate()
  14
+    return output, errors, p.returncode
19  tests/i18n/commands/compilation.py
@@ -99,3 +99,22 @@ def test_multiple_locales(self):
99 99
 
100 100
         self.assertTrue(os.path.exists(self.MO_FILE_HR))
101 101
         self.assertTrue(os.path.exists(self.MO_FILE_FR))
  102
+
  103
+
  104
+class CompilationErrorHandling(MessageCompilationTests):
  105
+
  106
+    LOCALE='ja'
  107
+    MO_FILE='locale/%s/LC_MESSAGES/django.mo' % LOCALE
  108
+
  109
+    def setUp(self):
  110
+        super(CompilationErrorHandling, self).setUp()
  111
+        self.addCleanup(self._rmfile, os.path.join(test_dir, self.MO_FILE))
  112
+
  113
+    def _rmfile(self, filepath):
  114
+        if os.path.exists(filepath):
  115
+            os.remove(filepath)
  116
+
  117
+    def test_error_reported_by_msgfmt(self):
  118
+        os.chdir(test_dir)
  119
+        with self.assertRaises(CommandError):
  120
+            call_command('compilemessages', locale=self.LOCALE, stderr=StringIO())
21  tests/i18n/commands/locale/ja/LC_MESSAGES/django.po
... ...
@@ -0,0 +1,21 @@
  1
+# SOME DESCRIPTIVE TITLE.
  2
+# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
  3
+# This file is distributed under the same license as the PACKAGE package.
  4
+# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
  5
+#
  6
+msgid ""
  7
+msgstr ""
  8
+"Project-Id-Version: PACKAGE VERSION\n"
  9
+"Report-Msgid-Bugs-To: \n"
  10
+"POT-Creation-Date: 2011-12-04 04:59-0600\n"
  11
+"PO-Revision-Date: 2013-02-26 21:29-0300\n"
  12
+"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
  13
+"Language-Team: LANGUAGE <LL@li.org>\n"
  14
+"Language: ja\n"
  15
+"MIME-Version: 1.0\n"
  16
+"Content-Type: text/plain; charset=UTF-8\n"
  17
+"Content-Transfer-Encoding: 8bit\n"
  18
+"Plural-Forms: nplurals=1; plural=0;\n"
  19
+
  20
+#, brainfuck-format
  21
+msgwhat!? "This is an invalid PO file. GNU msgfmt should reject it."
3  tests/i18n/tests.py
@@ -41,7 +41,8 @@
41 41
         MultipleLocaleExtractionTests)
42 42
 if can_run_compilation_tests:
43 43
     from .commands.compilation import (PoFileTests, PoFileContentsTests,
44  
-        PercentRenderingTests, MultipleLocaleCompilationTests)
  44
+        PercentRenderingTests, MultipleLocaleCompilationTests,
  45
+        CompilationErrorHandling)
45 46
 from .contenttypes.tests import ContentTypeTests
46 47
 from .forms import I18nForm, SelectDateForm, SelectDateWidget, CompanyForm
47 48
 from .models import Company, TestModel

0 notes on commit dfa9324

Please sign in to comment.
Something went wrong with that request. Please try again.