RFR(S): 8173475 : java/net/HttpURLConnection/SetAuthenticator tests have undeclared dependency on java.logging module
Hi All, Please review a small fix for tests. BugID: https://bugs.openjdk.java.net/browse/JDK-8173475 WebRev: http://cr.openjdk.java.net/~skovalev/8173475/webrev.00/ Issue: Two tests fail in case of usage '--limit-module' command line option. Root cause: One of the files HTTPTest.java has dependency on java.logging module. Second tests HTTPSetAuthenticatorTest.java extends first one. Therefore we need to add dependency declaration to the both tests. -- With best regards, Sergei
Re: RFR(s): 8170864: java/net/URLClassLoader/closetest/CloseTest.java has undeclared dependensies
Hi Colleagues, Thank you for review. Issue: One of networking tests fails in case using a command line option "--limit-module". Root cause: there is undeclared dependency on java.logging. Are you sure of this? The test does not use logging directly, and the HTTP server no longer uses jdk.util.logging ( it uses the system logger ). Logging required for logging handler. If I would not include java.logging I'll get below exception: java.lang.NoClassDefFoundError: java/util/logging/Handler at CloseTest.startHttpServer(CloseTest.java:158) at CloseTest.main(CloseTest.java:67) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:538) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110) at java.base/java.lang.Thread.run(Thread.java:844) that make me sure that java.logging required. I agree with Chris that is strange. It's possible that loading a java.sql class required logging to be present - I believe there's a requires transitive java.logging in the sql module. Now that you have removed that dependency it would be worth checking whether java.logging is still required. Also I am puzzled by: 28 * @library ../../../../com/sun/net/httpserver I wonder what in this test requires anything from this directory which is not a library and only contains httpserver tests? The class FileServerHandler located there. 158HttpContext ctx = httpServer.createContext ( 159 "/", new FileServerHandler(docroot) 160); best regards, -- daniel -- With best regards, Sergei
RFR(s): 8170864: java/net/URLClassLoader/closetest/CloseTest.java has undeclared dependensies
Hi Team, Please review a simple fix for networking test. BugID: https://bugs.openjdk.java.net/browse/JDK-8170864 WebRev: http://cr.openjdk.java.net/~skovalev/8170864/webrev.00/ Issue: One of networking tests fails in case using a command line option "--limit-module". Root cause: there is undeclared dependency on java.logging. Solution: add dependency declaration into jtreg header. The benefit of logging module usage already discussed in a review of similar CR (see: http://mail.openjdk.java.net/pipermail/net-dev/2016-November/010422.html). Also the failing tests uses "java.sql.Array" class to demonstrate class loading ability. There is two ways to resolve the issue: either add dependency declaration on java.sql module or change the Array class by something else. java.sql.Array class isn't important for the test logic and could be easily replaced. It seemed useful to be able to run the test in as many environment as possible. Therefore it is better to eliminate dependency on java.sql module. -- With best regards, Sergei
Re: RFR(s): 8169196: [TESTBUG] Three tests from sun/net/www have undeclared dependencies
Hi Daniel,
All remarks incorporated into:
http://cr.openjdk.java.net/~skovalev/8169196/webrev.01/
Also I've added a note to the
https://bugs.openjdk.java.net/browse/JDK-8038079
--
With best regards,
Sergei
04.11.16 14:20, Daniel Fuchs wrote:
Hi Sergey,
Yes - sorry - I misunderstood the purpose of the NoNTLM test.
The test is supposed to run *only* when NTLM *is not* supported.
When NTLM is supported, it is supposed to return without doing
anything.
Now as it happens, NTLM is supported by default in java.base.
The only case where NTLM is not supported is if some profile
strip down the binaries to exclude the package
sun.net.www.protocol.http.ntlm
(which seems to be the reason why java.base uses reflection to
access the classes in sun.net.www.protocol.http.ntlm).
Basically, this means that NTLM is not supported when
sun.net.www.protocol.http.NTLMAuthenticationProxy.supported == false.
Somehow the test assumes that this will happen if and only if
the java.security.jgss is not linked in (which I think is an obsolete
assertion, because now with --limit-mods and jlink you can exclude
java.security.jgss without necessarily running on a profile
where the binaries have been stripped).
If you add @modules jdk.security.auth to test, then what will happen
is that the test will only run when jdk.security.auth is linked
in, which in turn implies that java.security.jgss is also linked in
because jdk.security.auth requires java.security.jgss.
So the test will either not be run (when jdk.security.auth is
not linked in), or return immediately without doing anything
(when jdk.security.auth is linked in) - so if you add
@modules jdk.security.auth to test, the test serves no purpose
and it's equivalent to simply delete it.
So what you need to do here is replace:
218 // assume NTLM is not supported when Kerberos is not
available
219 try {
220 Class.forName("javax.security.auth.kerberos.KerberosPrincipal");
221 System.out.println("Kerberos is present, assuming
NTLM is supported too");
222 return;
223 } catch (ClassNotFoundException okay) { }
with something that mimics what java.base does to figure whether
NTLM is supported:
load sun.net.www.protocol.http.NTLMAuthenticationProxy,
through reflection (use the 3 args Class.forName to make
sure the static initializers are run), then use reflection
to access the "supported" field of that class, make it
accessible (Field.setAccessible), get its value,
and return if its value is true.
We need to use reflection here because NTLMAuthenticationProxy
is a package class (it is not public).
Then instead of adding @module jdk.security.auth which is a
nonsense, add @module java.base/sun.net.www.protocol.http
to grant the test the power to break in through the strong
encapsulation for testing purposes.
It would also probably be good to log a new defect asking
for this code to be revisited - or drop a note in
https://bugs.openjdk.java.net/browse/JDK-8038079
pointing to this test and mentioning that a better
entry point to figure out whether NTLM is supported or not
might be desirable.
best regards,
-- daniel
On 03/11/16 15:43, Sergei Kovalev wrote:
Daniel,
I case I remove lines 218-223 and disable jdk.security.auth module the
test fails with an Exception:
Expect client to choose: Basic
HTTP/1.1 401 Unauthorized
Content-Length: 0
Connection: close
WWW-Authenticate: Basic realm="wallyworld"
WWW-Authenticate: NTLM
Server received Authorization header: NTLM
TlRMTVNTUAABB4IIAAA=
STDERR:
java.lang.RuntimeException: Unexpected value
at NoNTLM.test(NoNTLM.java:174)
at NoNTLM.main(NoNTLM.java:229)
at
jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@9-ea/Native
Method)
at
jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@9-ea/NativeMethodAccessorImpl.java:62)
at
jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@9-ea/DelegatingMethodAccessorImpl.java:43)
at
java.lang.reflect.Method.invoke(java.base@9-ea/Method.java:537)
at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110)
at java.lang.Thread.run(java.base@9-ea/Thread.java:844)
JavaTest Message: Test threw exception: java.lang.RuntimeException:
Unexpected value
JavaTest Message: shutting down test
Looks like the module also requires for www authentication in other pice
of code.
Hi Sergey,
Great to get rid of yet another shell script.
The change looks good in general - except for NoNTLM.java:
I don't think the test should have @modules jdk.security.auth
as the test is precisely supposed to be able to run without it
(+ lines 218-223 are probably obsolete and I suspect they should
be removed - but that is for another day).
best regards,
-- daniel
On 03/11/16 13:48, Sergei Kovalev wrote:
Hi Team,
Please review one more small fix for m
RFR(s): 8169316: com/sun/net/httpserver tests have undeclared dependency on java.logging
Hi Team, Please review a very small fix for test suite. BugID: https://bugs.openjdk.java.net/browse/JDK-8169316 WebRev: http://cr.openjdk.java.net/~skovalev/8169316/webrev.00/ Issue: bunch of tests have undeclared dependency on java.logging module. This leads the test to fail in case no module present in tested custom JRE. As it was discussed in other review (http://mail.openjdk.java.net/pipermail/net-dev/2016-November/010423.html) we need the logging in the tests and TEST.properties is a better place to describe module dependencies. -- With best regards, Sergei
Re: RFR(s): 8169196: [TESTBUG] Three tests from sun/net/www have undeclared dependencies
Daniel, I case I remove lines 218-223 and disable jdk.security.auth module the test fails with an Exception: Expect client to choose: Basic HTTP/1.1 401 Unauthorized Content-Length: 0 Connection: close WWW-Authenticate: Basic realm="wallyworld" WWW-Authenticate: NTLM Server received Authorization header: NTLM TlRMTVNTUAABB4IIAAA= STDERR: java.lang.RuntimeException: Unexpected value at NoNTLM.test(NoNTLM.java:174) at NoNTLM.main(NoNTLM.java:229) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@9-ea/Native Method) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@9-ea/NativeMethodAccessorImpl.java:62) at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@9-ea/DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(java.base@9-ea/Method.java:537) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110) at java.lang.Thread.run(java.base@9-ea/Thread.java:844) JavaTest Message: Test threw exception: java.lang.RuntimeException: Unexpected value JavaTest Message: shutting down test Looks like the module also requires for www authentication in other pice of code. -- With best regards, Sergei 03.11.16 18:30, Daniel Fuchs wrote: Hi Sergey, Great to get rid of yet another shell script. The change looks good in general - except for NoNTLM.java: I don't think the test should have @modules jdk.security.auth as the test is precisely supposed to be able to run without it (+ lines 218-223 are probably obsolete and I suspect they should be removed - but that is for another day). best regards, -- daniel On 03/11/16 13:48, Sergei Kovalev wrote: Hi Team, Please review one more small fix for module dependencies issue. Bug ID: https://bugs.openjdk.java.net/browse/JDK-8169196 WebRev: http://cr.openjdk.java.net/~skovalev/8169196/webrev.00/index.html Added missed dependency on jdk.httpserver. Also shell test converted to pure java test.
RFR(s): 8169196: [TESTBUG] Three tests from sun/net/www have undeclared dependencies
Hi Team, Please review one more small fix for module dependencies issue. Bug ID: https://bugs.openjdk.java.net/browse/JDK-8169196 WebRev: http://cr.openjdk.java.net/~skovalev/8169196/webrev.00/index.html Added missed dependency on jdk.httpserver. Also shell test converted to pure java test. -- With best regards, Sergei
Re: RFR(s): 8169002: [TESTBUG] Several java/net/httpclient have undeclared dependency on java.logging module
Hi,
Beside of listed in the review tests there are another tests in the
folder. In case I'd update TEST.properties the not listed tests also
will get unnecessary dependency from java.logging.
--
With best regards,
Sergei
On 1 Nov 2016, at 18:40, Roger Riggs <http://mail.openjdk.java.net/mailman/listinfo/net-dev>> wrote:
>//>/Hi, />//>/Ok, leave the logging in. (There are currently 3 issues marked as
intermittent that refer to httpclient). />//>/Roger />//>/p.s. I'm also a fan of using the TEST.properties for test directives
that apply to multiple tests />/in a directory. In this case, "modules = java.logging”. /
If I understand correctly, the list of ‘modules' in TEST.properties is only used
if the test does not explicitly have any @modules tags. So, if you want any
specific exports, or modules, then you need to specify them all.
-Chris.
>//>/On 11/1/2016 2:34 PM, Daniel Fuchs wrote: />>/Hi Roger, />>//>>/I think we agree :-) />>//>>/On 01/11/16 18:01, Roger Riggs wrote: />>>/Hi Daniel, />>>//>>>/It seemed useful to be able to run the test in as many environments as />>>/possible />>>/though realistically java.util.logging may be there too. />>>//>>>/I don't see that setting the logging levels is intrinsic to the tests />>>/and would be used />>>/for debugging so perhaps that function can be dropped or configured
via the />>>/java.util.logging.config.file system property if/when needed. />>//>>/For the java.util.logging.config.file system property to work then />>/you would need java.logging to be linked in - so if you do want />>/a test to spit out logging traces then you should require java.logging />>/in @modules - whether you use logging.properties or programmatic />>/interface to configure logging. />>//>>/So it all depends on how useful said traces are when investigating />>/a test failure. If a test is known to fail intermittently and />>/test failure would be very difficult to analyze without the logging />>/traces then the test should probably require and configure java.logging />>/upfront. />>//>>/Otherwise I agree you might want to remove the useless code, unless />>/you do want to validate that no NPE or else happen while logging... />>//>>/best regards, />>//>>/-- daniel />>//>>//>>>//>>>/$.02, Roger />>>//>>>//>>>//>>>/On 11/1/2016 1:53 PM, Daniel Fuchs wrote: />>>>/Hi Roger, />>>>//>>>>/On 01/11/16 17:21, Roger Riggs wrote: />>>>>/Hi Sergei, />>>>>//>>>>>/I think it would be preferable to convert the tests to use />>>>>/System.getLogger. />>>>>/Is that possible? />>>>//>>>>/Some of the tests want to configure the logging, rather />>>>/than simply produce traces - so they will need java.logging />>>>/to do that: />>>>//>>>>/670 Logger logger = Logger.getLogger("com.sun.net.httpserver"); />>>>/671 ConsoleHandler ch = new ConsoleHandler(); />>>>/672 logger.setLevel(Level.ALL); />>>>/673 ch.setLevel(Level.ALL); />>>>/674 logger.addHandler(ch); />>>>//>>>>/It's recommended to use System.Logger to log messages, />>>>/but you will have to use java.util.logging if you want to configure />>>>/the logging framework. Of course a library shouldn't do that, />>>>/but a test is well in its right to configure logging to make />>>>/sure the traces will appear in the log. />>>>/Unless you do want to run the test in a VM that does not have />>>>/java.logging linked in. />>>>//>>>>/cheers, />>>>//>>>>/-- daniel />>>>//>>>>//>>>>>//>>>>>/Thanks, Roger />>>>>//>>>>>//>>>>>/On 11/1/2016 1:15 PM, Sergei Kovalev wrote: />>>>>>/Hello all, />>>>>>//>>>>>>/Please review a small fix for tests. />>>>>>//>>>>>>/BugID: https://bugs.openjdk.java.net/browse/JDK-8169002 />>>>>>/WebRev: http://cr.openjdk.java.net/~skovalev/8169002/webrev.00/
<http://cr.openjdk.java.net/%7Eskovalev/8169002/webrev.00/> />>>>>>//>>>>>>/Issue: Several tests from java/net/httpclient folder have undeclared />>>>>>/dependency on java.logging module. This issue leads the test to fail />>>>>>/in case module limitation. />>>>>>/Solution: added module declaration into jtreg header and organized />>>>>>/imports. /
Re: RFR(s): 8166791: Fix module dependencies for networking component tests
Ok. Will do 03.10.16 17:22, Chris Hegarty wrote: On 03/10/16 15:19, Sergei Kovalev wrote: Changed http://cr.openjdk.java.net/~skovalev/8166791/webrev.03/ This looks fine. Reviewed. No need to generate the webrev, but can you please check that you list the java.* modules before the jdk.* ones. Thanks, -Chris. -- With best regards, Sergei
Re: RFR(s): 8166791: Fix module dependencies for networking component tests
Fixed http://cr.openjdk.java.net/~skovalev/8166791/webrev.02/ 03.10.16 16:36, Alan Bateman wrote: On 03/10/2016 14:33, Sergei Kovalev wrote: SSL engin requires security provider. Its implementation located in jdk.security.auth. In case no jdk module added we getting an exception: test ErrorTest.test(): failure java.lang.IllegalArgumentException: Cannot support TLS_KRB5_WITH_3DES_EDE_CBC_SHA with currently installed providers at sun.security.ssl.CipherSuiteList.(java.base@9-ea/CipherSuiteList.java:81) at sun.security.ssl.SSLEngineImpl.setEnabledCipherSuites(java.base@9-ea/SSLEngineImpl.java:2087) at javax.net.ssl.SSLEngine.setSSLParameters(java.base@9-ea/SSLEngine.java:1269) at sun.security.ssl.SSLEngineImpl.setSSLParameters(java.base@9-ea/SSLEngineImpl.java:2166) The Kerberos ciphers are in java.security.jgss so that might be what you need here. -Alan -- With best regards, Sergei
Re: RFR(s): 8166791: Fix module dependencies for networking component tests
Hi Chris, Thank you for looking this. 03.10.16 16:05, Chris Hegarty wrote: Sergei, On 03/10/16 11:08, Sergei Kovalev wrote: Resending this for review 27.09.16 18:44, Sergei Kovalev wrote: Hi team, Could you please review small fix for regression tests. BugID: https://bugs.openjdk.java.net/browse/JDK-8166791 WebRev: http://cr.openjdk.java.net/~skovalev/8166791/webrev.00/ Issue: Severl network related tests failed in case of using "--limit-modules java.base" command line option. The test java/net/httpclient/http2/ErrorTest.java should declare dependencies on 'jdk.security.auth' module. I cannot find the reason why you think this module needs jdk.security.auth, can you please explain. SSL engin requires security provider. Its implementation located in jdk.security.auth. In case no jdk module added we getting an exception: test ErrorTest.test(): failure java.lang.IllegalArgumentException: Cannot support TLS_KRB5_WITH_3DES_EDE_CBC_SHA with currently installed providers at sun.security.ssl.CipherSuiteList.(java.base@9-ea/CipherSuiteList.java:81) at sun.security.ssl.SSLEngineImpl.setEnabledCipherSuites(java.base@9-ea/SSLEngineImpl.java:2087) at javax.net.ssl.SSLEngine.setSSLParameters(java.base@9-ea/SSLEngine.java:1269) at sun.security.ssl.SSLEngineImpl.setSSLParameters(java.base@9-ea/SSLEngineImpl.java:2166) ... java/net/httpclient/security/Driver.java should depend oh 'java.httpclient' and 'jdk.httpserver'. java/net/spi/URLStreamHandlerProvider/Basic.java and sun/net/www/protocol/jar/MultiReleaseJarURLConnection.java have declaration of dependency on 'java.compiler' but should be 'jdk.compiler'. They should require both java.compiler and jdk.compiler, no? You are right. The tests requires java.compiler for compilation and jdk.compiler for execution. I've done appropriate changes. Please take a look at: http://cr.openjdk.java.net/~skovalev/8166791/webrev.01/ From the test java/net/httpclient/http2/HpackDriver.java extracted one part that depends on 'jdk.localedata': java/net/httpclient/http2/HpackDriverHeaderTable.java because other part don't have any extra dependencies and could be executed in general case. -Chris. -- With best regards, Sergei
RFR(s): 8166791: Fix module dependencies for networking component tests
Hi team, Could you please review small fix for regression tests. BugID: https://bugs.openjdk.java.net/browse/JDK-8166791 WebRev: http://cr.openjdk.java.net/~skovalev/8166791/webrev.00/ Issue: Severl network related tests failed in case of using "--limit-modules java.base" command line option. The test java/net/httpclient/http2/ErrorTest.java should declare dependencies on 'jdk.security.auth' module. java/net/httpclient/security/Driver.java should depend oh 'java.httpclient' and 'jdk.httpserver'. java/net/spi/URLStreamHandlerProvider/Basic.java and sun/net/www/protocol/jar/MultiReleaseJarURLConnection.java have declaration of dependency on 'java.compiler' but should be 'jdk.compiler'. From the test java/net/httpclient/http2/HpackDriver.java extracted one part that depends on 'jdk.localedata': java/net/httpclient/http2/HpackDriverHeaderTable.java because other part don't have any extra dependencies and could be executed in general case. -- With best regards, Sergei
RFR(s): 8166285: Missing dependencies java.httpclient for tests from java/net pachage
Hello team, Could you please review below fix for: BugID: https://bugs.openjdk.java.net/browse/JDK-8166285 Webrev: http://cr.openjdk.java.net/~skovalev/8166285/webrev.00/ Issue: Several regression tests from java/net package failing on execution in case an option "--limi-modules java/base" provided. Solution: add missed modules declaration in appropriate test header. More details see in CR. -- With best regards, Sergei
