Hi Chris, JDK-8037781 will be sent to code review as soon as JDK-8035158 (this) is closed.
Thanks. Pavel On 21 Mar 2014, at 09:59, Chris Hegarty <chris.hega...@oracle.com> wrote: > Thanks for doing this Pavel. The changes, and test, look good to me. I can > sponsor this for you. > > I assume that once this changes is pushed, JDK-8037781: "Remove > sun.misc.Regexp* classes", can proceed. > > -Chris. > > On 19/03/14 18:03, Pavel Rappo wrote: >> Hi everyone, >> >> could you please review my change for JDK-8035158? >> >> DefaultProxySelector uses sun.misc.RegexpPool to parse properties that >> configure the proxy settings. This code should be changed to use >> java.util.regex public API so that the classes in sun.misc.Regex* can be >> removed. >> >> http://cr.openjdk.java.net/~chegar/8035158/webrev.00/ >> >> The main idea was to get rid of dependency on private regex engine and >> to use public API which is available since jdk4 -- java.util.regex.Pattern. >> >> Thanks >> Pavel >> >> P.S. During this refactoring I may have found an erroneous behavior. In >> the current implementation when a duplicate disjunct is added into >> http.nonProxyHosts (as well as into ftp.nonProxyHosts) property, >> sun.misc.RegexpPool throws an exception: >> >> throw new java.misc.REException(re + " is a duplicate"); >> >> Later while slicing pattern into a sequence of disjuncts >> sun.net.spi.DefaultProxySelector silently swallows this exception: >> >> StringTokenizer st = new StringTokenizer(nphosts, "|", false); >> try { >> while (st.hasMoreTokens()) { >> pool.add(st.nextToken().toLowerCase(), Boolean.TRUE); >> } >> } catch (sun.misc.REException ex) {} // *** >> >> That leads to a situation where resulting pattern is broken. Moreover >> the final result depends on a position of the duplicate in the property >> -- as the while loop abrupted as a whole, not just a single iteration. >> >> In my case I was testing with http.nonProxyHosts=localhost when noticed >> this issue. In the list of default loopback aliases localhost happens to >> sit first: >> >> String defStringVal = "localhost|127.*|[::1]|0.0.0.0|[::0]; >> >> So when you set a value of http.nonProxyHosts to a "localhost", the only >> host you'll be able to go directly to will be that localhost. Because >> nothing else will have chance to be added into the pool after the >> exception is thrown. >> >> That's the only behavior I've modified by now.