thriftrw / thriftrw-python

74%

master: 92%

LAST BUILD ON BRANCH abg/fix-build

branch: abg/fix-build

CHANGE BRANCH

x

Reset

• abg/fix-build
• 1.2.0
• 1.2.1
• 1.2.2
• 1.2.3
• 1.2.4
• 1.2.5
• 1.3.0
• 1.4.0
• 1.5.0
• 1.5.1
• 1.6.0
• 1.7.0
• 1.7.1
• 1.7.2
• 1.8.0
• 1.8.1
• abg/input-too-short
• abg/response-from-the-future
• abhinav-patch-1
• benchmark-underp
• chen/profile
• decimalfix
• enum-from-the-future
• enum-item-duplicates
• fixbuild
• hash
• license
• master
• message
• more-bench
• mutable-defaults
• no-malloc
• non-strict-exceptions
• opt2
• optread
• release-1.6
• saveopt2
• setup-dont-check-version
• streaming
• streaming-dev
• streaming-read
• strfix
• struct-docfix
• testmasterbuild
• travis-migrate
• validation-opt

Build # 662

Build Type

Pull #143

travis-ci

Committed by web-flow

Commit Message

Fix input too short errors being ignored

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.

Pull Request Pull Request #143: Fix input too short errors being ignored

Run Details

158 of 165 branches covered (95.76%)

8 of 8 new or added lines in 2 files covered. (100.0%)

2174 of 2931 relevant lines covered (74.17%)

0.74 hits per line