Repository: python/cpython

Issue:
msgfmt cannot cope with BOM - improve error message
BPO | [1697943](https://bugs.python.org/issue1697943)
--- | :---
Nosy | @loewis, @rhettinger, @Cito, @vstinner, @merwok, @serhiy-storchaka
Files | <li>[msgfmt.diff](https://bugs.python.org/file2348/msgfmt.diff "Uploaded as text/plain at 2007-04-10.20:58:04 by @Cito")</li>

<sup>*Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.*</sup>

<details><summary>Show more details</summary><p>

GitHub fields:
```python
assignee = 'https://github.com/loewis'
closed_at = None
created_at = <Date 2007-04-10.20:58:04.000>
labels = ['type-bug', 'expert-unicode', '3.11']
title = 'msgfmt cannot cope with BOM - improve error message'
updated_at = <Date 2021-04-22.15:26:29.553>
user = 'https://github.com/Cito'
```

bugs.python.org fields:
```python
activity = <Date 2021-04-22.15:26:29.553>
actor = 'iritkatriel'
assignee = 'loewis'
closed = False
closed_date = None
closer = None
components = ['Demos and Tools', 'Unicode']
creation = <Date 2007-04-10.20:58:04.000>
creator = 'cito'
dependencies = []
files = ['2348']
hgrepos = []
issue_num = 1697943
keywords = ['patch']
message_count = 9.0
messages = ['31755', '31756', '31757', '31758', '70042', '125940', '125941', '290519', '290524']
nosy_count = 6.0
nosy_names = ['loewis', 'rhettinger', 'cito', 'vstinner', 'eric.araujo', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1697943'
versions = ['Python 3.11']
```

</p></details>


<!-- gh-linked-prs -->
### Linked PRs
* gh-130187
<!-- /gh-linked-prs -->

Discussion:
@Cito: If a .po file has a BOM (byte order mark) at the beginning, as is often the case for utf-8 files created on Windows, msgfmt.py complines about a syntax error.

The attached patch fixes this problem.@rhettinger: 
Martin, is this your code?@loewis: 
It's my code, but I will need to establish first whether it's a bug. That depends on what the PO specification says, and, if is it silent on the matter, what GNU gettext does.@Cito: 
It may well be that GNU gettext also chokes on a BOM, because they aren't used under Linux. But I think as a Python tool it should be more Windows-tolerant. The annoying thing is that you get a syntax error, but everything looks right because the BOM is usually invisible. Such error messages are really frustrating. Either the BOM should be silently ignored (as in the patch) or the users should get a friendly error message asking them to save the file without BOM. If GNU behaves badly to Windows users, that's not an excuse to do the same. They are already suffering enough because of their (or their bosses') bad choice of OS ;-)

@Cito: Small improvement of the patch: Instead of hardcoding the BOM as
'\\xef\\xbb\\xbf', we should use codecs.BOM_UTF8.@vstinner: Extract of the Unicode standard: "Use of a BOM is neither required nor recommended for UTF-8, but may be encountered in contexts where UTF-8 data is converted from other encoding forms that use a BOM or where the BOM is used as a UTF-8 signature".

See also the following section explaing issues with UTF-8 BOM:
http://en.wikipedia.org/wiki/Byte_order_mark#UTF-8

I agree that Python should handle (UTF-8) BOM to read a CSV file (bpo-7185), because the file format is common on Windows.

But msgfmt is an UNIX tool: I would expect that Python behaves like the original msgfmt tool, fail with a fatal error on the BOM "invisible character". How do you explain to a user msgfmt fails but not msgfmt.py?

About the patch: \*ignore* the BOM is not a good idea. The BOM announces the encoding (eg. UTF-8): if a Content-Type header announces another encoding, you should raise an error.@vstinner: See also issue bpo-7651: "Python3: guess text file charset using the BOM".@serhiy-storchaka: Corresponding GNU gettext issue [1] was closed as "Not a Bug".

[1] https://savannah.gnu.org/bugs/?18345@Cito: > Corresponding GNU gettext issue [1] was closed as "Not a Bug".

Though I think the rationale given there pointing to RFC3629 section 6 is wrong, since that section explicitly refers to Internet protocols, but PO files are not an Internet protocol.

Anyway, if silently ignoring BOMs is considered a bad idea, then at least there should be a more helpful error message. Because the BOM is invisible, users - who may not even be aware that something like a BOM exist or that their editor saves files with BOM - may be frustrated about the current error message because they don't see any invalid character when they open the PO file in their editor. A more explicit error message like "PO files should not be saved with a byte order mark" might point users in the right direction.

After all, these tools are supposed to be used directly by human beings on the command line. Who said that command line tools must not be user friendly?@StanFromIreland: @picnixz You can also unassign "61337411-43fc-4a9c-b8d5-4060aede66d0"@serhiy-storchaka: `msgfmt` now produces clearer error message. An alternative -- accepting BOM -- is a part of larger and more complex issue. We could accept BOM in a number of other formats, but should we? For example, there are XML file with BOM in the wild (just search for XML BOM, there are a lot of questions how to handle them), but BOM is not a part of XML format, no one general tool accept XML with BOM, they are considered invalid. If Python will start to ignore BOM in some files, it will allow to pass invalid files which will not be accepted by other programs, and it may fail to detect some errors earlier.

Solution patch:
diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst
index d914acbbc67..e0e7286214a 100644
--- a/Doc/library/dis.rst
+++ b/Doc/library/dis.rst
@@ -97,23 +97,23 @@ The following options are accepted:
 
 .. program:: dis
 
-.. cmdoption:: -h, --help
+.. option:: -h, --help
 
    Display usage and exit.
 
-.. cmdoption:: -C, --show-caches
+.. option:: -C, --show-caches
 
    Show inline caches.
 
-.. cmdoption:: -O, --show-offsets
+.. option:: -O, --show-offsets
 
    Show offsets of instructions.
 
-.. cmdoption:: -P, --show-positions
+.. option:: -P, --show-positions
 
    Show positions of instructions in the source code.
 
-.. cmdoption:: -S, --specialized
+.. option:: -S, --specialized
 
    Show specialized bytecode.
 
diff --git a/Modules/_lzmamodule.c b/Modules/_lzmamodule.c
index 97f3a8f03da..0058e2eec2e 100644
--- a/Modules/_lzmamodule.c
+++ b/Modules/_lzmamodule.c
@@ -126,6 +126,9 @@ typedef struct {
     PyThread_type_lock lock;
 } Decompressor;
 
+#define Compressor_CAST(op)     ((Compressor *)(op))
+#define Decompressor_CAST(op)   ((Decompressor *)(op))
+
 /* Helper functions. */
 
 static int
@@ -857,14 +860,15 @@ Compressor_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
 }
 
 static void
-Compressor_dealloc(Compressor *self)
+Compressor_dealloc(PyObject *op)
 {
+    Compressor *self = Compressor_CAST(op);
     lzma_end(&self->lzs);
     if (self->lock != NULL) {
         PyThread_free_lock(self->lock);
     }
     PyTypeObject *tp = Py_TYPE(self);
-    tp->tp_free((PyObject *)self);
+    tp->tp_free(self);
     Py_DECREF(tp);
 }
 
@@ -875,7 +879,7 @@ static PyMethodDef Compressor_methods[] = {
 };
 
 static int
-Compressor_traverse(Compressor *self, visitproc visit, void *arg)
+Compressor_traverse(PyObject *self, visitproc visit, void *arg)
 {
     Py_VISIT(Py_TYPE(self));
     return 0;
@@ -1304,8 +1308,9 @@ _lzma_LZMADecompressor_impl(PyTypeObject *type, int format,
 }
 
 static void
-Decompressor_dealloc(Decompressor *self)
+Decompressor_dealloc(PyObject *op)
 {
+    Decompressor *self = Decompressor_CAST(op);
     if(self->input_buffer != NULL)
         PyMem_Free(self->input_buffer);
 
@@ -1315,12 +1320,12 @@ Decompressor_dealloc(Decompressor *self)
         PyThread_free_lock(self->lock);
     }
     PyTypeObject *tp = Py_TYPE(self);
-    tp->tp_free((PyObject *)self);
+    tp->tp_free(self);
     Py_DECREF(tp);
 }
 
 static int
-Decompressor_traverse(Decompressor *self, visitproc visit, void *arg)
+Decompressor_traverse(PyObject *self, visitproc visit, void *arg)
 {
     Py_VISIT(Py_TYPE(self));
     return 0;
@@ -1633,7 +1638,7 @@ lzma_clear(PyObject *module)
 static void
 lzma_free(void *module)
 {
-    lzma_clear((PyObject *)module);
+    (void)lzma_clear((PyObject *)module);
 }
 
 static PyModuleDef _lzmamodule = {
diff --git a/Tools/i18n/msgfmt.py b/Tools/i18n/msgfmt.py
index 3f731e941ea..f005c4e7b5b 100755
--- a/Tools/i18n/msgfmt.py
+++ b/Tools/i18n/msgfmt.py
@@ -32,9 +32,11 @@
 import struct
 import array
 from email.parser import HeaderParser
+import codecs
 
 __version__ = "1.2"
 
+
 MESSAGES = {}
 
 
@@ -116,6 +118,14 @@ def make(filename, outfile):
         print(msg, file=sys.stderr)
         sys.exit(1)
 
+    if lines[0].startswith(codecs.BOM_UTF8):
+        print(
+            f"The file {infile} starts with a UTF-8 BOM which is not allowed in .po files.\n"
+            "Please save the file without a BOM and try again.",
+            file=sys.stderr
+        )
+        sys.exit(1)
+
     section = msgctxt = None
     fuzzy = 0
 


Test patch:
diff --git a/Lib/test/test_tools/test_msgfmt.py b/Lib/test/test_tools/test_msgfmt.py
index e3e3035c4f4..a6073b8be03 100644
--- a/Lib/test/test_tools/test_msgfmt.py
+++ b/Lib/test/test_tools/test_msgfmt.py
@@ -39,6 +39,14 @@ def test_compilation(self):
 
                     self.assertDictEqual(actual._catalog, expected._catalog)
 
+    def test_po_with_bom(self):
+        with temp_cwd():
+            Path('bom.po').write_bytes(b'\xef\xbb\xbfmsgid "Python"\nmsgstr "Pioton"\n')
+
+            res = assert_python_failure(msgfmt, 'bom.po')
+            err = res.err.decode('utf-8')
+            self.assertIn('The file bom.po starts with a UTF-8 BOM', err)
+
     def test_invalid_msgid_plural(self):
         with temp_cwd():
             Path('invalid.po').write_text('''\