thrift-server
thrift-server copied to clipboard
thrift-client-ttwitter-filter: Does not handle upgrade failure or connection state
thrift-client-ttwitter-filter appears to incorrectly handle servers that don't support ttwitter (such as the apache node-thrift implementation).
-
https://github.com/creditkarma/thrift-server/blob/c8e9244a49c8635aa2a7594d8578ac6d201adbbb/packages/thrift-client-ttwitter-filter/src/main/ThriftClientTTwitterFilter.ts#L263 does not handle the thrift
EXCEPTION
message type which says that__can__finagle__trace__v3__
does not exist, and treats the connection as upgraded even though it shouldn't be. -
ttwitter upgrades should be done on a per-connection basis. This filter, however, belongs to the TcpConnection not the underlying connections from the connection pool, so it's very possible that the upgrade request could go to one server and the subsequent ttwitter-prefixed messages could go to other connections that haven't been officially upgraded. In principle, a thrift server with a strong internal state machine could refuse to accept ttwitter-prefixed messages if it hasn't gotten an upgrade request.