Jens Geyer created THRIFT-6271:
----------------------------------
Summary: C++: readAll on a concrete TWebSocketServer bypasses
WebSocket framing
Key: THRIFT-6271
URL: https://issues.apache.org/jira/browse/THRIFT-6271
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
The inheritance chain is {{TWebSocketServer}} -> {{THttpServer}} ->
{{THttpTransport}} -> {{TVirtualTransport<THttpTransport>}}.
{{TWebSocketServer}} ({{lib/cpp/src/thrift/transport/TWebSocketServer.h:54}})
overrides {{readAll_virt()}} ({{:63}}), which is what does the handshake and
the frame decoding. It does not define a non-virtual {{readAll()}}, so the one
it inherits is {{TVirtualTransport<THttpTransport>::readAll()}} -- and that one
is templated on {{THttpTransport}}, so it reads through
{{THttpTransport::read}} and never sees a WebSocket frame.
The result is that the same object behaves differently depending on the static
type of the pointer:
* through a {{TTransport}} / {{shared_ptr<TTransport>}} -- {{readAll_virt()}}
dispatches, WebSocket semantics
* through a {{TWebSocketServer*}} -- the inherited non-virtual {{readAll()}} is
found, HTTP semantics
In the shipped code path this does not bite, because the transport factory
hands out {{shared_ptr<TTransport>}}. {{TVirtualTransport.h}} already names the
shape in a comment on that very method:
{quote}subclasses that use TVirtualTransport to derive from another transport
implementation (i.e., not TTransportDefaults) should beware that this may
override any non-default readAll() implementation provided by the parent
transport class. They may need to redefine readAll() to call the correct parent
implementation, if desired.{quote}
which is exactly this case. It is recorded because it is easy to hit when
writing a test against the concrete type.
h2. Suggested
Give {{TWebSocketServer}} a non-virtual {{readAll()}} that forwards to its own
{{readAll_virt()}}, as the comment above suggests, so the concrete type behaves
like the abstract one.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
