thriftrw / thriftrw-python / 664

92%

push

travis-ci

Fix input too short errors being ignored (#143)

The `read_field_begin`, `read_map_begin`, `read_set_begin` and
`read_list_begin` methods of `ProtocolReader` return `cdef struct`s
(which translate to plain C structs) for performance reasons.
Unfortunately, that means that the generated C code does not know
whether an exception occurred while running those functions.

To fix this, we need to add an `except *` clause to these methods so
that cython generates code to check for exceptions and propagate them.

Before, `BinaryProtocolReader.read_field_begin` generated the following
section at the end,

```c
   /* function exit code */
  __pyx_L1_error:;
  __Pyx_WriteUnraisable("thriftrw.protocol.binary.BinaryProtocolReader.read_field_begin", __pyx_clineno, __pyx_lineno, __pyx_filename, 0, 0);
  __pyx_L0:;
  __Pyx_RefNannyFinishContext();
  return __pyx_r;
```

With this change, it generates,

```c
  /* function exit code */
  __pyx_L1_error:;
  __Pyx_AddTraceback("thriftrw.protocol.binary.BinaryProtocolReader.read_field_begin", __pyx_clineno, __pyx_lineno, __pyx_filename);
  __pyx_L0:;
  __Pyx_RefNannyFinishContext();
  return __pyx_r;
```

Note that the `__Pyx_WriteUnraisable` turned to `__Pyx_AddTraceback`.

---

Minor note about testing:

While investigating this, a bug was discovered in the test for the error
path. It is supposed to exercise the error on two code paths but instead
of doing,

    with pytest.raises(...):
        # code path 1

    with pytest.raises(...):
        # code path 2

The test was doing,

    with pytest.raises(...):
        # code path 1
        # code path 2

Which meant that the second code path wasn't actually exercised.

158 of 165 branches covered (95.76%)

2174 of 2931 relevant lines covered (74.17%)

0.74 hits per line