facebook / fbthrift

Facebook's branch of Apache Thrift, including a new C++ server.
Apache License 2.0
2.55k stars 608 forks source link

Does Thrift really work on L7 with `HTTP2RoutingHandler` and `HTTPClientChannel` #535

Closed wenhaocs closed 1 year ago

wenhaocs commented 1 year ago

I tried to run thrift sever using HTTP support. After some change, I tried to sniff the network packets, but couldn't find any http packets. Here are what I did. First I added the missing .cpp files to thrift/lib/cpp2/CMakeLists.txt(tags/v2021.11.29.00), including:

async/HTTPClientChannel.cpptransport/http2/client/H2ClientConnection.cpp
transport/http2/client/ThriftTransactionHandler.cp
transport/http2/common/H2Channel.cpp
transport/http2/common/HTTP2RoutingHandler.cpp
transport/http2/common/SingleRpcChannel.cpp
transport/http2/server/ThriftRequestHandler.cpp

Then I tried to implement a simple thrift server and client based on: https://github.com/facebook/fbthrift/blob/main/thrift/example/cpp2/server/ExampleServer.cpp https://github.com/facebook/fbthrift/blob/main/thrift/example/cpp2/client/ChatRoomClient.cpp

I can successfully send and receive messages. In the example client, it's using RocketClientChannel. I also tried changing it to apache::thrift::HTTPClientChannel::newHTTP2Channel.

Then I run the server and client, and tried to sniff the network packets using tshark: tshark -i lo -f "port 7778" -w /root/test.pcap.

Here are the packets captured:

image

No HTTP packets can be found. Can I check if thrift server is really running on L7 with the changes above?

wenhaocs commented 1 year ago

I find the issue actually is the network sniffer unable to recognize HTTP2 packets. My capture env is ubuntu 20.04 with tcpdump v4.9.3, tshark v3.2.3. My analysis env is Mac M1 with wireshark 4.0.3.

I do find there are prefix of 0x5052 inside raw packets using the thrift example above. According to RFC https://www.rfc-editor.org/rfc/rfc7540#section-3.5, it is the standard HTTP2 packets. But the sniff tools I used just can't recognize it.

yfeldblum commented 1 year ago

Seems like this has been resolved.