Details

      Description

      Following suit with the Java Random implementations, it'd be better if we switched CryptoCodec#generateSecureRandom to take a byte[] for parity.

      Also, let's document that this method needs to be thread-safe, which is an important consideration for CryptoCodec implementations.

      1. HADOOP-10713.002.patch
        3 kB
        Andrew Wang
      2. HADOOP-10713.001.patch
        1 kB
        Andrew Wang

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        5d 2h 20m 1 Yi Liu 22/Jun/14 08:21
        Yi Liu made changes -
        Fix Version/s 3.0.0 [ 12320357 ]
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s fs-encryption (HADOOP-10150 and HDFS-6134) [ 12326860 ]
        Hide
        Yi Liu added a comment -

        Committed to branch.

        Show
        Yi Liu added a comment - Committed to branch.
        Hide
        Yi Liu added a comment -

        +1, thanks Andrew.

        Show
        Yi Liu added a comment - +1, thanks Andrew.
        Hide
        Charles Lamb added a comment -

        +1

        Show
        Charles Lamb added a comment - +1
        Andrew Wang made changes -
        Attachment HADOOP-10713.002.patch [ 12651253 ]
        Hide
        Andrew Wang added a comment -

        Thanks Yi, new patch attached. I went ahead and refactored generateSecureRandom to be more like Random#nextBytes.

        Show
        Andrew Wang added a comment - Thanks Yi, new patch attached. I went ahead and refactored generateSecureRandom to be more like Random#nextBytes.
        Andrew Wang made changes -
        Description Random implementations have to deal with thread-safety; this should be specified in the javadoc so implementors know to do this for CryptoCodec subclasses. Following suit with the Java Random implementations, it'd be better if we switched CryptoCodec#generateSecureRandom to take a byte[] for parity.

        Also, let's document that this method needs to be thread-safe, which is an important consideration for CryptoCodec implementations.
        Andrew Wang made changes -
        Summary Document thread-safety of CryptoCodec#generateSecureRandom Refactor CryptoCodec#generateSecureRandom to take a byte[]
        Hide
        Yi Liu added a comment -

        Great, thanks Andrew Wang, this can save some allocations.
        +1 to use byte[] as parameter.

        Show
        Yi Liu added a comment - Great, thanks Andrew Wang , this can save some allocations. +1 to use byte[] as parameter.
        Andrew Wang made changes -
        Attachment HADOOP-10713.001.patch [ 12650729 ]
        Hide
        Andrew Wang added a comment -

        Trivial patch attached. While I was looking at this, I also wondered whether this API should be like SecureRandom#nextBytes(byte[]), taking a byte[], rather than taking an int numBytes and returning a new array. Yi Liu any thoughts? It might save us some allocations.

        Show
        Andrew Wang added a comment - Trivial patch attached. While I was looking at this, I also wondered whether this API should be like SecureRandom#nextBytes(byte[]) , taking a byte[] , rather than taking an int numBytes and returning a new array. Yi Liu any thoughts? It might save us some allocations.
        Andrew Wang made changes -
        Component/s security [ 12312526 ]
        Andrew Wang made changes -
        Field Original Value New Value
        Fix Version/s 3.0.0 [ 12320357 ]
        Andrew Wang created issue -

          People

          • Assignee:
            Andrew Wang
            Reporter:
            Andrew Wang
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development