<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hello Eirik,</p>
    <p>Looking at the commit history, that comment was added when, very
      long back, the JarFileFactory used to return a
      java.security.Permission for the cached JarFile. That code was
      then changed but the comment was left around which I believe was
      an oversight.</p>
    <p>Like you note, in its current form it is not accurate. It can be
      removed. If we do introduce a new comment for that block (which I
      think we should), then something like the following might be
      useful:</p>
    <p>/* <br>
       * If we are using a cached jarFile, which could have been added
      to the cache<br>
       * by someone else, then set our jarFileURLConnection to the one
      from the cache<br>
       * and update it to use the same value for useCaches as the one we
      had determined<br>
       * in our constructor.<br>
       */</p>
    <p>-Jaikiran<br>
    </p>
    <div class="moz-cite-prefix">On 20/11/24 10:38 pm, Eirik Bjørsnøs
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CA+pBWhuc4-pxFjV2Zh_yiPj9W6eOSFd7m6yNv0Pp0pqetCbG+Q@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">Hi,<br>
        <div><br>
        </div>
        <div>At the end of s.n.w.p.j.JarURLConnection::connect, I see
          this stray, permission related comment:</div>
        <div><br>
        </div>
        <blockquote class="gmail_quote"
style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><font
            face="monospace">/* we also ask the factory the permission
            that was required<br>
             * to get the jarFile, and set it as our permission.<br>
             */<br>
            if (useCaches) {<br>
                boolean oldUseCaches =
            jarFileURLConnection.getUseCaches();<br>
                jarFileURLConnection = factory.getConnection(jarFile);<br>
                jarFileURLConnection.setUseCaches(oldUseCaches);<br>
            }</font></blockquote>
        <div><br>
        </div>
        <div>
          <div>The "factory" here is JarFileFactory, which recently went
            through a SM-cleanup PR which removed permission checking.</div>
        </div>
        <div><br>
        </div>
        <div>The comment seems to have existed since the initial load,
          but have moved around a bit since then.</div>
        <div><br>
        </div>
        <div>It does not seem to make a lot of sense now, and digging
          through history I'm also struggling a bit to understand how it
          made sense even in the initial load.</div>
        <div><br>
        </div>
        <div>What's the best action here? Delete the comment? Replace it
          with something more appropriate? Wait for pending JEP486
          cleanup to take care of it?</div>
        <div><br>
        </div>
        <div>Thanks,</div>
        <div>Eirik.</div>
      </div>
    </blockquote>
  </body>
</html>