RFR(s): 8169196: [TESTBUG] Three tests from sun/net/www have undeclared dependencies

[Sergei Kovalev]

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
>> TlRMTVNTUAABAAAAB4IIAAAAAAAAAAAAAAAAAAAAAAA=
>> 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 at 9-ea/Native 
>>
>> Method)
>>         at
>> jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base at 9-ea/NativeMethodAccessorImpl.java:62) 
>>
>>
>>         at
>> jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base at 9-ea/DelegatingMethodAccessorImpl.java:43) 
>>
>>
>>         at 
>> java.lang.reflect.Method.invoke(java.base at 9-ea/Method.java:537)
>>         at
>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110) 
>>
>>
>>         at java.lang.Thread.run(java.base at 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 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.
>>>>
>