gh-135321: Changing data type of size variable for BINSTRING in _pickle

[Legoclones]

I changed the data type to int from Py_ssize_t for the size variable. This variable is also used as an argument to 3 other functions but should be auto-casted (similar to load_counted_long). I also changed the error message to match the error message in the Python load_binstring() function.

I would like to add tests for this specific case, but the payload that would show whether or not it works as intended requires a 2GB large pickle, and I'm not sure if you'd like that to be in the tests. Let me know what you think.

• Issue: Pickle BINSTRING incorrect data type for size #135321

[serhiy-storchaka]

The bug was introduced in bpo-17810/gh-62010. The code should use calc_binsize() instead of calc_binsize().

And it is worth to add a test:

diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py
index 9d6ae3e4d00..2a85e31078c 100644
--- a/Lib/test/pickletester.py
+++ b/Lib/test/pickletester.py
@@ -1100,6 +1100,11 @@ def test_large_32b_binunicode8(self):
         self.check_unpickling_error((pickle.UnpicklingError, OverflowError),
                                     dumped)
 
+    def test_large_binstring(self):
+        errmsg = 'UnpicklingError: BINSTRING pickle has negative byte count'
+        with self.assertRaisesRegex(pickle.UnpicklingError, errmsg):
+            self.loads(b'T\0\0\0\x80')
+
     def test_get(self):
         pickled = b'((lp100000\ng100000\nt.'
         unpickled = self.loads(pickled)

[Legoclones]

I made all the requested changes - thank you!

[serhiy-storchaka]

LGTM.

[miss-islington-app]

Thanks @Legoclones for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

[bedevere-app]

GH-135382 is a backport of this pull request to the 3.14 branch.

[bedevere-app]

GH-135383 is a backport of this pull request to the 3.13 branch.