Code Review: 7126889: Incorrect SSLEngine debug output

Xuelei Fan Xuelei.Fan at Oracle.COM
Thu Jan 26 02:00:52 PST 2012


The fix looks fine to me except some minor comments.

1. The copyright year should be 2012 now.

2. EngineArgs.java
     There is a resetPos() method in the class, which will reset 
application positions to original status. In the current fix, the 
"appRemaining" variable is not reset within the resetPos() method. As a 
result, after the call of resetPos(), the value of "appRemaining" 
variable may be not the expected value. For example,
     EngineArgs args = new EngineArgs();  // suppose the appRemaining 
value is 5
     args.gather(2);
     int remaining = args.getAppRemaining(); // remaining should be 3;
     args.resetPos();
     remaining = args.getAppRemaining();    // remaining is expected to be 5
                                                                         
// but indeed the value is 3.

The fix is OK in that in our implementation, once we reset the position 
of an instance of EngineArgs, we will abolish the instance and never use 
it any more.

To avoid any miss-use of this class in the future, I would like to have 
a method comment on resetPos() to talk about the note, or more effort, 
to reserve the original app remaining value and reset appRemaining to  
it in resetPos().

Thanks,
Xuelei

On 1/26/2012 1:15 PM, Brad Wetmore wrote:
> Xuelei (or anyone else available to review this 1-line change),
>
>     7126889: Incorrect SSLEngine debug output
>
> The JSSE debug information is currently reporting one extra byte being 
> written out, but is not actually doing that.  This is just a simple 
> adjustment to correct that error.
>
>     http://cr.openjdk.java.net/~wetmore/7126889
>
> Both jdk8 and jdk7u are there, the fix is identical.
>
> grep'ing System.out debug output in a shell script seemed to be the 
> easiest way to test.  Alan/Michael, let me know if you have a better 
> idea.
>
> The changes will also be pushed into 5/6/6-open in a separate effort.
>
> Brad
>




More information about the security-dev mailing list