Code Review: 7126889: Incorrect SSLEngine debug output
Xuelei.Fan at Oracle.Com
Thu Jan 26 15:56:28 PST 2012
looks fine to me.
Sent from my iPad
On Jan 27, 2012, at 6:58 AM, Brad Wetmore <bradford.wetmore at oracle.com> wrote:
> On 1/26/2012 2:00 AM, Xuelei Fan wrote:
>> 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
>> int remaining = args.getAppRemaining(); // remaining should be 3;
>> 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.
> As you noticed, that is only in the error case and this EngineArgs instance is going away very soon, but your point is well taken.
>> 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().
> We've been working together a long time. That's a comment I would make if I noticed the same thing! ;) Thanks.
> I also noticed a copy/paste error in the SSLEngineImpl exception code that I've cleaned up.
> This is a minor change, so I'll probably just check in following JPRT.
> If you're interested, the update is in the same location:
> but iteration *.01.
>> 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.
>>> 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.
More information about the security-dev