[
https://issues.apache.org/jira/browse/THRIFT-4405?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
James E. King III updated THRIFT-4405:
--------------------------------------
Description:
The following tests were added:
* The cpp client for cross test now uses a sequence ID that starts at INT32_MAX
- 10, and advances until it wraps around.
* The cpp client verifies the sequence ID that returns matches what was sent.
* The python client was changed to use pedantic sequence ID checking on all
protocols.
The following errors were identified and fixed:
* In c_glib, thrift_stored_message_protocol was limiting the sequence ID to
[0..G_INTMAX]. This was changed to allow any 32-bit value, matching other
implementations.
* In cpp, JSON protocol, when the server read the header, it used a uint64_t
for processing; this interacted with a bugfix from 2017 that dropped
boost::lexical_cast and switched to std::stringstream, and this corrupted the
negative sequence IDs.
* In python, compact protocol, a negative sequence ID was not handled properly.
Documentation was added for sequence number handling.
was:
Create a feature test that verifies sequence numbers are used properly. Write
a server that verifies clients are generating unique sequence IDs. Write a
client that makes sure servers return the same sequence ID that was given.
To do this, I enhanced the C++ TProcessorEventHandler class to include a
preReadSeq, which is like preRead but carries the sequence ID.
In the C++ TestServer, I check to see if the sequence numbers are unique and do
not repeat; if any of them do, the cpp test fails.
The following languages properly send sequence IDs (for the binary protocol):
* dart
* go
* nodejs
* java
* rs
The rest of the languages do not. Now, one could argue that unless a language
has a concurrent-safe client and server, sequence IDs are unnecessary. While
that is true, all languages should respect that the protocol has a sequence ID
and there could be future implementations that will require all clients are
well-behaved, which is why I am putting this test in.
Languages fixed up so unique sequence IDs are sent by the client, and verified
by the tests:
* cpp (was only sending unique sequence IDs for Concurrent clients, now it does
for the regular one too)
* csharp (seqid_ was not bring incremented with each use)
* lua (seqid_ was not bring incremented with each use)
* perl (seqid_ was not bring incremented with each use)
* ruby (seqid_ was not bring incremented with each use and a unit test was
updated to no longer be pending)
Languages left to do:
* c_glib
* erlang
* haskell
* php
* python
* python3
* any non-cross tested languages
> Incorrect handling of sequence numbers that wrap to negative
> ------------------------------------------------------------
>
> Key: THRIFT-4405
> URL: https://issues.apache.org/jira/browse/THRIFT-4405
> Project: Thrift
> Issue Type: Improvement
> Components: Test Suite
> Affects Versions: 0.11.0
> Environment: docker ubuntu-xenial
> Reporter: James E. King III
> Assignee: James E. King III
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The following tests were added:
> * The cpp client for cross test now uses a sequence ID that starts at
> INT32_MAX - 10, and advances until it wraps around.
> * The cpp client verifies the sequence ID that returns matches what was sent.
> * The python client was changed to use pedantic sequence ID checking on all
> protocols.
> The following errors were identified and fixed:
> * In c_glib, thrift_stored_message_protocol was limiting the sequence ID to
> [0..G_INTMAX]. This was changed to allow any 32-bit value, matching other
> implementations.
> * In cpp, JSON protocol, when the server read the header, it used a uint64_t
> for processing; this interacted with a bugfix from 2017 that dropped
> boost::lexical_cast and switched to std::stringstream, and this corrupted the
> negative sequence IDs.
> * In python, compact protocol, a negative sequence ID was not handled
> properly.
> Documentation was added for sequence number handling.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
