Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
chenggwang commented on PR #671:
URL: https://github.com/apache/tomcat/pull/671#issuecomment-1762749332
> The benefit seems null IMO, in the debugger it will print out that this is
null instead of a NPE, which should mean the same for a developer. Also,
Nio2Channel.toString is the same.
Following is the`NioChannel` attribute `SocketChannel sc`, along with its
`constructor`, `rest` methods and `toString` method.We initialize a
`NioChannel` through the constructor, only the `bufHandler` property is
assigned a value, and its `sc` property remains null. At this point the object
is nearly created, meaning we can use its properties and methods as we wish.
But when we use its toString() method, `super.toString() + ":" +
sc.toString()`, `sc` is null, so it throws an NPE. The `sc` is not assigned a
value until we call the reset method. Otherwise, sc.toString() throws NPE
directly instead of printing null
```java
public class NioChannel implements ByteChannel, ScatteringByteChannel,
GatheringByteChannel {
... Omit extraneous code
protected final SocketBufferHandler bufHandler;
protected SocketChannel sc = null;
protected NioSocketWrapper socketWrapper = null;
public NioChannel(SocketBufferHandler bufHandler) {
this.bufHandler = bufHandler;
}
...
public String toString() {
return super.toString() + ":" + sc.toString();
}
public void reset(SocketChannel channel, NioSocketWrapper socketWrapper)
throws IOException {
this.sc = channel;
this.socketWrapper = socketWrapper;
bufHandler.reset();
}
...Omit extraneous code
}
```
Going back to the object-oriented level, when we provide an object, we need
to make sure that the basic functionality it provides to the outside world
after initialization is complete is usable and reliable. The basic
functionality of course includes the `toString()` method, because this is its
calling card, through which the outside world should know the basic information
about the object. This is also the original purpose of the toString method.
On line 12 we create a channel with `createChannel(bufhandler)`，At this
point we are calling the constructor of the NioChannel.This is the same
explanation as I started with, sc is still null and all calls to channel's`
toString` method before line 14 report NPE.
Similarly `NioSocketWrapper newWrapper`, the `newWrapper.toString()` method
throws an NPE until the rest method is called, as it looks like this
`super.toString() + ":" + String.valueOf(socket)`, ` String.valueOf(socket)`
still calls socket(NioChannel)'s `sc.toString`, when `sc` is null.
```java
line 1 protected boolean setSocketOptions(SocketChannel socket) {
line 2 NioSocketWrapper socketWrapper = null;
line 3try {
// Allocate channel and wrapper
line 4NioChannel channel = null;
line 5if (nioChannels != null) {
line 6channel = nioChannels.pop();
}
line 7if (channel == null) {
line 8SocketBufferHandler bufhandler = new
SocketBufferHandler(
line 9 socketProperties.getAppReadBufSize(),
line 10socketProperties.getAppWriteBufSize(),
line 11socketProperties.getDirectBuffer());
line 12channel = createChannel(bufhandler);
}
line 13NioSocketWrapper newWrapper = new
NioSocketWrapper(channel, this);
line 14channel.reset(socket, newWrapper);
line 15connections.put(socket, newWrapper);
line 16socketWrapper = newWrapper;
```
createChannel method
```java
protected NioChannel createChannel(SocketBufferHandler buffer) {
if (isSSLEnabled()) {
return new SecureNioChannel(buffer, this);
}
return new NioChannel(buffer);
}
public NioChannel(SocketBufferHandler bufHandler) {
this.bufHandler = bufHandler;
}
```
https://github.com/apache/tomcat/assets/90715678/44871f56-e5a2-435e-86cf-e15063806971";>
Above is the `System.out.println` I added between line 13 and 14 to output
the `channel` and `newWrapper` objects, throwing NPE's.I have a monitoring
project that tracks changes in the life cycle of an object after any object is
created, we are monitoring all constructors, reflections, and start logging as
soon as an object is created. The two lines `System.out.println` in the diagram
are similar to the abstraction of our monitoring function as a buried point. To
clarify our buried points are at the bytecode level, there is no hardcoding of
tomcat source code dumping. However, the NPE occurred when logging, so I went
down to the source code to debug it, and found that it was the reason that
toString() didn't judge the null.
Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
chenggwang commented on PR #671: URL: https://github.com/apache/tomcat/pull/671#issuecomment-1762757733 @rmaucher @markt-asf @michael-o Such a change in tomcat functionality does not help, but after the initialization of the NioChannel object should provide a reliable, stable function, can not allow the caller to trigger an error resulting in the termination of the program, the current large and complex network, the monitoring of the infrastructure is particularly important at this time, the role of the toString is very obvious, if you modify the words, for the NIO2channel of the toString need to be the same change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[VOTE][RESULT] Release Apache Tomcat 11.0.0-M13
The following votes were cast: Binding: +1: remm, lihan, fschumacher, markt, isapir, schultz No other votes were cast. The vote therefore passes. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r64524 - /dev/tomcat/tomcat-11/v11.0.0-M13/ /release/tomcat/tomcat-11/v11.0.0-M13/
Author: markt Date: Sat Oct 14 12:57:49 2023 New Revision: 64524 Log: Release Tomcat 11.0.0-M13 Added: release/tomcat/tomcat-11/v11.0.0-M13/ - copied from r64523, dev/tomcat/tomcat-11/v11.0.0-M13/ Removed: dev/tomcat/tomcat-11/v11.0.0-M13/ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r64525 - in /release/tomcat: tomcat-11/v11.0.0-M11/ tomcat-9/v9.0.80/ tomcat-9/v9.0.81/
Author: markt Date: Sat Oct 14 12:58:49 2023 New Revision: 64525 Log: Drop old releases from CDN Removed: release/tomcat/tomcat-11/v11.0.0-M11/ release/tomcat/tomcat-9/v9.0.80/ release/tomcat/tomcat-9/v9.0.81/ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Add release date for 11.0.0-M13
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 35e7caf8c6 Add release date for 11.0.0-M13 35e7caf8c6 is described below commit 35e7caf8c61bb54f5f316c6a835c077dc78f0b72 Author: Mark Thomas AuthorDate: Sat Oct 14 10:03:27 2023 -0300 Add release date for 11.0.0-M13 --- webapps/docs/changelog.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index ea64cd14b3..f2cb8d1c79 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -122,7 +122,7 @@ - + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1912958 - in /tomcat/site/trunk: ./ docs/tomcat-11.0-doc/ docs/tomcat-11.0-doc/annotationapi/ docs/tomcat-11.0-doc/annotationapi/jakarta/annotation/ docs/tomcat-11.0-doc/annotationapi/jak
Author: markt Date: Sat Oct 14 13:27:19 2023 New Revision: 1912958 URL: http://svn.apache.org/viewvc?rev=1912958&view=rev Log: Update site with 11.0.0-M13 docs [This commit notification would consist of 58 parts, which exceeds the limit of 50 ones, so it was shortened to the summary.] - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r1912959 - in /tomcat/site/trunk: docs/download-11.html docs/index.html docs/migration-11.0.html docs/oldnews.html docs/whichversion.html xdocs/download-11.xml xdocs/index.xml xdocs/migrat
Author: markt Date: Sat Oct 14 13:28:21 2023 New Revision: 1912959 URL: http://svn.apache.org/viewvc?rev=1912959&view=rev Log: Update site for 11.0.0-M13 release Modified: tomcat/site/trunk/docs/download-11.html tomcat/site/trunk/docs/index.html tomcat/site/trunk/docs/migration-11.0.html tomcat/site/trunk/docs/oldnews.html tomcat/site/trunk/docs/whichversion.html tomcat/site/trunk/xdocs/download-11.xml tomcat/site/trunk/xdocs/index.xml tomcat/site/trunk/xdocs/migration-11.0.xml tomcat/site/trunk/xdocs/oldnews.xml tomcat/site/trunk/xdocs/whichversion.xml Modified: tomcat/site/trunk/docs/download-11.html URL: http://svn.apache.org/viewvc/tomcat/site/trunk/docs/download-11.html?rev=1912959&r1=1912958&r2=1912959&view=diff == --- tomcat/site/trunk/docs/download-11.html (original) +++ tomcat/site/trunk/docs/download-11.html Sat Oct 14 13:28:21 2023 @@ -19,7 +19,7 @@ Quick Navigation -[define v]11.0.0-M12[end] +[define v]11.0.0-M13[end] https://downloads.apache.org/tomcat/tomcat-11/KEYS";>KEYS | [v] (alpha) | Browse | Modified: tomcat/site/trunk/docs/index.html URL: http://svn.apache.org/viewvc/tomcat/site/trunk/docs/index.html?rev=1912959&r1=1912958&r2=1912959&view=diff == --- tomcat/site/trunk/docs/index.html (original) +++ tomcat/site/trunk/docs/index.html Sat Oct 14 13:28:21 2023 @@ -34,6 +34,40 @@ wiki page. Apache Tomcat, Tomcat, Apache, the Apache feather, and the Apache Tomcat project logo are trademarks of the Apache Software Foundation. +2023-10-14 Tomcat 11.0.0-M13 Released + +The Apache Tomcat Project is proud to announce the release of version 11.0.0-M13 +(alpha) of Apache Tomcat. This release is a milestone release and is targeted at +Jakarta EE 11. +Users of Tomcat 10 onwards should be aware that, as a result of the move from +Java EE to Jakarta EE as part of the transfer of Java EE to the Eclipse +Foundation, the primary package for all implemented APIs has changed from +javax.* to jakarta.*. This will almost certainly +require code changes to enable applications to migrate from Tomcat 9 and earlier +to Tomcat 10 and later. A +https://github.com/apache/tomcat-jakartaee-migration";>migration +tool is available to aid this process. +The notable changes in this release are: + + Correct a regression in 11.0.0-M12 that broke the Tomcat JBDC + connection pool. + Correct a regression in 11.0.0-M12 that broke HTTP compression. + + +Full details of these changes, and all the other changes, are available in the +Tomcat 11 +(alpha) changelog. + + +Note: There are known regressions: +with jdbc-pool (see bug https://bz.apache.org/bugzilla/show_bug.cgi?id=67664";>67664) and +with Connector configurations when compression is enabled +(see bug https://bz.apache.org/bugzilla/show_bug.cgi?id=67670";>67670). They will be fixed in the next release. + + + +https://tomcat.apache.org/download-11.cgi";>Download + 2023-10-13 Tomcat 9.0.82 Released The Apache Tomcat Project is proud to announce the release of version 9.0.82 @@ -166,44 +200,6 @@ reach https://tomcat.apache.org https://tomcat.apache.org/download-80.cgi";>Download -2023-10-10 Tomcat 11.0.0-M12 Released - -The Apache Tomcat Project is proud to announce the release of version 11.0.0-M12 -(alpha) of Apache Tomcat. This release is a milestone release and is targeted at -Jakarta EE 11. -Users of Tomcat 10 onwards should be aware that, as a result of the move from -Java EE to Jakarta EE as part of the transfer of Java EE to the Eclipse -Foundation, the primary package for all implemented APIs has changed from -javax.* to jakarta.*. This will almost certainly -require code changes to enable applications to migrate from Tomcat 9 and earlier -to Tomcat 10 and later. A -https://github.com/apache/tomcat-jakartaee-migration";>migration -tool is available to aid this process. -The notable changes in this release are: - -Provide a lifecycle listener that will automatically reload TLS -configurations a set time before the certificate is due to expire. This is -intended to be used with third-party tools that regularly renew TLS -certificates. -Remove support for HTTP/2 server push. -Update Tomcat Native to 2.0.6 to pick up Windows binaries built with OpenSSL -3.0.11. - - -Full details of these changes, and all the other changes, are available in the -Tomcat 11 -(alpha) changelog. - - -Note: There are known regressions: -with jdbc-pool (see bug https://bz.apache.org/bugzilla/show_bug.cgi?id=67664";>67664) and -with Connector configurations when compression is enabled -(see bug https://bz.apache.org/bugzilla/show_bug.cgi?id=67670";>67670). They will be fixed in the next release. - - - -https://tomcat.apache.org/download-11.cgi";>Download - 2023-10-03 Tomcat Native 1.2.39 Released The Apache Tom
Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
kkolinko commented on PR #671: URL: https://github.com/apache/tomcat/pull/671#issuecomment-1763032642 I do not like the use of "?:" operator here. Just simple `... + ":" + sc;` is enough, or if you want to be explicit `+ ":" + String.valueOf(sc);` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 64570] Transaction not rollbacked if autocommit is false
https://bz.apache.org/bugzilla/show_bug.cgi?id=64570 --- Comment #2 from Konstantin Kolinko --- https://tomcat.apache.org/tomcat-11.0-doc/jdbc-pool.html See "rollbackOnReturn" attribute. It is false by default. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Before closing the database connection, roll back the transaction [tomcat]
kkolinko commented on PR #667: URL: https://github.com/apache/tomcat/pull/667#issuecomment-1763097366 From code review point of view: -1 1. No error handling. 2. No configuration. The `rollbackOnReturn` option should take care of the original BZ issue. And if there is a bug, behaviour of the patch should depend on the value of that option. See `o.a.t.jdbc.pool.ConnectionPool#terminateTransaction(...)` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
chenggwang commented on PR #671: URL: https://github.com/apache/tomcat/pull/671#issuecomment-1763120686 > I do not like the use of "?:" operator here. Just simple `... + ":" + sc;` is enough, or if you want to be explicit `+ ":" + String.valueOf(sc);` If we don't consider the constant pool memory consumption, and readability (String + Object, which doesn't clearly express the intent). Your method is fine this way. In fact this will end up with one more entry in the constant pool. like this `#1 = Methodref #xx.#xxx // java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;` `#2 = Methodref #xx.#xxx // java/lang/StringBuilder.append:(Ljava/lang/Object;)Ljava/lang/StringBuilder;` `super.toString() + sc`, the first one is a string, the second one is an object,So two entries are needed.If they are all strings, only entry #1 is needed.Strong transfer object type, not recommended for + reasons . This is just a lazy approach.Avoid this if there are a lot of character operations in a lot of classes (message middleware, logging systems, etc.), memory is also precious! As for String.valueOf(sc), sc can't be null, direct NPE, not to mention it's a series of method stack calls, less efficient. Thank you for your input. The main discussion is about the need to ensure that toString() is available to the public immediately after the object is created, and I'm encountering NPEs caused by it in my project that monitors object lifecycles and buries it, do you have any good ideas?@kkolinko -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
chenggwang commented on PR #671: URL: https://github.com/apache/tomcat/pull/671#issuecomment-1763157260 This is a bug that tomcat itself just doesn't realize. Because it doesn't need to call toString immediately after the `NioChannel` object is created.However, for third-party software that wants to monitor the operation of the object, it will start logging information as soon as the object is created. The external system doesn't know that this is a “half-finished object” and must wait until the rest method is executed before calling toString. This fix may not help much with tomcat itself, but that shouldn't stop we from fixing it. After all, it couldn't be a better fix for an external monitoring system. Besides, fixing it won't do any harm to tomcat itself. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
markt-asf commented on PR #671: URL: https://github.com/apache/tomcat/pull/671#issuecomment-1763192873 If we are going to do anything for this > Just simple `... + ":" + sc;` is enough works for me if applied to both NIO and NIO2 rather than just NIO. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 67675] Tomcat and/or Java do not read encrypted private keys with DES-EDE3-CBC generated by openssl-req(1)
https://bz.apache.org/bugzilla/show_bug.cgi?id=67675 --- Comment #3 from Mark Thomas --- It seems that very few (no?) users are creating keys with pass-phrases this way as this isn't an issue that has been reported previously and we went through a phase of getting reports of unsupported formats when we added the "try and use an in-memory keystore for everything" code. I'm not adverse to trying to fix this as it is an OpenSSL default (which version by the way?). My only reservation at this point is how complex the fix might get. That depends on how much of the fix can use the standard Java APIs and how much we end up having to hand-craft. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Fix NioChannel's toString() throwing NullPointerException in some cases [tomcat]
chenggwang commented on PR #671: URL: https://github.com/apache/tomcat/pull/671#issuecomment-1763278643 > If we are going to do anything for this > > > Just simple `... + ":" + sc;` is enough > > works for me if applied to both NIO and NIO2 rather than just NIO Ok!Mark，I've modified NIO and NIO2 based on this suggestion, and the test for NIO2 passes (it's simpler I won't write a test case, I'll just post a picture) https://github.com/apache/tomcat/assets/90715678/2cbbb3b8-2aff-492b-ade2-9ceffc949d9a";> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
