Repository: python-babel/babel
Pull request #1135 (2024-09-25 22:16:30)

Fix of list index out of range error in PoFileParser.add_message when translations is empty
Addresses https://github.com/python-babel/babel/issues/1134

Previously, the `add_message` function would try to access the PoFileParser object's translations list without any checks. However, when an invalid po file is provided, this can lead to an index out of range error.

This PR adds a check to the `_finish_current_message` function and raises an `invalid_pofile` exception when the messages list is populated and translations is empty. This also adds a dummy translation to the translations list to avoid the index out of range error when an `invalid_pofile` exception is treated as a warning rather than raised.

gabe-sherman: I updated the error message and added a test. Let me know if there's any other changes that should be made!
tomasr8: Thanks! I'll take a look later today or tomorrow (ping me in case I forget)
gabe-sherman: Sorry for the delay, it's been a busy couple of days. I just added a couple more test cases!
gabe-sherman: Made those extra changes. Thanks for sticking with me as I work through everything!
tomasr8: > Made those extra changes. Thanks for sticking with me as I work through everything!

Thank _you_ for working on the fix! It's looking good to me but I don't have commit rights in this repo so I'm pinging @akx :slightly_smiling_face: 

Patch:
diff --git a/babel/dates.py b/babel/dates.py
index 3373e06..30321b7 100644
--- a/babel/dates.py
+++ b/babel/dates.py
@@ -1232,7 +1232,7 @@ def parse_date(
 
     format_str = get_date_format(format=format, locale=locale).pattern.lower()
     year_idx = format_str.index('y')
-    month_idx = format_str.index('m')
+    month_idx = format_str.find('m')
     if month_idx < 0:
         month_idx = format_str.index('l')
     day_idx = format_str.index('d')
@@ -1277,7 +1277,7 @@ def parse_time(
 
     # TODO: try ISO format first?
     format_str = get_time_format(format=format, locale=locale).pattern.lower()
-    hour_idx = format_str.index('h')
+    hour_idx = format_str.find('h')
     if hour_idx < 0:
         hour_idx = format_str.index('k')
     min_idx = format_str.index('m')
@@ -1890,6 +1890,14 @@ def match_skeleton(skeleton: str, options: Iterable[str], allow_different_fields
 
     if 'z' in skeleton and not any('z' in option for option in options):
         skeleton = skeleton.replace('z', 'v')
+    if 'k' in skeleton and not any('k' in option for option in options):
+        skeleton = skeleton.replace('k', 'H')
+    if 'K' in skeleton and not any('K' in option for option in options):
+        skeleton = skeleton.replace('K', 'h')
+    if 'a' in skeleton and not any('a' in option for option in options):
+        skeleton = skeleton.replace('a', '')
+    if 'b' in skeleton and not any('b' in option for option in options):
+        skeleton = skeleton.replace('b', '')
 
     get_input_field_width = dict(t[1] for t in tokenize_pattern(skeleton) if t[0] == "field").get
     best_skeleton = None
diff --git a/babel/messages/extract.py b/babel/messages/extract.py
index 8d4bbea..1b2a37f 100644
--- a/babel/messages/extract.py
+++ b/babel/messages/extract.py
@@ -33,7 +33,7 @@ from collections.abc import (
 from functools import lru_cache
 from os.path import relpath
 from textwrap import dedent
-from tokenize import COMMENT, NAME, OP, STRING, generate_tokens
+from tokenize import COMMENT, NAME, NL, OP, STRING, generate_tokens
 from typing import TYPE_CHECKING, Any
 
 from babel.messages._compat import find_entrypoints
@@ -530,7 +530,6 @@ def extract_python(
                 in_def = False
                 continue
             if funcname:
-                message_lineno = lineno
                 call_stack += 1
         elif in_def and tok == OP and value == ':':
             # End of a class definition without parens
@@ -580,11 +579,15 @@ def extract_python(
             elif tok == STRING:
                 val = _parse_python_string(value, encoding, future_flags)
                 if val is not None:
+                    if not message_lineno:
+                        message_lineno = lineno
                     buf.append(val)
 
             # Python 3.12+, see https://peps.python.org/pep-0701/#new-tokens
             elif tok == FSTRING_START:
                 current_fstring_start = value
+                if not message_lineno:
+                    message_lineno = lineno
             elif tok == FSTRING_MIDDLE:
                 if current_fstring_start is not None:
                     current_fstring_start += value
@@ -608,6 +611,9 @@ def extract_python(
                     # for the comment to still be a valid one
                     old_lineno, old_comment = translator_comments.pop()
                     translator_comments.append((old_lineno + 1, old_comment))
+
+            elif tok != NL and not message_lineno:
+                message_lineno = lineno
         elif call_stack > 0 and tok == OP and value == ')':
             call_stack -= 1
         elif funcname and call_stack == -1:
diff --git a/babel/messages/pofile.py b/babel/messages/pofile.py
index 5cd65d8..fa5c859 100644
--- a/babel/messages/pofile.py
+++ b/babel/messages/pofile.py
@@ -247,6 +247,9 @@ class PoFileParser:
 
     def _finish_current_message(self) -> None:
         if self.messages:
+            if not self.translations:
+                self._invalid_pofile("", self.offset, f"missing msgstr for msgid '{self.messages[0].denormalize()}'")
+                self.translations.append([0, _NormalizedString("")])
             self._add_message()
 
     def _process_message_line(self, lineno, line, obsolete=False) -> None:


Test patch:
diff --git a/tests/messages/test_extract.py b/tests/messages/test_extract.py
index 7d3a05a..bcc6aa4 100644
--- a/tests/messages/test_extract.py
+++ b/tests/messages/test_extract.py
@@ -34,6 +34,11 @@ msg7 = _(hello.there)
 msg8 = gettext('Rabbit')
 msg9 = dgettext('wiki', model.addPage())
 msg10 = dngettext(getDomain(), 'Page', 'Pages', 3)
+msg11 = ngettext(
+    "bunny",
+    "bunnies",
+    len(bunnies)
+)
 """)
         messages = list(extract.extract_python(buf,
                                                extract.DEFAULT_KEYWORDS.keys(),
@@ -49,6 +54,7 @@ msg10 = dngettext(getDomain(), 'Page', 'Pages', 3)
             (8, 'gettext', 'Rabbit', []),
             (9, 'dgettext', ('wiki', None), []),
             (10, 'dngettext', (None, 'Page', 'Pages', None), []),
+            (12, 'ngettext', ('bunny', 'bunnies', None), []),
         ]
 
     def test_extract_default_encoding_ascii(self):
@@ -97,10 +103,10 @@ add_notice(req, ngettext("Bar deleted.",
         messages = list(extract.extract_python(buf, ('ngettext', '_'), ['NOTE:'],
 
                                                {'strip_comment_tags': False}))
-        assert messages[0] == (3, 'ngettext', ('Catalog deleted.', 'Catalogs deleted.', None), ['NOTE: This Comment SHOULD Be Extracted'])
+        assert messages[0] == (2, 'ngettext', ('Catalog deleted.', 'Catalogs deleted.', None), ['NOTE: This Comment SHOULD Be Extracted'])
         assert messages[1] == (6, '_', 'Locale deleted.', ['NOTE: This Comment SHOULD Be Extracted'])
         assert messages[2] == (10, 'ngettext', ('Foo deleted.', 'Foos deleted.', None), ['NOTE: This Comment SHOULD Be Extracted'])
-        assert messages[3] == (15, 'ngettext', ('Bar deleted.', 'Bars deleted.', None), ['NOTE: This Comment SHOULD Be Extracted', 'NOTE: And This One Too'])
+        assert messages[3] == (14, 'ngettext', ('Bar deleted.', 'Bars deleted.', None), ['NOTE: This Comment SHOULD Be Extracted', 'NOTE: And This One Too'])
 
     def test_declarations(self):
         buf = BytesIO(b"""\
diff --git a/tests/messages/test_pofile.py b/tests/messages/test_pofile.py
index c0ded12..3609b5c 100644
--- a/tests/messages/test_pofile.py
+++ b/tests/messages/test_pofile.py
@@ -1014,3 +1014,20 @@ msgstr ""
 "Language: \n"
 ''')
     assert pofile.read_po(buf).locale is None
+
+
+@pytest.mark.parametrize("case", ['msgid "foo"', 'msgid "foo"\nmsgid_plural "foos"'])
+@pytest.mark.parametrize("abort_invalid", [False, True])
+def test_issue_1134(case: str, abort_invalid: bool):
+    buf = StringIO(case)
+
+    if abort_invalid:
+        # Catalog not created, aborted with PoFileError
+        with pytest.raises(pofile.PoFileError) as excinfo:
+            pofile.read_po(buf, abort_invalid=True)
+        assert str(excinfo.value) == "missing msgstr for msgid 'foo' on 0"
+    else:
+        # Catalog is created with warning, no abort
+        output = pofile.read_po(buf)
+        assert len(output) == 1
+        assert output["foo"].string in ((''), ('', ''))
diff --git a/tests/test_dates.py b/tests/test_dates.py
index 31ac782..549929b 100644
--- a/tests/test_dates.py
+++ b/tests/test_dates.py
@@ -17,7 +17,7 @@ import freezegun
 import pytest
 
 from babel import Locale, dates
-from babel.dates import NO_INHERITANCE_MARKER, UTC, _localize
+from babel.dates import NO_INHERITANCE_MARKER, UTC, _localize, parse_pattern
 from babel.util import FixedOffsetTimezone
 
 
@@ -619,6 +619,19 @@ def test_format_skeleton(timezone_getter):
     assert (dates.format_skeleton('EHm', dt, tzinfo=timezone_getter('Asia/Bangkok'), locale='th') == 'อา. 22:30 น.')
 
 
+@pytest.mark.parametrize(('skeleton', 'expected'), [
+    ('Hmz', 'Hmv'),
+    ('kmv', 'Hmv'),
+    ('Kmv', 'hmv'),
+    ('Hma', 'Hm'),
+    ('Hmb', 'Hm'),
+    ('zkKab', 'vHh'),
+])
+def test_match_skeleton_alternate_characters(skeleton, expected):
+    # https://github.com/unicode-org/icu/blob/5e22f0076ec9b55056cd8a84e9ef370632f44174/icu4j/main/core/src/main/java/com/ibm/icu/text/DateIntervalInfo.java#L1090-L1102
+    assert dates.match_skeleton(skeleton, (expected,)) == expected
+
+
 def test_format_timedelta():
     assert (dates.format_timedelta(timedelta(weeks=12), locale='en_US')
             == '3 months')
@@ -666,6 +679,24 @@ def test_parse_time(input, expected):
     assert dates.parse_time(input, locale='en_US') == expected
 
 
+def test_parse_time_alternate_characters(monkeypatch):
+    # 'K' can be used as an alternative to 'H'
+    def get_time_format(*args, **kwargs):
+        return parse_pattern('KK:mm:ss')
+    monkeypatch.setattr(dates, 'get_time_format', get_time_format)
+
+    assert dates.parse_time('9:30') == time(9, 30)
+
+
+def test_parse_date_alternate_characters(monkeypatch):
+    # 'l' can be used as an alternative to 'm'
+    def get_date_format(*args, **kwargs):
+        return parse_pattern('yyyy-ll-dd')
+    monkeypatch.setattr(dates, 'get_time_format', get_date_format)
+
+    assert dates.parse_date('2024-10-20') == date(2024, 10, 20)
+
+
 @pytest.mark.parametrize('case', ['', 'a', 'aaa'])
 @pytest.mark.parametrize('func', [dates.parse_date, dates.parse_time])
 def test_parse_errors(case, func):
diff --git a/tests/test_numbers.py b/tests/test_numbers.py
index a96bdbe..5e6ee01 100644
--- a/tests/test_numbers.py
+++ b/tests/test_numbers.py
@@ -52,7 +52,6 @@ class FormatDecimalTestCase(unittest.TestCase):
         """
         assert numbers.format_decimal(5.5, '0', locale='sv') == '6'
         assert numbers.format_decimal(6.5, '0', locale='sv') == '6'
-        assert numbers.format_decimal(6.5, '0', locale='sv') == '6'
         assert numbers.format_decimal(1.2325, locale='sv') == '1,232'
         assert numbers.format_decimal(1.2335, locale='sv') == '1,234'