[jira] [Commented] (HIVE-22841) ThriftHttpServlet#getClientNameFromCookie should handle CookieSigner IllegalArgumentException on invalid cookie signature

["Kevin Risden (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HIVE-22841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17031670#comment-17031670 ] 

Kevin Risden commented on HIVE-22841:
-------------------------------------

Option b seems more reasonable since CookieSigner#verifyAndExtract is only used in one place. It doesn't require changing the signature of CookieSigner#verifyAndExtract. I'm working on a patch now.

As a side note the existing CookieSigner tests don't even check IllegalArgumentException.

> ThriftHttpServlet#getClientNameFromCookie should handle CookieSigner IllegalArgumentException on invalid cookie signature
> -------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HIVE-22841
>                 URL: https://issues.apache.org/jira/browse/HIVE-22841
>             Project: Hive
>          Issue Type: Bug
>          Components: HiveServer2
>            Reporter: Kevin Risden
>            Assignee: Kevin Risden
>            Priority: Major
>
> Currently CookieSigner throws an IllegalArgumentException if the cookie signature is invalid. 
> {code:java}
> if (!MessageDigest.isEqual(originalSignature.getBytes(), currentSignature.getBytes())) {
>       throw new IllegalArgumentException("Invalid sign, original = " + originalSignature +
>         " current = " + currentSignature);
>     }
> {code}
> CookieSigner is only used in the ThriftHttpServlet#getClientNameFromCookie and doesn't handle the IllegalArgumentException. It is only checking if the value from the cookie is null or not.
> https://github.com/apache/hive/blob/master/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java#L295
> {code:java}
>       currValue = signer.verifyAndExtract(currValue);
>       // Retrieve the user name, do the final validation step.
>       if (currValue != null) {
> {code}
> This should be fixed to either:
> a) Have CookieSigner not return an IllegalArgumentException
> b) Improve ThriftHttpServlet to handle CookieSigner throwing an IllegalArgumentException



--
This message was sent by Atlassian Jira
(v8.3.4#803005)