Commit 06a2d529 authored by Jifan Zhang's avatar Jifan Zhang Committed by Facebook Github Bot

Test and Fix IOBuf Python Iterable on Fragmented Data

Summary:
The Python chained `IOBuf` has undesirable behavior on cyclic pattern data. For example, when we have a data chain with
```
chain = make_chain([IOBuf(b"aaa"), IOBuf(b"aaaa")])
```
`b"".join(chain)` will yield `b"aaa"` rather than `b"aaaaaaa"`.

The root cause to this bug is because in the `__iter__` method of the Python `IOBuf`, the original code checks whether the circular chain has been traversed by the `!=` operator (`self != next`), which has been overridden by `__richcmp__` function. The rich comparator then invokes the comparator in C++, which compares the underlying data in a `IOBuf` chain rather than their reference locations. In the above example, therefore, `chain == chain.next` would return `True`. However, in `__iter__`, in order to check whether the traversal is back to the head of the chain, we should compare by reference rather value.

Reviewed By: yfeldblum

Differential Revision: D16589600

fbshipit-source-id: 3b03c4d502bdc385edca3502949be03440543a21
parent 21680794
...@@ -115,7 +115,7 @@ cdef class IOBuf: ...@@ -115,7 +115,7 @@ cdef class IOBuf:
"Iterates through the chain of buffers returning a memory view for each" "Iterates through the chain of buffers returning a memory view for each"
yield memoryview(self, PyBUF_C_CONTIGUOUS) yield memoryview(self, PyBUF_C_CONTIGUOUS)
next = self.next next = self.next
while next is not None and next != self: while next is not None and next is not self:
yield memoryview(next, PyBUF_C_CONTIGUOUS) yield memoryview(next, PyBUF_C_CONTIGUOUS)
next = next.next next = next.next
......
...@@ -29,6 +29,18 @@ class IOBufTests(unittest.TestCase): ...@@ -29,6 +29,18 @@ class IOBufTests(unittest.TestCase):
self.assertEqual(memoryview(chain.next), control[1]) # type: ignore self.assertEqual(memoryview(chain.next), control[1]) # type: ignore
self.assertEqual(b''.join(chain), b''.join(control)) self.assertEqual(b''.join(chain), b''.join(control))
def test_cyclic_chain(self) -> None:
control = [b'aaa', b'aaaa']
chain = make_chain([IOBuf(x) for x in control])
self.assertTrue(chain.is_chained)
self.assertTrue(chain)
self.assertEqual(bytes(chain), control[0])
self.assertEqual(len(chain), len(control[0]))
self.assertEqual(chain.chain_size(), sum(len(x) for x in control))
self.assertEqual(chain.chain_count(), len(control))
self.assertEqual(memoryview(chain.next), control[1]) # type: ignore
self.assertEqual(b''.join(chain), b''.join(control))
def test_hash(self) -> None: def test_hash(self) -> None:
x = b"omg" x = b"omg"
y = b"wtf" y = b"wtf"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment