Match behavior between Python and C base 85 functions

As we're now keeping the existing Python base 85 functions, the C
implementations should behave exactly the same, down to exception
type and wording. It is also no longer an error to try to decode
data of length 1 mod 5.

Author: kangtastic

Lib/_base64.py

@@ -1,33 +1,57 @@
 """C accelerator wrappers for originally pure-Python parts of base64."""
 
-from binascii import a2b_ascii85, a2b_base85, b2a_ascii85, b2a_base85
+from binascii import Error, a2b_ascii85, a2b_base85, b2a_ascii85, b2a_base85
+from base64 import _bytes_from_decode_data, bytes_types
 
-__all__ = ['a85encode', 'a85decode',
-           'b85encode', 'b85decode',
-           'z85encode', 'z85decode']
 
+# Base 85 encoder functions in base64 silently convert input to bytes.
+def _bytes_from_encode_data(b):
+    return b if isinstance(b, bytes_types) else memoryview(b).tobytes()
 
-def a85encode(b, *, foldspaces=False, wrapcol=0, pad=False, adobe=False):
+
+# Functions in binascii raise binascii.Error instead of ValueError.
+def raise_valueerror(func):
+    def _func(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except Error as e:
+            raise ValueError(e) from None
+    return _func
+
+
+@raise_valueerror
+def _a85encode(b, *, foldspaces=False, wrapcol=0, pad=False, adobe=False):
+    b = _bytes_from_encode_data(b)
     return b2a_ascii85(b, fold_spaces=foldspaces,
                        wrap=adobe, width=wrapcol, pad=pad)
 
 
-def a85decode(b, *, foldspaces=False, adobe=False, ignorechars=b' \t\n\r\v'):
+@raise_valueerror
+def _a85decode(b, *, foldspaces=False, adobe=False, ignorechars=b' \t\n\r\v'):
+    b = _bytes_from_decode_data(b)
     return a2b_ascii85(b, fold_spaces=foldspaces,
                        wrap=adobe, ignore=ignorechars)
 
 
-def b85encode(b, pad=False):
+@raise_valueerror
+def _b85encode(b, pad=False):
+    b = _bytes_from_encode_data(b)
     return b2a_base85(b, pad=pad, newline=False)
 
 
-def b85decode(b):
+@raise_valueerror
+def _b85decode(b):
+    b = _bytes_from_decode_data(b)
     return a2b_base85(b, strict_mode=True)
 
 
-def z85encode(s):
+@raise_valueerror
+def _z85encode(s):
+    s = _bytes_from_encode_data(s)
     return b2a_base85(s, newline=False, z85=True)
 
 
-def z85decode(s):
+@raise_valueerror
+def _z85decode(s):
+    s = _bytes_from_decode_data(s)
     return a2b_base85(s, strict_mode=True, z85=True)

Lib/base64.py

@@ -578,7 +578,21 @@ def decodebytes(s):
 
 # Use accelerated implementations of originally pure-Python parts if possible.
 try:
-    from _base64 import *
+    from _base64 import (_a85encode, _a85decode, _b85encode,
+                         _b85decode, _z85encode, _z85decode)
+    from functools import update_wrapper
+    update_wrapper(_a85encode, a85encode)
+    update_wrapper(_a85decode, a85decode)
+    update_wrapper(_b85encode, b85encode)
+    update_wrapper(_b85decode, b85decode)
+    update_wrapper(_z85encode, z85encode)
+    update_wrapper(_z85decode, z85decode)
+    a85encode = _a85encode
+    a85decode = _a85decode
+    b85encode = _b85encode
+    b85decode = _b85decode
+    z85encode = _z85encode
+    z85decode = _z85decode
 except ImportError:
     pass
 

Lib/test/test_binascii.py

@@ -242,6 +242,21 @@ def test_ascii85_valid(self):
                 res += b
             self.assertEqual(res, rawdata)
 
+        # Test decoding inputs with length 1 mod 5
+        params = [
+            (b"a", False, False, b"", b""),
+            (b"xbw", False, False, b"wx", b""),
+            (b"<~c~>", False, True, b"", b""),
+            (b"{d ~>", False, True, b" {", b""),
+            (b"ye", True, False, b"", b"    "),
+            (b"z\x01y\x00f", True, False, b"\x00\x01", b"\x00\x00\x00\x00    "),
+            (b"<~FCfN8yg~>", True, True, b"", b"test    "),
+            (b"FE;\x03#8zFCf\x02N8yh~>", True, True, b"\x02\x03", b"tset\x00\x00\x00\x00test    "),
+        ]
+        for a, fold_spaces, wrap, ignore, b in params:
+            kwargs = {"fold_spaces": fold_spaces, "wrap": wrap, "ignore": ignore}
+            self.assertEqual(binascii.a2b_ascii85(self.type2test(a), **kwargs), b)
+
     def test_ascii85_invalid(self):
         # Test Ascii85 with invalid characters interleaved
         lines, i = [], 0
@@ -284,19 +299,16 @@ def _assertRegexTemplate(assert_regex, data, **kwargs):
                 binascii.a2b_ascii85(self.type2test(data), **kwargs)
 
         def assertMissingDelimiter(data):
-            _assertRegexTemplate(r"(?i)end with '~>'", data, wrap=True)
+            _assertRegexTemplate(r"(?i)end with b'~>'", data, wrap=True)
 
         def assertOverflow(data):
-            _assertRegexTemplate(r"(?i)85 overflow", data)
+            _assertRegexTemplate(r"(?i)Ascii85 overflow", data)
 
         def assertInvalidSpecial(data):
             _assertRegexTemplate(r"(?i)'[yz]'.+5-tuple", data, fold_spaces=True)
 
         def assertInvalidChar(data, **kwargs):
-            _assertRegexTemplate(r"(?i)invalid in Ascii85", data, **kwargs)
-
-        def assertInvalidLength(data):
-            _assertRegexTemplate(r"(?i)invalid length", data)
+            _assertRegexTemplate(r"(?i)Non-Ascii85 digit", data, **kwargs)
 
         # Test Ascii85 with missing delimiters
         assertMissingDelimiter(b"")
@@ -331,15 +343,6 @@ def assertInvalidLength(data):
         assertInvalidChar(b"\tFCb", ignore=b"\n")
         assertInvalidChar(b"xxxB\nP\thU'D v/F+", ignore=b" \n\tv")
 
-        # Test Ascii85 with invalid length of final group (1 mod 5)
-        assertInvalidLength(b"a")
-        assertInvalidLength(b"b")
-        assertInvalidLength(b"zc")
-        assertInvalidLength(b"zza")
-        assertInvalidLength(b"!!!!!a")
-        assertInvalidLength(b"+<VdL+<VdLZ")
-        assertInvalidLength(b"Z" * (5 * 43 + 21))
-
     def test_ascii85_width(self):
         # Test Ascii85 splitting lines by width
         def assertEncode(a_expected, data, n, wrap=False):
@@ -411,7 +414,7 @@ def test_ascii85_ignore(self):
         def assertIgnore(data, expected, ignore=b"", **kwargs):
             data = self.type2test(data)
             ignore = self.type2test(ignore)
-            with self.assertRaisesRegex(binascii.Error, r"(?i)invalid in Ascii85"):
+            with self.assertRaisesRegex(binascii.Error, r"(?i)Non-Ascii85 digit"):
                 binascii.a2b_ascii85(data, **kwargs)
             res = binascii.a2b_ascii85(data, ignore=ignore, **kwargs)
             self.assertEqual(res, expected)
@@ -441,6 +444,11 @@ def test_base85_valid(self):
             res += b
         self.assertEqual(res, self.rawdata)
 
+        # Test decoding inputs with length 1 mod 5
+        self.assertEqual(binascii.a2b_base85(self.type2test(b"a")), b"")
+        self.assertEqual(binascii.a2b_base85(self.type2test(b" b ")), b"")
+        self.assertEqual(binascii.a2b_base85(self.type2test(b"b/Y\"*,j'Nc")), b"test")
+
     def test_base85_invalid(self):
         # Test base85 with invalid characters interleaved
         lines, i = [], 0
@@ -480,9 +488,6 @@ def _assertRegexTemplate(assert_regex, data, **kwargs):
         def assertOverflow(data):
             _assertRegexTemplate(r"(?i)base85 overflow", data)
 
-        def assertInvalidLength(data):
-            _assertRegexTemplate(r"(?i)invalid length", data)
-
         # Test base85 with out-of-range encoded value
         assertOverflow(b"}")
         assertOverflow(b"|O")
@@ -492,13 +497,6 @@ def assertInvalidLength(data):
         assertOverflow(b"|NsC0~")
         assertOverflow(b"|NsC0|NsC0|NsD0")
 
-        # Test base85 with invalid length of final group (1 mod 5)
-        assertInvalidLength(b"0")
-        assertInvalidLength(b"1")
-        assertInvalidLength(b"^^^^^^")
-        assertInvalidLength(b"|NsC0|NsC0a")
-        assertInvalidLength(b"_" * (5 * 43 + 21))
-
     def test_base85_pad(self):
         # Test base85 with encode padding
         rawdata = b"n1n3Tee\n ch@rAc\te\r$"
@@ -514,7 +512,7 @@ def test_base85_strict_mode(self):
         # Test base85 with strict mode on
         def assertNonBase85Data(data, expected):
             data = self.type2test(data)
-            with self.assertRaisesRegex(binascii.Error, r"(?i)invalid in base85"):
+            with self.assertRaisesRegex(binascii.Error, r"(?i)bad base85 character"):
                 binascii.a2b_base85(data, strict_mode=True)
             default_res = binascii.a2b_base85(data)
             non_strict_res = binascii.a2b_base85(data, strict_mode=False)

Modules/binascii.c

@@ -709,7 +709,7 @@ binascii_a2b_ascii85_impl(PyObject *module, Py_buffer *data, int fold_spaces,
             state = get_binascii_state(module);
             if (state != NULL) {
                 PyErr_SetString(state->Error,
-                                "Expected Ascii85 data to end with '~>'");
+                                "Ascii85 encoded byte sequences must end with b'~>'");
             }
             return NULL;
         }
@@ -782,7 +782,8 @@ binascii_a2b_ascii85_impl(PyObject *module, Py_buffer *data, int fold_spaces,
         } else if (!ignore_map[this_ch]) {
             state = get_binascii_state(module);
             if (state != NULL) {
-                PyErr_Format(state->Error, "'%c' invalid in Ascii85", this_ch);
+                PyErr_Format(state->Error,
+                             "Non-Ascii85 digit found: %c", this_ch);
             }
             goto error_end;
         }
@@ -792,16 +793,6 @@ binascii_a2b_ascii85_impl(PyObject *module, Py_buffer *data, int fold_spaces,
             continue;
         }
 
-        /* Treat encoded length of 1 mod 5 as an error. */
-        if (ascii_len == -3) {
-            state = get_binascii_state(module);
-            if (state != NULL) {
-                PyErr_SetString(state->Error,
-                                "Ascii85 data has invalid length");
-            }
-            goto error_end;
-        }
-
         /* Write current chunk. */
         chunk_len = ascii_len < 1 ? 3 + ascii_len : 4;
         for (Py_ssize_t i = 0; i < chunk_len; i++) {
@@ -989,8 +980,10 @@ binascii_a2b_base85_impl(PyObject *module, Py_buffer *data, int strict_mode,
                 (leftchar *= 85) > UINT32_MAX - this_digit) {
                 state = get_binascii_state(module);
                 if (state != NULL) {
-                    PyErr_SetString(state->Error,
-                                    z85 ? "z85 overflow" : "base85 overflow");
+                    PyErr_Format(state->Error,
+                                 "%s overflow in hunk starting at byte %d",
+                                 z85 ? "z85" : "base85",
+                                 (data->len - ascii_len) / 5 * 5);
                 }
                 goto error_end;
             }
@@ -999,8 +992,8 @@ binascii_a2b_base85_impl(PyObject *module, Py_buffer *data, int strict_mode,
         } else if (strict_mode) {
             state = get_binascii_state(module);
             if (state != NULL) {
-                PyErr_Format(state->Error, "'%c' %s", this_ch,
-                             z85 ? "invalid in z85" : "invalid in base85");
+                PyErr_Format(state->Error, "bad %s character at position %d",
+                             z85 ? "z85" : "base85", data->len - ascii_len);
             }
             goto error_end;
         }
@@ -1010,17 +1003,6 @@ binascii_a2b_base85_impl(PyObject *module, Py_buffer *data, int strict_mode,
             continue;
         }
 
-        /* Treat encoded length of 1 mod 5 as an error. */
-        if (ascii_len == -3) {
-            state = get_binascii_state(module);
-            if (state != NULL) {
-                PyErr_Format(state->Error,
-                             z85 ? "z85 data has invalid length"
-                                 : "base85 data has invalid length");
-            }
-            goto error_end;
-        }
-
         /* Write current chunk. */
         chunk_len = ascii_len < 1 ? 3 + ascii_len : 4;
         for (Py_ssize_t i = 0; i < chunk_len; i++) {