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 darwin
Type "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 function


I'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 revision bc90fc9b70b7. 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 revision bc90fc9b70b7. 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 both
functions in. It's still used in pickletools and serves a useful
purpose 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 for
shared objects.

Here is an updated patch.

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

New changeset 2a4fb01fa1a3 by Berker Peksag in branch '3.5':
Issue #25270: Prevent codecs.escape_encode() from raising SystemError when an empty bytestring is passed
https://hg.python.org/cpython/rev/2a4fb01fa1a3

New changeset 8a649009a0e9 by Berker Peksag in branch '3.6':
Issue #25270: Merge from 3.5
https://hg.python.org/cpython/rev/8a649009a0e9

New changeset 48b55cada2c9 by Berker Peksag in branch 'default':
Issue #25270: Merge from 3.6
https://hg.python.org/cpython/rev/48b55cada2c9

Thanks for the reviews everyone!

stage: patch review -> resolved

messages: + msg276699

messages: + msg276689
versions: + Python 3.7

nosy: + berker.peksag
versions: + Python 3.6, - Python 3.2, Python 3.3
components: + Extension Modules, - Interpreter Core
keywords: + patch
stage: needs patch -> patch review

stage: needs patch