THRIFT-3877: fix py/py3 server, java client with http transport
The java TestClient asks the server to runa oneway request that
sleeps for 3 seconds. If the java TestClient sees the duration
of the call exceed one second, it fails the test. This means the
server did not participate in the "fire and forget" dynamics of
ONEWAY requests. In this case the THttpServer was processing the
RPC before sending the transport response. The fix was to enhance
the TProcessor so that the THttpServer has an opportunity to inspect
the message header before processing the RPC.
This is partly due to the violation of the THttpServer in the
layered architecture. It is essentially implementing a combined
server and transport, whereas there should be a distinct server,
protocol, and transport separation. Many languages seem to have
this problem where HTTP was introduced.
diff --git a/lib/py/src/server/THttpServer.py b/lib/py/src/server/THttpServer.py
index 85cf400..47e817d 100644
--- a/lib/py/src/server/THttpServer.py
+++ b/lib/py/src/server/THttpServer.py
@@ -21,6 +21,7 @@
from six.moves import BaseHTTPServer
+from thrift.Thrift import TMessageType
from thrift.server import TServer
from thrift.transport import TTransport
@@ -32,7 +33,9 @@
to override this behavior (e.g., to simulate a misconfigured or
overloaded web server during testing), it can raise a ResponseException.
The function passed to the constructor will be called with the
- RequestHandler as its only argument.
+ RequestHandler as its only argument. Note that this is irrelevant
+ for ONEWAY requests, as the HTTP response must be sent before the
+ RPC is processed.
"""
def __init__(self, handler):
self.handler = handler
@@ -43,6 +46,9 @@
This class is not very performant, but it is useful (for example) for
acting as a mock version of an Apache-based PHP Thrift endpoint.
+ Also important to note the HTTP implementation pretty much violates the
+ transport/protocol/processor/server layering, by performing the transport
+ functions here. This means things like oneway handling are oddly exposed.
"""
def __init__(self,
processor,
@@ -68,26 +74,45 @@
inputProtocolFactory, outputProtocolFactory)
thttpserver = self
+ self._replied = None
class RequestHander(BaseHTTPServer.BaseHTTPRequestHandler):
def do_POST(self):
# Don't care about the request path.
- itrans = TTransport.TFileObjectTransport(self.rfile)
- otrans = TTransport.TFileObjectTransport(self.wfile)
+ thttpserver._replied = False
+ iftrans = TTransport.TFileObjectTransport(self.rfile)
itrans = TTransport.TBufferedTransport(
- itrans, int(self.headers['Content-Length']))
+ iftrans, int(self.headers['Content-Length']))
otrans = TTransport.TMemoryBuffer()
iprot = thttpserver.inputProtocolFactory.getProtocol(itrans)
oprot = thttpserver.outputProtocolFactory.getProtocol(otrans)
try:
+ thttpserver.processor.on_message_begin(self.on_begin)
thttpserver.processor.process(iprot, oprot)
except ResponseException as exn:
exn.handler(self)
else:
+ if not thttpserver._replied:
+ # If the request was ONEWAY we would have replied already
+ data = otrans.getvalue()
+ self.send_response(200)
+ self.send_header("Content-Length", len(data))
+ self.send_header("Content-Type", "application/x-thrift")
+ self.end_headers()
+ self.wfile.write(data)
+
+ def on_begin(self, name, type, seqid):
+ """
+ Inspect the message header.
+
+ This allows us to post an immediate transport response
+ if the request is a ONEWAY message type.
+ """
+ if type == TMessageType.ONEWAY:
self.send_response(200)
- self.send_header("content-type", "application/x-thrift")
+ self.send_header("Content-Type", "application/x-thrift")
self.end_headers()
- self.wfile.write(otrans.getvalue())
+ thttpserver._replied = True
self.httpd = server_class(server_address, RequestHander)