Ughh, I hate supporting PyPy

Codecov Report

Merging #573 (15bfae0) into main (1860724) will increase coverage by 0.14%.
The diff coverage is 92.85%.

❗ Current head 15bfae0 differs from pull request most recent head efb514b. Consider uploading reports for the commit efb514b to get more accurate results

@@            Coverage Diff             @@
##             main     #573      +/-   ##
==========================================
+ Coverage   91.49%   91.64%   +0.14%     
==========================================
  Files           6        6              
  Lines        1905     1938      +33     
==========================================
+ Hits         1743     1776      +33     
  Misses        162      162              

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Looks like this upstream issue might be related: https://foss.heptapod.net/pypy/pypy/-/issues/3520

Hmm, I'd have expected this line to fail too if it was the same cause since bytes() would have returned the original unsliced bytes string:

I'm just going to create a new ticket.

Filed upstream: https://foss.heptapod.net/pypy/pypy/-/issues/3872

How do we want to handle this?

1. Merging this PR without adding some kind of additional safeguard for PyPy sounds to me like another CVE in the making.
2. Cancelling this PR just because of PyPy feels silly.
3. Adding if PyPy: use old logic / else use new logic is both ugly from the developer's perspective (all the fiddly remember to dec-ref/free this object in all successful and erroneous cases would double) and potentially confusing to users who may wonder why ujson happily reads bytearray() under CPython but not PyPy.
4. Any better ideas?

How about an extra test inside the bytes-like case for PyPy, testing specifically for bytes and bytearray? That keeps the error condition code minimal, and it'd still add support for bytearray on PyPy. We basically have to exclude anything that isn't guaranteed to be C-contiguous (which bytes and bytearray are, I think, although I doubt it's documented anywhere) until that bug is fixed.

  bool is_bytes_like = !PyObject_GetBuffer(arg, &buffer, PyBUF_C_CONTIGUOUS);
  if (is_bytes_like)
  {
    #ifdef PYPY_VERSION
      // PyPy's buffer protocol implementation is buggy: https://foss.heptapod.net/pypy/pypy/-/issues/3872
      if (!PyBytes_Check(arg) && !PyByteArray_Check(arg)) {
        PyBuffer_Release(&buffer);
        PyErr_Format(PyExc_TypeError, "Expected string, bytes, or bytearray object");
        return NULL;
      }
    #endif
    raw = buffer.buf;
    sarg_length = buffer.len;
  }

Not sure what we'd want to do once the bug is fixed in PyPy though. Maybe we could change that to a version check, either in the code or somewhere in the build process, but not sure that's the best approach.