On Thu, 28 Apr 2022 06:31:30 GMT, Jaikiran Pai wrote:
> More of a FYI - the CheckCleanerBound test failed on one of the test setups.
> So I've created https://bugs.openjdk.java.net/browse/JDK-8285785 to track
> that failure.
Thank you! I will add the sleep back.
-
PR: https://gi
On Wed, 27 Apr 2022 16:22:38 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
On Wed, 27 Apr 2022 16:22:38 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Wed, 27 Apr 2022 16:02:04 GMT, Roger Riggs wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> remove trailing whitespace
>
> test/jdk/javax/security/auth/callback/PasswordCallback/CheckCleanerBound.java
> li
On Wed, 27 Apr 2022 16:02:05 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Wed, 27 Apr 2022 13:44:14 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> rename and split the test case
>
> test/jdk/javax/security/auth/callback/PasswordCallback/PasswordCleanup.java
>
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Tue, 26 Apr 2022 16:04:14 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
On Tue, 26 Apr 2022 15:45:47 GMT, Xue-Lei Andrew Fan wrote:
>> Ok, then I would suggest changing the name of the test as it is misleading.
>> I suggest creating a directory named "PasswordCallback" and then adding a
>> test named perhaps "CheckCleanerNotBoundToThis" or something like that. I
>
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Tue, 26 Apr 2022 15:19:30 GMT, Sean Mullan wrote:
>> The test case is used to check that the Cleaner used is not bind to 'this'
>> object, and the cleaner during finalization could work. Unfortunately, as
>> the cleaner behavior is not visible, I don't find a way to automated test
>> that
On Tue, 26 Apr 2022 04:27:37 GMT, Xue-Lei Andrew Fan wrote:
>> test/jdk/javax/security/auth/callback/PasswordCleanup.java line 74:
>>
>>> 72: }
>>> 73:
>>> 74: private static void checkClearing() throws Exception {
>>
>> How is this test testing that the password is cleared?
>
> The te
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Mon, 25 Apr 2022 20:41:47 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Code clean up per feedback
>
> test/jdk/javax/security/auth/callback/PasswordCleanup.java line 58:
>
>> 56: }
On Mon, 25 Apr 2022 20:37:38 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Code clean up per feedback
>
> test/jdk/javax/security/auth/callback/PasswordCleanup.java line 83:
>
>> 81:
On Thu, 21 Apr 2022 06:55:22 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
On Mon, 25 Apr 2022 05:48:04 GMT, Xue-Lei Andrew Fan wrote:
> > However, I think that we need to carefully check the interactions between
> > cleaners and methods that explicitly allow the contents to be cleared so
> > that there are not unexpected results.
>
> I think @RogerRiggs explained th
On Wed, 20 Apr 2022 19:30:11 GMT, Sean Mullan wrote:
> However, I think that we need to carefully check the interactions between
> cleaners and methods that explicitly allow the contents to be cleared so that
> there are not unexpected results.
I think @RogerRiggs explained the behavior of Cle
On Thu, 21 Apr 2022 06:50:58 GMT, Xue-Lei Andrew Fan wrote:
>> I'd recommend setting `cleanable` to null after it's been cleaned to make
>> the state machine easier to reason about. The invariant would be: if
>> `cleanable` is non-null, then we have something dirty that needs to be
>> cleaned.
On Wed, 20 Apr 2022 23:38:59 GMT, Stuart Marks wrote:
>> Calling `Cleanable.clean()` calls the `Runnable` action at-most-once.
>> Each `Cleanable` inserted in a list when it is created and is removed when
>> `clear()` or `clean()` is invoked.
>> The action is called only if it is still in the li
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Wed, 20 Apr 2022 21:09:54 GMT, Roger Riggs wrote:
>>> > Won't there be a small performance hit (perhaps negligible) for code that
>>> > already calls clearPassword?
>>>
>>> The impact should be minimal. If clearPassword() has been called, the
>>> cleanup (Cleanerable.clean()) won't happen a
On Wed, 20 Apr 2022 19:59:16 GMT, Sean Mullan wrote:
>>> Won't there be a small performance hit (perhaps negligible) for code that
>>> already calls clearPassword?
>>
>> The impact should be minimal. If clearPassword() has been called, the
>> cleanup (Cleanerable.clean()) won't happen again
On Tue, 19 Apr 2022 15:21:49 GMT, Xue-Lei Andrew Fan wrote:
> > Won't there be a small performance hit (perhaps negligible) for code that
> > already calls clearPassword?
>
> The impact should be minimal. If clearPassword() has been called, the cleanup
> (Cleanerable.clean()) won't happen agai
On Tue, 19 Apr 2022 15:03:05 GMT, Xue-Lei Andrew Fan wrote:
> > I am not quite seeing the rationale for this change.
>
> There were a lot of effort to clean up the buffering password in JDK. In some
> circumstances, the PasswordCallback would be called for further using of the
> password. Howe
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Tue, 19 Apr 2022 14:35:37 GMT, Xue-Lei Andrew Fan wrote:
>> Yes, I suppose that is a good enough reason, although this class never had a
>> finalizer AFAIK. Won't there be a small performance hit (perhaps negligible)
>> for code that already calls `clearPassword`? A specification clarificati
On Mon, 18 Apr 2022 17:45:22 GMT, Sean Mullan wrote:
> I am not quite seeing the rationale for this change.
There were a lot of effort to clean up the buffering password in JDK. In some
circumstances, the PasswordCallback would be called for further using of the
password. However, because th
On Mon, 18 Apr 2022 20:04:05 GMT, Sean Mullan wrote:
>> Yes, exactly. I'd recommend it calling `cleanable.clean()` prior to storing
>> the new password, so that the cleaning action for the old password is
>> explicitly and immediately unregistered.
>
> Yes, I suppose that is a good enough reaso
On Mon, 18 Apr 2022 19:27:21 GMT, Stuart Marks wrote:
>> I think its "belt and suspenders".
>> If the caller does not call `clearPassword` before calling a second
>> `setPassword,` the previous char array will still contain the previous
>> password and remain uncleared in memory for a (longer)
On Mon, 18 Apr 2022 19:10:44 GMT, Roger Riggs wrote:
>> What about code that is already calling `clearPassword` between calls to
>> `setPassword`? This seems to be a change in the design of this API. The
>> `clearPassword` method is there to allow callers to manage and clear the
>> passwords i
On Mon, 18 Apr 2022 18:15:25 GMT, Sean Mullan wrote:
>> setPassword can/should always clear the previous password. It is an
>> internal copy that no one else has a reference to and is being replaced.
>> It will need to either explicitly call cleanable.clean() or fill/erase the
>> array itself.
On Mon, 18 Apr 2022 18:07:59 GMT, Roger Riggs wrote:
>> I can see why that might be a good idea. Would require a specification
>> change though. I also think it is fine to keep the behavior the same, and
>> place the responsibility on the application to call `clearPassword` before
>> setting a
On Mon, 18 Apr 2022 17:27:04 GMT, Sean Mullan wrote:
>> src/java.base/share/classes/javax/security/auth/callback/PasswordCallback.java
>> line 123:
>>
>>> 121: cleanable = CleanerFactory.cleaner().register(
>>> 122: this, cleanerFor(inputPassword));
>>> 123:
On Mon, 18 Apr 2022 15:21:18 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
On Mon, 18 Apr 2022 16:39:36 GMT, Stuart Marks wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update test case
>
> src/java.base/share/classes/javax/security/auth/callback/PasswordCallback.java
> line 123:
On Mon, 18 Apr 2022 15:21:18 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
On Mon, 18 Apr 2022 14:33:35 GMT, Roger Riggs wrote:
> Please combine these closely related cases into a single test, preferably
> with TestNG or JUnit.
I normally prefer to have one job in one test, so that it is easier for
debugging. I'm fine to combine them as the cases are simple enough.
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registered
> for cleanup. An application could call clearPassword() fo
On Sat, 16 Apr 2022 15:45:21 GMT, Xue-Lei Andrew Fan wrote:
> Please review this password cleanup enhancement in the PasswordCallback
> implementation. This is one of the effort to clean up the buffered passwords.
>
> The PasswordCallback.setPassword() clones the password, but is not registere
Please review this password cleanup enhancement in the PasswordCallback
implementation. This is one of the effort to clean up the buffered passwords.
The PasswordCallback.setPassword() clones the password, but is not registered
for cleanup. An application could call clearPassword() for the purp
43 matches
Mail list logo
