Python 3.5.0 (default, Sep 13 2015, 10:33:07) [GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)] on darwinType "help", "copyright", "credits" or "license" for more information.>>> import codecs>>> codecs.escape_encode(b'')Traceback (most recent call last):  File "<stdin>", line 1, in <module>SystemError:Objects/bytesobject.c:3553: bad argument to internal functionI've tested this on Python 3.2 through 3.5.

IMO, the "if (size == 0)" logic should be moved down several lines to avoid introducing a redundant "PyBytes_FromStringAndSize" call.

The patch looks fine to me, but I still wonder how p - PyBytes_AS_STRING(v) can be negative when size == 0...Ah, now I get it: the new size is 0, but the refcount is not 1, since the nullstring is shared. This causes the exception.From _PyBytes_Resize():    if (!PyBytes_Check(v) || Py_REFCNT(v) != 1 || newsize < 0) {        *pv = 0;        Py_DECREF(v);        PyErr_BadInternalCall();        return -1;    }

May be better to test a condition "size > 0" before calling _PyBytes_Resize(), as in many other case where _PyBytes_Resize() is used.Or accept shared objects in _PyBytes_Resize() if new size is equal to old size. This will allow to getrid of additional tests before calling _PyBytes_Resize().

The patch looks sufficient to fix the problem, though I do like Serhiy’s suggestions.For the record, because I was curious: Function codecs.escape_encode() is not documented, and barely tested. It was used for the documented “string_escape” codec in Python 2, but this codec was removed for Python 3 in revisionbc90fc9b70b7. The function was apparently added to support pickling, but I don’t see any evidence that it was ever used. Only the decode counterpart was used. I wonder if the encode function could be removed at some point.

On 01.10.2015 04:35, Martin Panter wrote:> For the record, because I was curious: Function codecs.escape_encode() is not documented, and barely tested. It was used for the documented “string_escape” codec in Python 2, but this codec was removed for Python 3 in revisionbc90fc9b70b7. The function was apparently added to support pickling, but I don’t see any evidence that it was ever used. Only the decode counterpart was used. I wonder if the encode function could be removed at some point.It's a codec, so either we remove both functions or leave bothfunctions in. It's still used in pickletools and serves a usefulpurpose there (to unescape embedded escapes in byte streams).

On 30.09.2015 15:11, Serhiy Storchaka wrote:> May be better to test a condition "size > 0" before calling _PyBytes_Resize(), as in many other case where _PyBytes_Resize() is used.> > Or accept shared objects in _PyBytes_Resize() if new size is equal to old size. This will allow to getrid of additional tests before calling _PyBytes_Resize().Agreed. It would be good to make _PyBytes_Resize() more robust forshared objects.

Here is an updated patch.

Thanks for the review, Serhiy. Here's an updated patch.

New changeset2a4fb01fa1a3 by Berker Peksag in branch '3.5':Issue#25270: Prevent codecs.escape_encode() from raising SystemError when an empty bytestring is passedhttps://hg.python.org/cpython/rev/2a4fb01fa1a3New changeset8a649009a0e9 by Berker Peksag in branch '3.6':Issue#25270: Merge from 3.5https://hg.python.org/cpython/rev/8a649009a0e9New changeset48b55cada2c9 by Berker Peksag in branch 'default':Issue#25270: Merge from 3.6https://hg.python.org/cpython/rev/48b55cada2c9

Thanks for the reviews everyone!