<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 23, 2017, at 6:59 AM, Adam Petcher <<a href="mailto:adam.petcher@oracle.com" class="">adam.petcher@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class=""><p class="">Comments below.<br class="">
</p>
<br class="">
<div class="moz-cite-prefix">On 1/21/2017 11:02 PM, Mandy Chung
wrote:<br class="">
</div>
<blockquote cite="mid:072845C6-A2FE-4AD5-B43E-2B5E478037DB@oracle.com" type="cite" class="">
<pre wrap="" class=""></pre>
<blockquote type="cite" class="">
<pre wrap="" class="">On Jan 21, 2017, at 6:37 PM, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a> wrote:
On 01/22/2017 09:18 AM, Mandy Chung wrote:
</pre>
<blockquote type="cite" class="">
<pre wrap="" class="">AFAIK, no permission check from RB::getBundle loading this resource bundle. The implementation should wrap all security sensitive calls with doPriv. I also mentioned that in [1]
</pre>
</blockquote>
<pre wrap="" class="">I see.
It just feels strange to see getString() and getAuthResourcesString() implemented so differently in this webrev. Since you think they should be the same, how about creating a private method that includes the VM.initLevel and bundles.computeIfAbsent calls? We'll let Adam to decide if getString() can also call it.
</pre>
</blockquote>
<pre wrap="" class="">I agree it looks strange but I hope Adam can leverage that. It’s better to leave it for the fix for JDK-8168075.</pre>
</blockquote>
<br class="">
Thanks. I've updated <a class="issue-link" data-issue-key="JDK-8172808" href="https://bugs.openjdk.java.net/browse/JDK-8172808" id="key-val" rel="4912272">JDK-8172808</a> to indicate that there
is some potential for refactoring here. <br class="">
<br class="">
Though it seems like there is an issue with ResourceMgr::getString
in your latest diff. The bundle is loaded, but it is not stored in
the map (unless I'm missing it). So the resource bundle would be
loaded for every call to this method.<br class=""></div></div></blockquote><br class=""></div><div>Good catch. I missed that. The good things is that ResourceBundle is cached after it’s loaded via getBundle method and there should be no loss of performance. I’ll fix it and should probably take JDK-8172808.</div><div><br class=""></div><div>Mandy</div><br class=""></body></html>