[
https://issues.apache.org/jira/browse/THRIFT-3607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15958889#comment-15958889
]
James E. King, III commented on THRIFT-3607:
--------------------------------------------
I have concerns about sending the e.what() information to the client when an
internal server error occurs. This could be used as an attack vector by
someone to write a malicious client to force error conditions and learn about
the server. I think it is more appropriate for the server to use the
GlobalLogger to send the exception to the log on the server. The client should
simply get an internal error, much like HTTP response code 500 is handled. Web
server operators would not want the details of an internal error to go out to
any client hitting the server. It would expose internal details about the
server and the application.
So what I would suggest is modifying the description for item 3 to say that it
should both log the exception details through GlobalLogger and then send an
opaque INTERNAL_ERROR.
Note that in pull request #1240 the author put details from runtime exceptions
(the what()) into the TApplicationException message, which I feel is a security
issue.
> Unify exception handling policy of TProcessor
> ---------------------------------------------
>
> Key: THRIFT-3607
> URL: https://issues.apache.org/jira/browse/THRIFT-3607
> Project: Thrift
> Issue Type: Improvement
> Components: Wish List
> Reporter: Aki Sukegawa
>
> A discussion in THRIFT-1805 uncovered inconsistent error handling behaviors
> of TProcessors across languages and releases.
> Most outstanding are Java sync and async processors as described there, but
> others are also subtly different in details.
> I propose unifying the TProcessor behavior by specifying mappings from
> "uncaught server handler exceptions" to "observable server behaviors" as
> follows:
> * TApplicationException -> send TApplicationException with the message and
> the type as thrown
> * TTransportException -> the connection is either already broken or newly
> broken
> * other exceptions -> send opaque TApplicationException(INTERNAL_ERROR)
> That way users can still arbitrarily disconnect the client by throwing
> TTransportException.
> (IMO ideally this should have been done by exposing "client context" object
> to the handler instead)
> The first one can be a bit controversial as it can be regarded as information
> leak.
> Also some may prefer unifying the TProcessor behavior to catch-all, so that
> servers never die on handler exceptions.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
