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

[Daniel Fuchs]

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.
>>>