Re: JDK-8215102 (Follow-up)

2019-01-24 Thread Jaikiran Pai
Hello Severin,

Thank you for spending time on this. Although that JIRA was raised for
in context of MySQL driver, having watched this discussion and looked a
bit into the exception stacktrace, I think it's not really specific to
MySQL.

So I decided to reproduce this with just plain Java SSLSocket APIs and
was able to reproduce this. I have attached a very simple jtreg test
case which fails against the latest upstream jdk source repo. The test
case has details about what it does, but in short, the test creates a
SSLServerSocket (server) and a SSLSocket (client) and the client sends
some random data to the server and then calls SSLSocket.shutdownInput().
That call triggers the exception that's being discussed here. Reading
the javadoc of SSLSocket.shutdownInput() I don't think the usage of this
API is incorrect (this is of course based on my limited knowledge of
that API).

-Jaikiran

On 24/01/19 10:31 PM, Severin Gehwolf wrote:
> Hi,
>
> Thanks for your feedback!
>
> I've tried to reproduce this on my end too, but failed. At least with
> MariaDB 10.2.21 and their recent jdbc driver and JDK 11.
>
> This looks like a JDBC driver issue on newer JDKs. Hence, I've noted
> that in the bug and closed it:
> https://bugs.openjdk.java.net/browse/JDK-8215102
>
> If somebody manages to reproduce this with recent JDBC drivers I'd be
> happy to re-open it. mysql-connector-java-5.1.43.jar seems rather old.
> Current is 8.0.14.
>
> Thanks,
> Severin
>
> On Tue, 2019-01-22 at 18:14 +0530, Jaikiran Pai wrote:
>> FWIW - I don't think this is related to WildFly server. So I decided
>> to try and reproduce this in a trivial standalone program and I was
>> able to reproduce this. Here's the code to reproduce this issue:
>>
>> import java.sql.*;
>>
>> public class ConnectionCloseTest {
>>
>> public static void main(final String[] args) throws Exception {
>> final String url = "jdbc:mysql://localhost/?requireSSL=true";
>> final String user = "youruser"; // set the right user
>> final String pass = "yourpassword"; // set the right password
>> Class.forName("com.mysql.jdbc.Driver");
>> final Connection conn = DriverManager.getConnection(url,
>> user, pass);
>> System.out.println("Got connection");
>> conn.close();
>> System.out.println("Closed connection");
>> }
>>
>> }
>> It's important to start the MySQL server with ssl enabled. For that I
>> just had to set:
>> [mysqld]
>> ssl=1
>> in my MySQL server configuration. On the client side you will need
>> the mysql JDBC driver jar in the classpath. The one I used for this
>> test was mysql-connector-java-5.1.43.jar.
>> Running this with Java 8 doesn't throw any exceptions or WARN logs.
>> However, running it against Java 11 and even Java 12 latest EA build,
>> throws an exception, which gets logged as a WARN by the driver (and
>> things move on) on connection close:
>>
>> WARN: Caught while disconnecting...
>>
>> EXCEPTION STACK TRACE:
>>
>>
>>
>> ** BEGIN NESTED EXCEPTION ** 
>>
>> javax.net.ssl.SSLException
>> MESSAGE: closing inbound before receiving peer's close_notify
>>
>> STACKTRACE:
>>
>> javax.net.ssl.SSLException: closing inbound before receiving peer's
>> close_notify
>> at
>> java.base/sun.security.ssl.Alert.createSSLException(Alert.java:133)
>> at
>> java.base/sun.security.ssl.Alert.createSSLException(Alert.java:117)
>> at
>> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.ja
>> va:307)
>> at
>> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.ja
>> va:263)
>> at
>> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.ja
>> va:254)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.shutdownInput(SSLSocketImpl.
>> java:650)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.shutdownInput(SSLSocketImpl.
>> java:629)
>> at com.mysql.jdbc.MysqlIO.quit(MysqlIO.java:2246)
>> at
>> com.mysql.jdbc.ConnectionImpl.realClose(ConnectionImpl.java:4234)
>> at com.mysql.jdbc.ConnectionImpl.close(ConnectionImpl.java:1472)
>> at ConnectionCloseTest.main(ConnectionCloseTest.java:13)
>> at
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Nativ
>> e Method)
>> at
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Native
>> MethodAccessorImpl.java:62)
>> at
>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(De
>> legatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:567)
>> at
>> jdk.compiler/com.sun.tools.javac.launcher.Main.execute(Main.java:415)
>> at
>> jdk.compiler/com.sun.tools.javac.launcher.Main.run(Main.java:192)
>> at
>> jdk.compiler/com.sun.tools.javac.launcher.Main.main(Main.java:132)
>>
>>
>> ** END NESTED EXCEPTION **
>> -Jaikiran
>>
>> On 22/01/19 7:43 AM, Dennis Gesker wrote:
>>> Hi Severing:
>>>
>>> I'll post the generic error when I get to the office. It seems to
>>> throw the complaints 

RE: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-24 Thread Langer, Christoph
Hi Sean,

thanks for looking at this. I agree. Will remove othervm...

Best regards
Christoph

> -Original Message-
> From: Sean Mullan 
> Sent: Donnerstag, 24. Januar 2019 17:43
> To: Langer, Christoph ; OpenJDK Dev list
> ; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: Re: RFR 8217657: Move the test for default value of
> jdk.includeInExceptions into own test
> 
> I don't think you really need to run the test with the othervm flag,
> unless you are concerned other tests may be setting this property and
> (incorrectly) not running in a separate VM, which would be a bug in my
> opinion. Well, then maybe you should run it in othervm just in case.
> Otherwise, looks ok to me.
> 
> --Sean
> 
> On 1/23/19 11:05 AM, Langer, Christoph wrote:
> > Hi,
> >
> > please review a small test update.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8217657
> >
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/
> >
> > I'd like to move the test for the correct default value of security
> > property "jdk.includeInExceptions" into an own testcase in the
> > jdk.security area. This seems a bit more natural than to hide this check
> > in a java/net specific test and will help finding/maintaining tests when
> > the (default-)value for that property changes. For instance new values
> > get added or other OpenJDK builds have different defaults (e.g.
> SAPMachine).
> >
> > Thanks
> >
> > Christoph
> >


Re: JDK-8215102 (Follow-up)

2019-01-24 Thread Severin Gehwolf
Hi,

Thanks for your feedback!

I've tried to reproduce this on my end too, but failed. At least with
MariaDB 10.2.21 and their recent jdbc driver and JDK 11.

This looks like a JDBC driver issue on newer JDKs. Hence, I've noted
that in the bug and closed it:
https://bugs.openjdk.java.net/browse/JDK-8215102

If somebody manages to reproduce this with recent JDBC drivers I'd be
happy to re-open it. mysql-connector-java-5.1.43.jar seems rather old.
Current is 8.0.14.

Thanks,
Severin

On Tue, 2019-01-22 at 18:14 +0530, Jaikiran Pai wrote:
> FWIW - I don't think this is related to WildFly server. So I decided
> to try and reproduce this in a trivial standalone program and I was
> able to reproduce this. Here's the code to reproduce this issue:
> 
> import java.sql.*;
> 
> public class ConnectionCloseTest {
> 
> public static void main(final String[] args) throws Exception {
> final String url = "jdbc:mysql://localhost/?requireSSL=true";
> final String user = "youruser"; // set the right user
> final String pass = "yourpassword"; // set the right password
> Class.forName("com.mysql.jdbc.Driver");
> final Connection conn = DriverManager.getConnection(url,
> user, pass);
> System.out.println("Got connection");
> conn.close();
> System.out.println("Closed connection");
> }
> 
> }
> It's important to start the MySQL server with ssl enabled. For that I
> just had to set:
> [mysqld]
> ssl=1
> in my MySQL server configuration. On the client side you will need
> the mysql JDBC driver jar in the classpath. The one I used for this
> test was mysql-connector-java-5.1.43.jar.
> Running this with Java 8 doesn't throw any exceptions or WARN logs.
> However, running it against Java 11 and even Java 12 latest EA build,
> throws an exception, which gets logged as a WARN by the driver (and
> things move on) on connection close:
> 
> WARN: Caught while disconnecting...
> 
> EXCEPTION STACK TRACE:
> 
> 
> 
> ** BEGIN NESTED EXCEPTION ** 
> 
> javax.net.ssl.SSLException
> MESSAGE: closing inbound before receiving peer's close_notify
> 
> STACKTRACE:
> 
> javax.net.ssl.SSLException: closing inbound before receiving peer's
> close_notify
> at
> java.base/sun.security.ssl.Alert.createSSLException(Alert.java:133)
> at
> java.base/sun.security.ssl.Alert.createSSLException(Alert.java:117)
> at
> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.ja
> va:307)
> at
> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.ja
> va:263)
> at
> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.ja
> va:254)
> at
> java.base/sun.security.ssl.SSLSocketImpl.shutdownInput(SSLSocketImpl.
> java:650)
> at
> java.base/sun.security.ssl.SSLSocketImpl.shutdownInput(SSLSocketImpl.
> java:629)
> at com.mysql.jdbc.MysqlIO.quit(MysqlIO.java:2246)
> at
> com.mysql.jdbc.ConnectionImpl.realClose(ConnectionImpl.java:4234)
> at com.mysql.jdbc.ConnectionImpl.close(ConnectionImpl.java:1472)
> at ConnectionCloseTest.main(ConnectionCloseTest.java:13)
> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Nativ
> e Method)
> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Native
> MethodAccessorImpl.java:62)
> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(De
> legatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:567)
> at
> jdk.compiler/com.sun.tools.javac.launcher.Main.execute(Main.java:415)
> at
> jdk.compiler/com.sun.tools.javac.launcher.Main.run(Main.java:192)
> at
> jdk.compiler/com.sun.tools.javac.launcher.Main.main(Main.java:132)
> 
> 
> ** END NESTED EXCEPTION **
> -Jaikiran
> 
> On 22/01/19 7:43 AM, Dennis Gesker wrote:
> > Hi Severing:
> > 
> > I'll post the generic error when I get to the office. It seems to
> > throw the complaints when it closes a connection.
> > 
> > Here is the thing...
> > 
> > 1. I'm glad this found its way to you (being Red Hat guy) as we
> > first noticed it in WildFly 15.0.1.  (But, wasn't looking for it
> > before as we only need it for a few XA migration transactions.)
> > 
> > 2. It MIGHT be the driver as we use PostgreSQL driver mostly -- in
> > the same container -- and no errors there on WildFly 15.0.1 and JDK
> > 11.
> > 
> > 3. I will also try to fall back to JDK 8 and see if it continues in
> > WildFly 15.0.1.
> > 
> > 4. The error occurs -- it would seem -- as the pool closes idle
> > connections.
> > 
> > 5.  I'll post the pool/data source config in WildFly as well -- it
> > seems correct and seems to work OK in my application.
> > 
> > Oh, yeah...
> > 
> > And, I found the form to be a contributor (not comitter) and will
> > fill that out tomorrow as well and submit it to you directly.
> > 
> > --drg
> > 
> > On Mon, Jan 21, 2019, 09:23 Severin Gehwolf  > wrote:
> > > On Mon, 2019-01-21 at 07:57 -0700, Dennis Gesker wrote:
> > > > 

Re: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-24 Thread Sean Mullan
I don't think you really need to run the test with the othervm flag, 
unless you are concerned other tests may be setting this property and 
(incorrectly) not running in a separate VM, which would be a bug in my 
opinion. Well, then maybe you should run it in othervm just in case. 
Otherwise, looks ok to me.


--Sean

On 1/23/19 11:05 AM, Langer, Christoph wrote:

Hi,

please review a small test update.

Bug: https://bugs.openjdk.java.net/browse/JDK-8217657

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/

I’d like to move the test for the correct default value of security 
property “jdk.includeInExceptions” into an own testcase in the 
jdk.security area. This seems a bit more natural than to hide this check 
in a java/net specific test and will help finding/maintaining tests when 
the (default-)value for that property changes. For instance new values 
get added or other OpenJDK builds have different defaults (e.g. SAPMachine).


Thanks

Christoph



RE: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-24 Thread Lindenmaier, Goetz
Thanks!

I should have stated that I don't need a new webrev, thanks anyways.

Best regards,
  Goetz.

> -Original Message-
> From: Langer, Christoph
> Sent: Donnerstag, 24. Januar 2019 11:59
> To: Lindenmaier, Goetz ; OpenJDK Dev list
> ; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: RE: RFR 8217657: Move the test for default value of
> jdk.includeInExceptions into own test
> 
> Hi Goetz,
> 
> 
> 
> thanks for the review.
> 
> 
> 
> I've added the comment as you suggested:
> http://cr.openjdk.java.net/~clanger/webrevs/8217657.1/
> 
> 
> 
> Will run it through our nightly tests before submitting...
> 
> 
> 
> /Christoph
> 
> 
> 
> 
> 
> From: Lindenmaier, Goetz
> Sent: Donnerstag, 24. Januar 2019 08:48
> To: Langer, Christoph ; OpenJDK Dev list
> ; OpenJDK Network Dev list  d...@openjdk.java.net>
> Subject: RE: RFR 8217657: Move the test for default value of
> jdk.includeInExceptions into own test
> 
> 
> 
> Hi Christoph,
> 
> 
> 
> it is a good idea to keep testing these two matters separately.
> 
> 
> 
> Could you please document in the new test that in OpenJDK
> 
> it is decided to keep this property empty?
> 
> Something like:
> 
> @comment In OpenJDK, this property is empty by default and on purpose. This
> test assures the default is not changed.
> 
> 
> 
> Otherwise, looks good.
> 
> 
> 
> Best regards,
> 
>   Goetz.
> 
> 
> 
> From: net-dev mailto:net-dev-
> boun...@openjdk.java.net> > On Behalf Of Langer, Christoph
> Sent: Mittwoch, 23. Januar 2019 17:06
> To: OpenJDK Dev list mailto:security-
> d...@openjdk.java.net> >; OpenJDK Network Dev list  d...@openjdk.java.net  >
> Subject: [CAUTION] RFR 8217657: Move the test for default value of
> jdk.includeInExceptions into own test
> 
> 
> 
> Hi,
> 
> 
> 
> please review a small test update.
> 
> 
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8217657
> 
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/
> 
> 
> 
> I'd like to move the test for the correct default value of security property
> "jdk.includeInExceptions" into an own testcase in the jdk.security area. This
> seems a bit more natural than to hide this check in a java/net specific test 
> and
> will help finding/maintaining tests when the (default-)value for that property
> changes. For instance new values get added or other OpenJDK builds have
> different defaults (e.g. SAPMachine).
> 
> 
> 
> Thanks
> 
> Christoph
> 
> 



RE: RFR 8217657: Move the test for default value of jdk.includeInExceptions into own test

2019-01-24 Thread Langer, Christoph
Hi Goetz,

thanks for the review.

I've added the comment as you suggested: 
http://cr.openjdk.java.net/~clanger/webrevs/8217657.1/

Will run it through our nightly tests before submitting...

/Christoph


From: Lindenmaier, Goetz
Sent: Donnerstag, 24. Januar 2019 08:48
To: Langer, Christoph ; OpenJDK Dev list 
; OpenJDK Network Dev list 

Subject: RE: RFR 8217657: Move the test for default value of 
jdk.includeInExceptions into own test

Hi Christoph,

it is a good idea to keep testing these two matters separately.

Could you please document in the new test that in OpenJDK
it is decided to keep this property empty?
Something like:
@comment In OpenJDK, this property is empty by default and on purpose. This 
test assures the default is not changed.

Otherwise, looks good.

Best regards,
  Goetz.

From: net-dev 
mailto:net-dev-boun...@openjdk.java.net>> On 
Behalf Of Langer, Christoph
Sent: Mittwoch, 23. Januar 2019 17:06
To: OpenJDK Dev list 
mailto:security-dev@openjdk.java.net>>; OpenJDK 
Network Dev list mailto:net-...@openjdk.java.net>>
Subject: [CAUTION] RFR 8217657: Move the test for default value of 
jdk.includeInExceptions into own test

Hi,

please review a small test update.

Bug: https://bugs.openjdk.java.net/browse/JDK-8217657
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8217657.0/

I'd like to move the test for the correct default value of security property 
"jdk.includeInExceptions" into an own testcase in the jdk.security area. This 
seems a bit more natural than to hide this check in a java/net specific test 
and will help finding/maintaining tests when the (default-)value for that 
property changes. For instance new values get added or other OpenJDK builds 
have different defaults (e.g. SAPMachine).

Thanks
Christoph