'Re: RFE Review : JDK-5016517 - Replace plaintext passwords by hashed passwords for out-of-the-box JM'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFE Review : JDK-5016517 - Replace plaintext passwords by hashed passwords for out-of-the-box JM
From:       mandy chung <mandy.chung () oracle ! com>
Date:       2017-10-31 17:07:52
Message-ID: a5314a16-041a-6e74-c12a-e47e1db2356c () oracle ! com
[Download RAW message or body]

On 10/31/17 8:55 AM, Harsha Wardhana B wrote:
>
> Hi Mandy,
>
> Below is the new webrev incorporating below review comments.
>
> http://cr.openjdk.java.net/~hb/5016517/webrev.06/

Looks okay in general except this:

  286         // Check if header needs to be inserted
  287         if (sbuf.indexOf("# The passwords in this file are hashed") != 0) {
  288             String lastUpdated = "# file last updated on - "
  289                     + new SimpleDateFormat("MM/dd/yyyy HH:mm:ss").format(new Date()) + "\n\n";
  290             sbuf.insert(0, header + lastUpdated);
  291         }

Relying on matching the partial header string is fragile.
Also the timestamp is not updated if the file contains such
heading but the file is re-written again.

You should probably drop the header (auto-inserted), not add
it to sbuf, and always add the header when updating the
password file.

Mandy


[Attachment #3 (text/html)]

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    <br>
    <div class="moz-cite-prefix">On 10/31/17 8:55 AM, Harsha Wardhana B
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:d5a1aa0a-654d-700a-f2f7-cd078d9474d0@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <p>Hi Mandy,</p>
      <p>Below is the new webrev incorporating below review comments.</p>
      <a class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Ehb/5016517/webrev.06/"
        moz-do-not-send="true">http://cr.openjdk.java.net/~hb/5016517/webrev.06/</a><br>
    </blockquote>
    <br>
    Looks okay in general except this:<br>
    <pre> 286         // Check if header needs to be inserted
 287         if (sbuf.indexOf("# The passwords in this file are hashed") != 0) {
 288             String lastUpdated = "# file last updated on - "
 289                     + new SimpleDateFormat("MM/dd/yyyy HH:mm:ss").format(new Date()) + "\n\n";
 290             sbuf.insert(0, header + lastUpdated);
 291         }

Relying on matching the partial header string is fragile.
Also the timestamp is not updated if the file contains such
heading but the file is re-written again.

You should probably drop the header (auto-inserted), not add
it to sbuf, and always add the header when updating the 
password file.

Mandy

</pre>
  </body>
</html>


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic