On Thu, 11 Sep 2025 06:26:37 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains three commits: >> >> - Merge branch 'master' into 8354469 >> - decouple PassFailJFrame.java change; simplify code flow >> - the fix > > src/java.base/share/classes/sun/security/util/Password.java line 193: > >> 191: ba[bb.position()] = '\n'; >> 192: } >> 193: return ba; > > Some random comments. This is all internal to this class, and it's all > vaguely cleanup, so it's up to you whether to take any action on any of these > points. > > It might be worth having a comment on this method that explains why it's > doing this. The "obvious" way of encoding the characters is to use > String.getBytes(), but you probably don't want to create a String, because (I > think) you want to be able to erase the arrays afterwards. Worth explaining, > if true. > > At lines 190-191 the mixture of buffer and array stuff is kind of confusing. > I think you can check whether bb.remaining() > 0 and use bb.put(). > > There are actually three possibilities for what's in the returned byte array: > 1) it's completely filled with encoded bytes from the input; 2) it contains > the encoded input followed by an NL byte; or 3) it contains the encoded > input, an NL byte, followed by an arbitrary number of zero bytes. The way > that the returned array is processed by the caller handles this, but it seems > a bit brittle. One possibility is always to return an array of the exact > correct length, including an NL byte. Of course this entails an extra copy > (and an extra array to erase) but there are fewer cases for the caller to > handle. Thanks. I'll add some comments and do not mix buffer and array. The `\n` trick as a stop sign will be kept to avoid too many changes. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27196#discussion_r2341920574
