Don't send non-Thrift exception messages to client #187
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BACKGROUND
Two kinds of exceptions can be thrown by a handler processing a Thrift request:
exceptions
in a .thrift file. The server serializes them and sends them to the client. Clients should expect them to happen from time to time and should handle them gracefully.TApplicationException
to the client containing the message from the exception.THIS CHANGE
Stop sending the exception message to the client. I think this is a bad idea from a security standpoint. Random exceptions raised by the server could potentially contain sensitive info like passwords. One hopes this never happens, of course, but we have no control over the code people write. It's not reasonable default behavior to send those messages to clients. The client could be untrusted or the transport could be insecure.
It could be argued that a flag should be added to enable the server to send these messages to clients. I think that's a reasonable thing to do (but I don't plan to do it).
I updated both the Apache code and the thrift-server code, and the tests for both.
Here's the diff of a
ping
function generated for thrift-server before and after this change: