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

Colin Patrick McCabe commented on HADOOP-10735:
-----------------------------------------------

{code}
      StringTokenizer parser = new StringTokenizer(codecString, ",");
      while (parser.hasMoreElements()) {
        String c = parser.nextToken().trim();
        if (!c.isEmpty()) {
{code}

Seems kind of verbose.  How about just:
{code}
for (String c : 
Splitter.on(',').trimResults().omitEmptyStrings().split(codecString)) {
    ...
}
{code}

{code}
            if (!CryptoCodec.class.isAssignableFrom(cls)) {
              throw new IllegalArgumentException("Class " + c + 
                  " is not a CryptoCodec.");
            }
            result.add(cls.asSubclass(CryptoCodec.class));
{code}

The {{isAssignableFrom}} part is not necessary.  {{asSubclass}} will throw a 
{{ClassCastException}} if the class can be cast to a subclass as requested.

{code}
  try {
      codec = ReflectionUtils.newInstance(klasses.get(0), conf);
      codec.checkAvailable();
    } catch (Exception e) {
      CryptoCodec fallback = null;
      for (int i = 1; i < klasses.size(); i++) {
        fallback = ReflectionUtils.newInstance(klasses.get(i), conf);
{code}

Don't special-case the first list element.  You can always have a boolean or 
something that controls whether you print out "falling back to X"

{code}
        if (codec.getCipherSuite() == fallback.getCipherSuite()) {
{code}

This doesn't make sense to me.  Let's say I ask for the Foobar2000 codec, but 
you don't have it (the class doesn't exist because you're running an older 
version of Hadoop.)  Then this is going to give you a null pointer exception 
and nothing will work... the fallback fails completely.

It makes more sense to have a configuration key for the cipher suite you're 
using, and then another set of configuration keys that determine which classes 
to use for those configuration keys.

It seems to me that clients are not going to want fallback to a different 
cipher suite, if there is nothing available for the current cipher suite.  They 
would not be able to read other clients' files, or write files that other 
clients could handle, when using a different cipher suite.  Clients they just 
want fallback between different implementations of the *same* cipher suite.  So 
it makes sense to have separate config keys for "cipher suite" and 
"implementation classes."

> Fall back AesCtrCryptoCodec implementation from OpenSSL to JCE if non native 
> support.
> -------------------------------------------------------------------------------------
>
>                 Key: HADOOP-10735
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10735
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: security
>    Affects Versions: fs-encryption (HADOOP-10150 and HDFS-6134)
>            Reporter: Yi Liu
>            Assignee: Yi Liu
>             Fix For: fs-encryption (HADOOP-10150 and HDFS-6134)
>
>         Attachments: HADOOP-10735.001.patch, HADOOP-10735.002.patch
>
>
> If there is no native support or OpenSSL version is too low not supporting 
> AES-CTR, but {{OpensslAesCtrCryptoCodec}} is configured, we need to fall back 
> it to JCE implementation.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to