On 21/07/16 14:37, Daniel Fuchs wrote:
Hi,
Looks good to me Chris.
Thanks for the review Daniel.
The only thing I noticed is this:
- if (! (e instanceof SecurityException) &&
- !(e.getCause() instanceof SecurityException) ||
- !exceptionExpected)
- {
- System.out.println ("FAIL");
- //e.printStackTrace();
+ if (!expectException || !(e.getCause() instanceof
SecurityException)) {
+ System.out.println ("FAIL. Unexpected: " +
e.getMessage());
+ e.printStackTrace();
The new version does not check that e is an instance
of SecurityExcption. I'm also a bit surprised to see that
the test expects the SecurityException to be wrapped.
The SE is expected to be wrapped because of the use of
getResponseCode(), as opposed to getInputStream(). getInputStream()
would throw SE directly, but getResponseCode() attempts to return
an actual integer response code where possible. It's implementation
internally calls getInputStream(), but will catch all exceptions
and attempt to get the integer response code, which will fail
later with a 'rememberedException' which gets wrapper with a
RuntimeException. You can see an example stacktrace below [*].
I could restore the original code in the test, as it is not
relevant to the changes to make the test more stable. It was
just an attempt at simplification, since the exception will
never be an instance of SE.
-Chris.
[*]
java.lang.RuntimeException: java.security.AccessControlException: access
denied ("java.net.SocketPermission" "127.0.0.1:36609" "connect,resolve")
at
sun.net.www.protocol.http.HttpURLConnection.getInputStream0(java.base/HttpURLConnection.java:1447)
at
sun.net.www.protocol.http.HttpURLConnection.getInputStream(java.base/HttpURLConnection.java:1433)
at
sun.net.www.protocol.http.HttpURLConnection.getHeaderField(java.base/HttpURLConnection.java:2966)
at
java.net.HttpURLConnection.getResponseCode(java.base/HttpURLConnection.java:489)
at URLTest.test(URLTest.java:191)
at URLTest.test(URLTest.java:158)
at URLTest.test1(URLTest.java:97)
at URLTest.main(URLTest.java:61)
at
jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base/Native
Method)
at
jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base/NativeMethodAccessorImpl.java:62)
at
jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base/DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(java.base/Method.java:533)
at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110)
at java.lang.Thread.run(java.base/Thread.java:843)
Caused by: java.security.AccessControlException: access denied
("java.net.SocketPermission" "127.0.0.1:36609" "connect,resolve")
at
java.security.AccessControlContext.checkPermission(java.base/AccessControlContext.java:468)
at
java.security.AccessController.checkPermission(java.base/AccessController.java:894)
at
java.lang.SecurityManager.checkPermission(java.base/SecurityManager.java:541)
at
java.lang.SecurityManager.checkConnect(java.base/SecurityManager.java:1043)
at sun.net.www.http.HttpClient.New(java.base/HttpClient.java:313)
at sun.net.www.http.HttpClient.New(java.base/HttpClient.java:326)
at
sun.net.www.protocol.http.HttpURLConnection.getNewHttpClient(java.base/HttpURLConnection.java:1166)
at
sun.net.www.protocol.http.HttpURLConnection.plainConnect0(java.base/HttpURLConnection.java:1102)
at
sun.net.www.protocol.http.HttpURLConnection.plainConnect(java.base/HttpURLConnection.java:996)
at
sun.net.www.protocol.http.HttpURLConnection.connect(java.base/HttpURLConnection.java:930)
at
sun.net.www.protocol.http.HttpURLConnection.getInputStream0(java.base/HttpURLConnection.java:1505)
at
sun.net.www.protocol.http.HttpURLConnection.getInputStream(java.base/HttpURLConnection.java:1433)
at
java.net.HttpURLConnection.getResponseCode(java.base/HttpURLConnection.java:480)
... 10 more
cheers,
-- daniel
On 20/07/16 15:41, Chris Hegarty wrote:
The test uses hardcoded port numbers for its listening HTTP and HTTPS
severs.
These hardcoded port numbers are problematic as the ports may not
necessarily
be available. The reason for the hardcoded port numbers is that the
test uses a
number of static, checked in, policy files, to grant ( and restrict )
permission
required for various explicit test scenarios.
A better solution, which avoids the need for hardcoded port numbers,
is for the
test to configure the security policy dynamically. This also has the
advantage of
improving the readability of the test, since the code that grants the
permissions
is along side the test code that makes assertions based on these
permissions.
The individual test scenarios have not changed.
http://cr.openjdk.java.net/~chegar/8078568/
https://bugs.openjdk.java.net/browse/JDK-8078568
-Chris.
