Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

thrift-client-ttwitter-filter: Does not handle upgrade failure or connection state #109

Open
jlai opened this issue Jul 29, 2019 · 0 comments

Comments

@jlai
Copy link

jlai commented Jul 29, 2019

thrift-client-ttwitter-filter appears to incorrectly handle servers that don't support ttwitter (such as the apache node-thrift implementation).

  1. 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.

  2. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant