Re: [tomcat] branch main updated: Align with spec

2023-03-07 Thread Mark Thomas

On 07/03/2023 07:14, Han Li wrote:

On Mar 7, 2023, at 14:39, Konstantin Kolinko  wrote:
вт, 7 мар. 2023 г. в 09:17, mailto:li...@apache.org>>:


This is an automated email from the ASF dual-hosted git repository.

lihan 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 1fc4b7c95d Align with spec
1fc4b7c95d is described below

commit 1fc4b7c95dce1db3d86db9393c78023b93725f63
Author: lihan 
AuthorDate: Tue Mar 7 14:16:53 2023 +0800

Align with spec


-1 (veto)

Please revert.

Ok.


The text of the specification comes with a license.

I have not checked recently (with the spec is managed by Eclipse
Foundation), but in earlier times (for specs copyrighted by Oracle) it
was clear that you were not allowed to copy their text as you wish.

You are not the first one to make such changes. There were similar
discussions in earlier years.


I probably understand what means, and I have another question that if I just 
align code with spec there’s no problem, right?


That is generally problematic too.

It is OK to copy the stuff we have to copy to implement the spec - i.e. 
the method signatures.


While there may be other parts of the code where copyright does not 
apply, that judgement is a subjective one. The only place to get a 
definitive answer is in the courts.


Generally, we should not be copying anything apart from the message 
signatures from the specifications.


It is worth mentioning a couple of complicating factors:

- A long time ago the specification projects were based at the ASF so
  some of the code has ASF license headers and copyrights.

- I am a committer both here and for most of the specifications that
  Tomcat implements so it is possible for me to commit the same changes
  to both projects under different licenses. It is not possible for
  someone else to take my work from one project and commit to the other
  and change the license. If anyone else does that, the original license
  has to be retained (which creates legal hoops for folks to jump
  through if the copy is taken in either direction).

One of the tests in the TCK is to ensure that the spec has the correct 
public API. Tomcat passes those tests for all stable versions so there 
should not be any changes in the public API between Tomcat and the 
specification projects.


You may occasionally see differences in the public API in the current 
development branch. That is usually because I am working on a change to 
the specification.


There are differences in implementation detail. These are expected given 
that the implementation has been developed in separate projects by 
(mostly) different teams.


If you identify differences in functional behavior then that is worth 
looking at a little more closely. Usually the change history and 
associated comments provides and explanation. To give a couple of examples:


- There are a couple of Tomcat specific features that required changes
  in the specification classes.

- If the specification language was ambiguous, the specification project
  and the Tomcat may interpret the language differently resulting in
  slightly different behavior (the specification projects are trying to
  clarify these as they find them).

- You might have found a bug.

Sorry this turned out to be such a long email. If you got this far, 
thanks for taking the time to read it.


Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: [tomcat] branch main updated: Align with spec

2023-03-07 Thread Mark Thomas

On 07/03/2023 08:24, Han Li wrote:

Hi Mark,Remy and Konstatntin,

Thank you for such detailed answers, I read all your reply in full. I read, and 
the reason for this commit is because of user feedback about a bug, see below:
https://github.com/apache/tomcat/pull/595 


And then I found that it was inconsistent with the spec, in my realize, the 
Jakarta package in the tomcat project should be aligned with the spec (which is 
obviously wrong so far),
So I just simply copied and pasted it and committed it.

According to the reply, I found that I just need to follow the PR’s change to 
submit without doing anything extra.


Correct.

This is a good example of an implementation difference.

The specification provides an error message that the request and/or 
response are not of the right type but does not specify which is(are) 
wrong. Tomcat provides a specific error message that tells you which one 
is wrong.


Both approaches are valid. I'd argue Tomcat's is a little better - but I 
would wouldn't I ;)


Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: [tomcat] branch main updated: Align with spec

2023-03-07 Thread Rémy Maucherat
On Tue, Mar 7, 2023 at 8:55 AM Konstantin Kolinko
 wrote:
>
> вт, 7 мар. 2023 г. в 10:14, Han Li :
> >
> >
> >
> > > On Mar 7, 2023, at 14:39, Konstantin Kolinko  
> > > wrote:
> > >
> > > вт, 7 мар. 2023 г. в 09:17, mailto:li...@apache.org>>:
> > >>
> > >> This is an automated email from the ASF dual-hosted git repository.
> > >>
> > >> lihan 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 1fc4b7c95d Align with spec
> > >> 1fc4b7c95d is described below
> > >>
> > >> commit 1fc4b7c95dce1db3d86db9393c78023b93725f63
> > >> Author: lihan 
> > >> AuthorDate: Tue Mar 7 14:16:53 2023 +0800
> > >>
> > >> Align with spec
> > >
> > > -1 (veto)
> > >
> > > Please revert.
> > Ok.
> > >
> > > The text of the specification comes with a license.
> > >
> > > I have not checked recently (with the spec is managed by Eclipse
> > > Foundation), but in earlier times (for specs copyrighted by Oracle) it
> > > was clear that you were not allowed to copy their text as you wish.
> > >
> > > You are not the first one to make such changes. There were similar
> > > discussions in earlier years.
> >
> > I probably understand what means, and I have another question that if I 
> > just align code with spec there’s no problem, right?
> >
>
> Regarding javadoc,
> I think it is OK to document what Tomcat does. (What it has to do is
> dictated by the spec, but what it actually does is our implementation
> details, and can be documented).
>
> Regarding code,
> If you are talking about alignment of method signatures,  those should
> already have been aligned.  I know that the TCK (i.e. the set of tests
> that comes with specification) has tests that check signatures of all
> methods. As Tomcat was tested with TCK some time ago, I think those
> methods have already been tested.
>
> If you are talking about alignment of implementation details,
> there is no reason to do so,

Yes, that's a good clarification. Code should be added (without cut &
pasting ;) ) so that there are no behavior differences. In this
particular commit, there's no behavior change, so no need to fix
anything.

Rémy

> The specification is just a document (pdf) plus javadoc (and method
> signatures documented there), and a set of tests (TCK). If you are
> looking at the code that comes with the spec, that code is just a
> "reference implementation", serves as an example and does not define
> any required behaviour.
>
> If there is a bug, i.e. behaviour of Tomcat differs with that is
> dictated by official javadoc, it is a bug, and should be fixed as
> such.
>
>
> Best regards,
> Konstantin Kolinko
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: tomcat-native v2.0+ breaks unix domain socket support on java16-

2023-03-07 Thread Mark Thomas

On 07/03/2023 07:45, Rémy Maucherat wrote:

On Tue, Mar 7, 2023 at 12:01 AM Graham Leggett  wrote:


Hi all,

A while back I added unix domain socket support to tomcat-native, and patched 
tomcat to use it until java16 is available.

Unfortunately unix domain socket support was removed from tomcat-native 2.0+, 
and now tomcat-native 2.0+ is appearing in distros, meaning that unix domain 
socket support just broke.

https://tomcat.apache.org/native-doc/miscellaneous/changelog.html

"Remove all API methods (and supporting code) that are not used by Tomcat 
10.1.x to support the use of OpenSSL as a replacement for JSSE to provide TLS 
functionality. (markt)”

Is there a way to fix this, or has tomcat just broken everything for anyone in 
a RHEL environment?


With tomcat-native 2.0, support for the whole APR connector is
removed. So this also includes domain sockets since it is part of the
APR connector. tomcat-native 2.0 now only does OpenSSL for NIO and
NIO2.

If distros are updating existing branches to tomcat-native 2.0, well,
then they should not. 1.x is still supported.


I'm a little surprised that a distro would update to a new major version 
of Tomcat Native but not update to Java 17 LTS given that Java 17 LTS 
was released a good nine months before the first Tomcat Native 2.0.x 
release.


My expectation would be that any distro updating to Tomcat Native 2.0.x 
would also have updated to Java 17 so Tomcat users wanting to use domain 
sockets can use the Java support.


Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: tomcat-native v2.0+ breaks unix domain socket support on java16-

2023-03-07 Thread Konstantin Kolinko
вт, 7 мар. 2023 г. в 02:01, Graham Leggett :
>
> Hi all,
>
> A while back I added unix domain socket support to tomcat-native, and patched 
> tomcat to use it until java16 is available.

Java 17 (LTS) is already available for a while.
Is it available for your platform?

> Unfortunately unix domain socket support was removed from tomcat-native 2.0+, 
> and now tomcat-native 2.0+ is appearing in distros, meaning that unix domain 
> socket support just broke.
>
> https://tomcat.apache.org/native-doc/miscellaneous/changelog.html
>
> "Remove all API methods (and supporting code) that are not used by Tomcat 
> 10.1.x to support the use of OpenSSL as a replacement for JSSE to provide TLS 
> functionality. (markt)”
>
> Is there a way to fix this, or has tomcat just broken everything for anyone 
> in a RHEL environment?

Tomcat 9.0 and 8.5 are expected to run with Tomcat Native 1.2.x and
that version is still supported.
(They can run with Tomcat Native 2.0.x as long as APR connector is not used).

Tomcat Native 2.0 is the version used by Tomcat 10.1.x and beyond.
Tomcat 10.1.x does not have the APR connector. It has only NIO and
NIO2. The APR connector has been removed.

Is your Tomcat failing to start because it is explicitly configured to
use the APR connector (Http11AprProtocol, AjpAprProtocol)?

Best regards,
Konstantin Kolinko

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch main updated (e020b669a5 -> 154fdc5636)

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


from e020b669a5 Revert "Align with spec"
 add 154fdc5636 Fix wrong exception message.#595

No new revisions were added by this update.

Summary of changes:
 java/jakarta/servlet/http/HttpFilter.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch 9.0.x updated: Fix wrong exception message.#595

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
 new c3bc80cebf Fix wrong exception message.#595
c3bc80cebf is described below

commit c3bc80cebf12b2425562a042aed3c9d58ee99448
Author: lihan 
AuthorDate: Tue Mar 7 16:47:29 2023 +0800

Fix wrong exception message.#595
---
 java/javax/servlet/http/HttpFilter.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/javax/servlet/http/HttpFilter.java 
b/java/javax/servlet/http/HttpFilter.java
index 40394a6343..11ad7a2326 100644
--- a/java/javax/servlet/http/HttpFilter.java
+++ b/java/javax/servlet/http/HttpFilter.java
@@ -48,7 +48,7 @@ public abstract class HttpFilter extends GenericFilter {
 throw new ServletException(request + " not HttpServletRequest");
 }
 if (!(response instanceof HttpServletResponse)) {
-throw new ServletException(request + " not HttpServletResponse");
+throw new ServletException(response + " not HttpServletResponse");
 }
 doFilter((HttpServletRequest) request, (HttpServletResponse) response, 
chain);
 }


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch 10.1.x updated: Add a changelog entry.

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
 new 43cf5dc73c Add a changelog entry.
43cf5dc73c is described below

commit 43cf5dc73c86a6e2a037a7ee068d5654455ef0e1
Author: lihan 
AuthorDate: Tue Mar 7 17:12:19 2023 +0800

Add a changelog entry.
---
 webapps/docs/changelog.xml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f2718b42d0..3018a49ad2 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -152,6 +152,11 @@
 Refactor code using MD5Encoder to use
 HexUtils.toHexString(). (markt)
   
+  
+66507: Fix a bug that $JAVA_OPTS is not passed
+to the jvm in catalina.sh when calling 
version.
+Patch suggested by Eric Hamilton. (lihan)
+  
 
   
 


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch 9.0.x updated: Add a changelog entry.

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
 new 9e14971e48 Add a changelog entry.
9e14971e48 is described below

commit 9e14971e4874a979c93cb6c10faf583f385f9cc6
Author: lihan 
AuthorDate: Tue Mar 7 17:12:19 2023 +0800

Add a changelog entry.
---
 webapps/docs/changelog.xml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a8ae064e76..f9e1d61715 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -152,6 +152,11 @@
 Refactor code using MD5Encoder to use
 HexUtils.toHexString(). (markt)
   
+  
+66507: Fix a bug that $JAVA_OPTS is not passed
+to the jvm in catalina.sh when calling 
version.
+Patch suggested by Eric Hamilton. (lihan)
+  
 
   
 


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch main updated: Add a changelog entry.

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan 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 b189ac03df Add a changelog entry.
b189ac03df is described below

commit b189ac03df02e00d44001c8181886466ead62eb4
Author: lihan 
AuthorDate: Tue Mar 7 17:12:19 2023 +0800

Add a changelog entry.
---
 webapps/docs/changelog.xml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0bcbfc81d9..7efb35c2e1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -162,6 +162,11 @@
 Refactor code using MD5Encoder to use
 HexUtils.toHexString(). (markt)
   
+  
+66507: Fix a bug that $JAVA_OPTS is not passed
+to the jvm in catalina.sh when calling 
version.
+Patch suggested by Eric Hamilton. (lihan)
+  
 
   
 


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: [tomcat] branch main updated: Fix BZ 66507 - Catalina version command not honoring JAVA_OPTS variable

2023-03-07 Thread Mark Thomas

On 07/03/2023 03:39, li...@apache.org wrote:

This is an automated email from the ASF dual-hosted git repository.

lihan 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 2ee1135c46 Fix BZ 66507 - Catalina version command not honoring 
JAVA_OPTS variable
2ee1135c46 is described below

commit 2ee1135c46654ff366b50077ef0bce7090666dfa
Author: lihan 
AuthorDate: Tue Mar 7 11:38:51 2023 +0800

 Fix BZ 66507 - Catalina version command not honoring JAVA_OPTS variable
 
 Patch suggested by Eric Hamilton

 https://bz.apache.org/bugzilla/show_bug.cgi?id=66507


This needs a changelog entry.

Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch 10.1.x updated: Fix wrong exception message.#595

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
 new 4958e268c1 Fix wrong exception message.#595
4958e268c1 is described below

commit 4958e268c16bd6fc49e71db23f202f72b935ed3d
Author: lihan 
AuthorDate: Tue Mar 7 16:47:29 2023 +0800

Fix wrong exception message.#595
---
 java/jakarta/servlet/http/HttpFilter.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/jakarta/servlet/http/HttpFilter.java 
b/java/jakarta/servlet/http/HttpFilter.java
index 4c7154bea6..b02a306dbf 100644
--- a/java/jakarta/servlet/http/HttpFilter.java
+++ b/java/jakarta/servlet/http/HttpFilter.java
@@ -48,7 +48,7 @@ public abstract class HttpFilter extends GenericFilter {
 throw new ServletException(request + " not HttpServletRequest");
 }
 if (!(response instanceof HttpServletResponse)) {
-throw new ServletException(request + " not HttpServletResponse");
+throw new ServletException(response + " not HttpServletResponse");
 }
 doFilter((HttpServletRequest) request, (HttpServletResponse) response, 
chain);
 }


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 66512] File downloads render as empty if access to Tomcat based application is delegated via AJP

2023-03-07 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66512

--- Comment #15 from Alexander Schüßler  ---
Dear Mark and Christopher,

in a nutshell:
Both attempts will resolve the issue indeed. The Tomcat Dev build resolves the
issue but also if I compile our application to a build with a fixed encoding.

We are thinking about working with
https://cxf.apache.org/javadoc/latest-3.1.x/org/apache/cxf/attachment/Rfc5987Util.html
in the future.

FYI already 2016 the developer said that we should be RFC compliant but we are
still using an own implementation. The original reason why it has been
implemented like this was, that apparently on download of files with some
special characters the file name was changed after download.

E.g., "35173 J SOPs ED.log" was then downloaded as
"35173%20j%26j%20sops%20ed.log".

It seems though that currently even with the proposal suggested by Christopher
the latter issue does not happen anymore, so I guess we can change it here.

Thanks so much again for all your help. This was very efficient. Appreciate it.

Cheers!

Alex

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



[Bug 66512] File downloads render as empty if access to Tomcat based application is delegated via AJP

2023-03-07 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66512

Mark Thomas  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #16 from Mark Thomas  ---
Fixed in:
- 11.0.x for 11.0.0-M5 onwards
- 10.1.x for 10.1.8 onwards
-  9.0.x for  9.0.74 onwards
-  8.5.x for  8.5.88 onwards

-- 
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: [tomcat] branch main updated: Align with spec

2023-03-07 Thread Han Li
Hi Mark,Remy and Konstatntin,

Thank you for such detailed answers, I read all your reply in full. I read, and 
the reason for this commit is because of user feedback about a bug, see below:
https://github.com/apache/tomcat/pull/595 


And then I found that it was inconsistent with the spec, in my realize, the 
Jakarta package in the tomcat project should be aligned with the spec (which is 
obviously wrong so far),
So I just simply copied and pasted it and committed it.

According to the reply, I found that I just need to follow the PR’s change to 
submit without doing anything extra.


Thanks 

Han

> On Mar 7, 2023, at 16:06, Rémy Maucherat  wrote:
> 
> On Tue, Mar 7, 2023 at 8:55 AM Konstantin Kolinko
> mailto:knst.koli...@gmail.com>> wrote:
>> 
>> вт, 7 мар. 2023 г. в 10:14, Han Li :
>>> 
>>> 
>>> 
 On Mar 7, 2023, at 14:39, Konstantin Kolinko  
 wrote:
 
 вт, 7 мар. 2023 г. в 09:17, mailto:li...@apache.org>>:
> 
> This is an automated email from the ASF dual-hosted git repository.
> 
> lihan 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 1fc4b7c95d Align with spec
> 1fc4b7c95d is described below
> 
> commit 1fc4b7c95dce1db3d86db9393c78023b93725f63
> Author: lihan 
> AuthorDate: Tue Mar 7 14:16:53 2023 +0800
> 
> Align with spec
 
 -1 (veto)
 
 Please revert.
>>> Ok.
 
 The text of the specification comes with a license.
 
 I have not checked recently (with the spec is managed by Eclipse
 Foundation), but in earlier times (for specs copyrighted by Oracle) it
 was clear that you were not allowed to copy their text as you wish.
 
 You are not the first one to make such changes. There were similar
 discussions in earlier years.
>>> 
>>> I probably understand what means, and I have another question that if I 
>>> just align code with spec there’s no problem, right?
>>> 
>> 
>> Regarding javadoc,
>> I think it is OK to document what Tomcat does. (What it has to do is
>> dictated by the spec, but what it actually does is our implementation
>> details, and can be documented).
>> 
>> Regarding code,
>> If you are talking about alignment of method signatures, those should
>> already have been aligned. I know that the TCK (i.e. the set of tests
>> that comes with specification) has tests that check signatures of all
>> methods. As Tomcat was tested with TCK some time ago, I think those
>> methods have already been tested.
>> 
>> If you are talking about alignment of implementation details,
>> there is no reason to do so,
> 
> Yes, that's a good clarification. Code should be added (without cut &
> pasting ;) ) so that there are no behavior differences. In this
> particular commit, there's no behavior change, so no need to fix
> anything.
> 
> Rémy
> 
>> The specification is just a document (pdf) plus javadoc (and method
>> signatures documented there), and a set of tests (TCK). If you are
>> looking at the code that comes with the spec, that code is just a
>> "reference implementation", serves as an example and does not define
>> any required behaviour.
>> 
>> If there is a bug, i.e. behaviour of Tomcat differs with that is
>> dictated by official javadoc, it is a bug, and should be fixed as
>> such.
>> 
>> 
>> Best regards,
>> Konstantin Kolinko
>> 
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org 
> 
> For additional commands, e-mail: dev-h...@tomcat.apache.org 
> 


Re: [tomcat] branch main updated: Fix BZ 66507 - Catalina version command not honoring JAVA_OPTS variable

2023-03-07 Thread Han Li



> On Mar 7, 2023, at 16:07, Mark Thomas  wrote:
> 
> On 07/03/2023 03:39, li...@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>> lihan 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 2ee1135c46 Fix BZ 66507 - Catalina version command not honoring 
>> JAVA_OPTS variable
>> 2ee1135c46 is described below
>> commit 2ee1135c46654ff366b50077ef0bce7090666dfa
>> Author: lihan 
>> AuthorDate: Tue Mar 7 11:38:51 2023 +0800
>> Fix BZ 66507 - Catalina version command not honoring JAVA_OPTS variable
>>  Patch suggested by Eric Hamilton
>> https://bz.apache.org/bugzilla/show_bug.cgi?id=66507
> 
> This needs a changelog entry.

Done.

Han
> 
> Mark
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch 8.5.x updated: Add a changelog entry.

2023-03-07 Thread lihan
This is an automated email from the ASF dual-hosted git repository.

lihan pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
 new c41187f75e Add a changelog entry.
c41187f75e is described below

commit c41187f75e88eec0d0dfb35f6b06b8f9f7e6080a
Author: lihan 
AuthorDate: Tue Mar 7 17:12:19 2023 +0800

Add a changelog entry.
---
 webapps/docs/changelog.xml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8494481a8d..affc066fa7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -152,6 +152,11 @@
 Refactor code using MD5Encoder to use
 HexUtils.toHexString(). (markt)
   
+  
+66507: Fix a bug that $JAVA_OPTS is not passed
+to the jvm in catalina.sh when calling 
version.
+Patch suggested by Eric Hamilton. (lihan)
+  
 
   
 


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 66513] Primary Key Violation using PersistentManager + PersistentValves +

2023-03-07 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66513

Christopher Schultz  changed:

   What|Removed |Added

 Status|NEW |NEEDINFO

--- Comment #1 from Christopher Schultz  ---
The synchronization in PersistentManager and DataSourceStore/JDBCStore appear
to be reasonable. The session itself is being used as the monitor and not an
arbitrary object stored within the session, which might have been a problem.

I think your problem is not multiple simultaneous requests on a single node
(which ought to be safe), but multiple simultaneous requests across more than
one node.

When saving the session, DataSourceStore/JDBCStore will DELETE the existing
session record and INSERT a new record in its place. This does not happen in a
transaction and therefore it's possible for two nodes in the group to execute
this series of queries:

Node A: DELETE FROM sessions...
Node B: DELETE FROM sessions...
Node A: INSERT INTO sessions...
Node B: INSERT INTO sessions...

This could be fixed with a transaction which surrounds the DELETE and
subsequent INSERT.

It could also be fixed with, as a TODO in the code suggests, using an UPDATE if
the record already exists. (IMHO this is much better for performance as fewer
indexes, etc. would need to be updated when saving a session.)

But there is another problem: even if the database synchronization issues are
resolved, you may find that you have replaced the problem with a race-condition
within your own application. Let's say the following situation occurs
(paraphrasing):

Node A: session.setAttribute("counts", session.getAttribute("counts") + 1);
Node B: session.setAttribute("counts", session.getAttribute("counts") + 1);
Node B: BEGIN ; DELETE ; INSERT
Node A: BEGIN ; DELETE ; INSERT

One of the increments has been lost -- the increment from node B. You can
abstract this to *any* change to the session, since Tomcat saves the session
all at once and not as individual records e.g. for each attribute. The same can
be true if Node A adds a new attribute to the session and Node B adds a
different one.

If the sessions ever disagree about the contents of the session, some session
data will be lost. It is very difficult to completely prevent this kind of
thing from happening given the architecture of the DataSourceStore/JDBCStore.

In light of this, do you think that either using a transaction or switching to
an INSERT ... ON UPDATE type of behavior would resolve this problem for you in
a satisfactory way? If your sessions are colliding in the db itself but you
aren't worried about conflicting data, then either of these solutions will
probably work for you.

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



[tomcat] branch main updated: Minor Panama API change

2023-03-07 Thread remm
This is an automated email from the ASF dual-hosted git repository.

remm 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 0176919759 Minor Panama API change
0176919759 is described below

commit 0176919759426bd3094752220af04b744b550490
Author: remm 
AuthorDate: Tue Mar 7 15:13:01 2023 +0100

Minor Panama API change
---
 .../src/main/java/org/apache/tomcat/util/openssl/Constants$root.java| 2 +-
 .../src/main/java/org/apache/tomcat/util/openssl/openssl_h.java | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/Constants$root.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/Constants$root.java
index c4e5e9f4c2..7dac29f661 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/Constants$root.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/Constants$root.java
@@ -36,7 +36,7 @@ final class Constants$root {
 static final OfLong C_LONG_LONG$LAYOUT = JAVA_LONG;
 static final OfFloat C_FLOAT$LAYOUT = JAVA_FLOAT;
 static final OfDouble C_DOUBLE$LAYOUT = JAVA_DOUBLE;
-static final OfAddress C_POINTER$LAYOUT = 
ADDRESS.withBitAlignment(64).withTargetLayout(MemoryLayout.sequenceLayout(Constants$root.C_CHAR$LAYOUT));
+static final AddressLayout C_POINTER$LAYOUT = 
ADDRESS.withBitAlignment(64).withTargetLayout(MemoryLayout.sequenceLayout(Constants$root.C_CHAR$LAYOUT));
 }
 
 
diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/openssl_h.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/openssl_h.java
index a22ebdb52c..ede1a779a5 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/openssl_h.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/openssl/openssl_h.java
@@ -33,7 +33,7 @@ public class openssl_h  {
 public static final OfLong C_LONG_LONG = Constants$root.C_LONG_LONG$LAYOUT;
 public static final OfFloat C_FLOAT = Constants$root.C_FLOAT$LAYOUT;
 public static final OfDouble C_DOUBLE = Constants$root.C_DOUBLE$LAYOUT;
-public static final OfAddress C_POINTER = Constants$root.C_POINTER$LAYOUT;
+public static final AddressLayout C_POINTER = 
Constants$root.C_POINTER$LAYOUT;
 /**
  * {@snippet :
  * #define BIO_CLOSE 1


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 66508] Tomcat after a GC pause causes the HTTP threads to be blocked to acquire a semaphore to process WebSockets connection closure.

2023-03-07 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66508

Mark Thomas  changed:

   What|Removed |Added

Version|Tomcat 8.5.84   |8.5.72

--- Comment #2 from Mark Thomas  ---
Correct version field to match text report.

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



Connector Comparison in documentation - still needed?

2023-03-07 Thread Christopher Schultz

All,

The "Connector Comparison" used to be an important read when choosing a 
connector:


For the 8.5 and 9.0 releases:

https://tomcat.apache.org/tomcat-8.5-doc/config/http.html#Connector_Comparison

https://tomcat.apache.org/tomcat-9.0-doc/config/http.html#Connector_Comparison

The only difference between any of the connectors (besides "[initially 
available in] Tomcat Version" is that the APR connector is "Blocking" 
during the SSL handshake. Everything else is the same for all connectors.


For the 10.1 and 11.0 releases:

https://tomcat.apache.org/tomcat-10.1-doc/config/http.html#Connector_Comparison

https://tomcat.apache.org/tomcat-11.0-doc/config/http.html#Connector_Comparison

All connectors (again, except for the "Tomcat Version") have identical 
attributes.


Is it worth having this documentation any more, at least for specific 
versions of Tomcat? Maybe we should have a non-version-specific 
"Connector Comparison" chart which would make it more clear that there 
were significant differences in the past which are no longer present.


-chris

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: Connector Comparison in documentation - still needed?

2023-03-07 Thread Mark Thomas

On 07/03/2023 13:55, Christopher Schultz wrote:

All,

The "Connector Comparison" used to be an important read when choosing a 
connector:


For the 8.5 and 9.0 releases:

https://tomcat.apache.org/tomcat-8.5-doc/config/http.html#Connector_Comparison

https://tomcat.apache.org/tomcat-9.0-doc/config/http.html#Connector_Comparison

The only difference between any of the connectors (besides "[initially 
available in] Tomcat Version" is that the APR connector is "Blocking" 
during the SSL handshake. Everything else is the same for all connectors.


There is also the SSL support. I think that probably makes it worth 
keeping up to 9.0.x



For the 10.1 and 11.0 releases:

https://tomcat.apache.org/tomcat-10.1-doc/config/http.html#Connector_Comparison

https://tomcat.apache.org/tomcat-11.0-doc/config/http.html#Connector_Comparison

All connectors (again, except for the "Tomcat Version") have identical 
attributes.


I'm struggling to think of a justification for keeping it from 10.1.x 
onwards.


Mark




Is it worth having this documentation any more, at least for specific 
versions of Tomcat? Maybe we should have a non-version-specific 
"Connector Comparison" chart which would make it more clear that there 
were significant differences in the past which are no longer present.


-chris

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: PersistentManager, PersistentValve, and DataSource/JDBCStore can cause PK violations

2023-03-07 Thread Han Li


> On Mar 8, 2023, at 07:29, Christopher Schultz  
> wrote:
> 
> All,
> 
> Please see https://bz.apache.org/bugzilla/show_bug.cgi?id=66513 for reference.
> 
> It appears that the synchronization used by the PersistentManager can cause 
> problems when used with the PersistentValve and DataSource/JDBCStore.
> 
> The problem is that PersistentManager assumes that the Session object can be 
> used as a synchronization monitor to load/update the session in the Store. 
> The DataSource/JDBCStore implementation uses an INSERT to create a new 
> session, and a DELETE-then-INSERT to re-write the session data in the db.
> 
> When two requests arrive simultaneously, thread scheduling can cause 
> DELETE-then-DELETE-then-INSERT-then-INSERT which causes a duplicate PK 
> violation.
> 
> If the PersistentValve were not in use, the in-memory Session would be 
> stable. PersistentValve re-loads the Session from the Store on ever request, 
> rendering the PersistentManager's synchronized(session) attempt to protect 
> things useless.
> 
> I think a simple way to fix this might be to change:
> 
> // PersistentManager.java:478~
>if(session != null) {
>synchronized(session){
>session = super.findSession(session.getIdInternal());
>if(session != null){
>   // To keep any external calling code from messing up the
>   // concurrency.
>   session.access();
>   session.endAccess();
>}
>}
>}
> 
> to this:
> 
>if(session != null) {
>sessionId = String.intern(sessionId);
>synchronized(sessionId){
>session = super.findSession(session.getIdInternal());
>if(session != null){
>   // To keep any external calling code from messing up the
>   // concurrency.
>   session.access();
>   session.endAccess();
>}
>}
>}
> 
> This swaps the Session object for the sessionId as the synchronization 
> monitor. We use String.intern to ensure that we always have the same exact 
> object, even across sessions, request, etc.
-1

This method does seem very simple and solves this problem, but it’s not as good 
as you might think, see below:
https://shipilev.net/jvm/anatomy-quarks/10-string-intern/ 


So I don’t think it should be the preferred option.

Han

> 
> This is *a* way to solve this problem. There are other ways.
> 
> Another way is also a TODO in the DataSourceRealm code which suggests using 
> UPDATE for sessions that already exist. That is probably worth implementing, 
> and it would fix this particular issue.
> 
> Note that it is essentially impossible to prevent thread scheduling, requests 
> to other members of the cluster, etc. to prevent data-loss from the session 
> and this BZ isn't asking us to fix that. It's only asking that a single 
> Tomcat node with PersistentValve enabled doesn't cause thee duplicate PK 
> violations for some pretty basic usages.
> 
> -chris
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 



[GitHub] [tomcat-jakartaee-migration] jbisotti opened a new issue, #44: Does this work tool work at the source or binary level?

2023-03-07 Thread via GitHub


jbisotti opened a new issue, #44:
URL: https://github.com/apache/tomcat-jakartaee-migration/issues/44

   From the documentation, it is unclear to me if this tools updates source 
code or if it manipulates bytecode. Which is it? Thanks.


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



[tomcat] branch main updated: Panama API updates

2023-03-07 Thread remm
This is an automated email from the ASF dual-hosted git repository.

remm 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 d0c192e52f Panama API updates
d0c192e52f is described below

commit d0c192e52fe413e1fe412ddfa6df94eaaf01
Author: remm 
AuthorDate: Wed Mar 8 00:02:56 2023 +0100

Panama API updates
---
 .../tomcat/util/net/openssl/panama/OpenSSLContext.java   | 14 +++---
 .../tomcat/util/net/openssl/panama/OpenSSLEngine.java| 16 
 .../org/apache/tomcat/util/openssl/RuntimeHelper.java|  2 +-
 .../openssl/SSL_CTX_set_cert_verify_callback$cb.java |  2 +-
 .../util/openssl/SSL_CTX_set_tmp_dh_callback$dh.java |  2 +-
 .../tomcat/util/openssl/SSL_set_info_callback$cb.java|  2 +-
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index d851c9731e..92d4ec0772 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -794,7 +794,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 return SSL_TLSEXT_ERR_NOACK();
 }
 try (var localArena = Arena.ofConfined()) {
-MemorySegment inSeg = in.reinterpret(inlen, localArena.scope(), 
null);
+MemorySegment inSeg = in.reinterpret(inlen, localArena, null);
 byte[] advertisedBytes = inSeg.toArray(ValueLayout.JAVA_BYTE);
 for (byte[] negotiableProtocolBytes : state.negotiableProtocols) {
 for (int i = 0; i <= advertisedBytes.length - 
negotiableProtocolBytes.length; i++) {
@@ -803,9 +803,9 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 if (advertisedBytes[i + j] == 
negotiableProtocolBytes[j]) {
 if (j == negotiableProtocolBytes.length - 1) {
 // Match
-MemorySegment outSeg = 
out.reinterpret(ValueLayout.ADDRESS.byteSize(), localArena.scope(), null);
+MemorySegment outSeg = 
out.reinterpret(ValueLayout.ADDRESS.byteSize(), localArena, null);
 outSeg.set(ValueLayout.ADDRESS, 0, 
inSeg.asSlice(i));
-MemorySegment outlenSeg = 
outlen.reinterpret(ValueLayout.JAVA_BYTE.byteSize(), localArena.scope(), null);
+MemorySegment outlenSeg = 
outlen.reinterpret(ValueLayout.JAVA_BYTE.byteSize(), localArena, null);
 outlenSeg.set(ValueLayout.JAVA_BYTE, 0, 
(byte) negotiableProtocolBytes.length);
 return SSL_TLSEXT_ERR_OK();
 }
@@ -851,7 +851,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 continue;
 }
 MemorySegment buf = bufPointer.get(ValueLayout.ADDRESS, 0);
-certificateChain[i] = buf.reinterpret(length, 
localArena.scope(), null).toArray(ValueLayout.JAVA_BYTE);
+certificateChain[i] = buf.reinterpret(length, localArena, 
null).toArray(ValueLayout.JAVA_BYTE);
 CRYPTO_free(buf, MemorySegment.NULL, 0); // OPENSSL_free macro
 }
 MemorySegment cipher = SSL_get_current_cipher(ssl);
@@ -965,7 +965,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 // The password is too long
 log.error(sm.getString("openssl.passwordTooLong"));
 } else {
-MemorySegment bufSegment = buf.reinterpret(bufsiz, 
localArena.scope(), null);
+MemorySegment bufSegment = buf.reinterpret(bufsiz, 
localArena, null);
 bufSegment.copyFrom(callbackPasswordNative);
 return (int) callbackPasswordNative.byteSize();
 }
@@ -1383,9 +1383,9 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 this.negotiableProtocols = negotiableProtocols;
 // Use another arena to avoid keeping a reference through segments
 // This also allows making further accesses to the main pointers 
safer
-this.sslCtx = sslCtx.reinterpret(ValueLayout.ADDRESS.byteSize(), 
stateArena.scope(), null);
+this.sslCtx = sslCtx.reinterpret(ValueLayout.ADDRESS.byteSize(), 
stateArena, null);
 if (!MemorySegment.NULL.equals(confCtx)) {
-  

[Bug 66513] Primary Key Violation using PersistentManager + PersistentValves +

2023-03-07 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66513

Christopher Schultz  changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

--- Comment #3 from Christopher Schultz  ---
(In reply to Vincent Liautaud from comment #2)
> I confirm that this issue happens when using only one node.

Interesting.

> In my opinion, the issue comes from the session object used to synchronize
> the delete/insert sql request in DataSourceStore/JDBCStore. From what i can
> see the session object is different for each request (the session attributes
> can be the same but the objects themselves are different).

Aha, yes. I believe the PersistentValve causes the session to be re-loaded for
each request. So that could result in separate Session objects in memory for a
single session-id.

I think this would be very difficult to "fix" because the application will get
whatever session is returned by the Manager at the time it is requested. Thread
A may cause the session to be loaded from storage and handed to Request A only
to have Thread B do the same thing immediately afterward.

You are right: they will get separate objects each time, and therefore
synchronization is nearly impossible. I'm not sure what the best was to do this
is, because it's very easy to make bad mistakes like synchronizing on the whole
session-store which will kill performance.

> I made a test by replacing the "synchtonize(session)" by a
> "synchronize(lock)" with a lock object define like this : "private static
> final Object lock = new Object();"
> => It solves the issue (all "primary key constraint violation" disappear.
> Of course this is not the good solution because it synchronize the code
> block for each request, even for request using different sessionId (what
> could cause performance problems). 
> 
> That is why i suggested to lock on a object that would lock all requests
> using the same sessionId. 

This could be done with a separate map of locks-for-session-ids. Or maybe lock
on the String object which holds the session-id as the key of the
session-id-to-Session map. Though I don't think you can ask a Map for its key
object(s) without iterating through the whole set of Map.Entry values.

String.intern?

> Finaly regarding the race condition you mention at the end of your comment,
> this is not something that should happen in our standard JEE application
> using sessionIds stored in cookies (as all the servlet requests using a
> specific sessionId should come from a unique client/browser "in sequence" -
> no AJAX usage or equivalent => So we should not attend conditions where
> concurrent requests updating the session attributes for a same sessionId be
> treated by different nodes in //. The issue we face with those "primary key
> violation" comes from an incomplete filter (where some requests on static
> components like gif/css/... are not filtered correctly).
> 
> Do you agree with that ?

Yes, I think so. Thanks for giving more details about your specific use-case.
Specifically, you aren't trying to ensure bullet-proof session-management under
high-frequency session-contention. This is mostly about the
PersistentValve+PersistentManager+DataSourceStore/JDBCStore tripping over its
own feet.

I think we can take this to the development mailing list for discussion, then
come back here with a solution + fix.

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



PersistentManager, PersistentValve, and DataSource/JDBCStore can cause PK violations

2023-03-07 Thread Christopher Schultz

All,

Please see https://bz.apache.org/bugzilla/show_bug.cgi?id=66513 for 
reference.


It appears that the synchronization used by the PersistentManager can 
cause problems when used with the PersistentValve and DataSource/JDBCStore.


The problem is that PersistentManager assumes that the Session object 
can be used as a synchronization monitor to load/update the session in 
the Store. The DataSource/JDBCStore implementation uses an INSERT to 
create a new session, and a DELETE-then-INSERT to re-write the session 
data in the db.


When two requests arrive simultaneously, thread scheduling can cause 
DELETE-then-DELETE-then-INSERT-then-INSERT which causes a duplicate PK 
violation.


If the PersistentValve were not in use, the in-memory Session would be 
stable. PersistentValve re-loads the Session from the Store on ever 
request, rendering the PersistentManager's synchronized(session) attempt 
to protect things useless.


I think a simple way to fix this might be to change:

// PersistentManager.java:478~
if(session != null) {
synchronized(session){
session = super.findSession(session.getIdInternal());
if(session != null){
   // To keep any external calling code from messing up the
   // concurrency.
   session.access();
   session.endAccess();
}
}
}

to this:

if(session != null) {
sessionId = String.intern(sessionId);
synchronized(sessionId){
session = super.findSession(session.getIdInternal());
if(session != null){
   // To keep any external calling code from messing up the
   // concurrency.
   session.access();
   session.endAccess();
}
}
}

This swaps the Session object for the sessionId as the synchronization 
monitor. We use String.intern to ensure that we always have the same 
exact object, even across sessions, request, etc.


This is *a* way to solve this problem. There are other ways.

Another way is also a TODO in the DataSourceRealm code which suggests 
using UPDATE for sessions that already exist. That is probably worth 
implementing, and it would fix this particular issue.


Note that it is essentially impossible to prevent thread scheduling, 
requests to other members of the cluster, etc. to prevent data-loss from 
the session and this BZ isn't asking us to fix that. It's only asking 
that a single Tomcat node with PersistentValve enabled doesn't cause 
thee duplicate PK violations for some pretty basic usages.


-chris

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[GitHub] [tomcat-jakartaee-migration] ebourg commented on issue #44: Does this work tool work at the source or binary level?

2023-03-07 Thread via GitHub


ebourg commented on issue #44:
URL: 
https://github.com/apache/tomcat-jakartaee-migration/issues/44#issuecomment-1459025587

   It works at both source and binary level, recursively.


-- 
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 66513] Primary Key Violation using PersistentManager + PersistentValves +

2023-03-07 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=66513

--- Comment #2 from Vincent Liautaud  ---
Hi Christopher,
i confirm that this issue happens when using only one node.
In my opinion, the issue comes from the session object used to synchronize the
delete/insert sql request in DataSourceStore/JDBCStore. From what i can see the
session object is different for each request (the session attributes can be the
same but the objects themselves are different).

I made a test by replacing the "synchtonize(session)" by a "synchronize(lock)"
with a lock object define like this : "private static final Object lock = new
Object();"
=> It solves the issue (all "primary key constraint violation" disappear.
Of course this is not the good solution because it synchronize the code block
for each request, even for request using different sessionId (what could cause
performance problems). 

That is why i suggested to lock on a object that would lock all requests using
the same sessionId. 

Finaly regarding the race condition you mention at the end of your comment,
this is not something that should happen in our standard JEE application using
sessionIds stored in cookies (as all the servlet requests using a specific
sessionId should come from a unique client/browser "in sequence" - no AJAX
usage or equivalent => So we should not attend conditions where concurrent
requests updating the session attributes for a same sessionId be treated by
different nodes in //. The issue we face with those "primary key violation"
comes from an incomplete filter (where some requests on static components like
gif/css/... are not filtered correctly).

Do you agree with that ?

Regards

-- 
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: tomcat-native v2.0+ breaks unix domain socket support on java16-

2023-03-07 Thread Coty Sutherland
Hi there,

On Mon, Mar 6, 2023 at 6:01 PM Graham Leggett 
wrote:

> Hi all,
>
> A while back I added unix domain socket support to tomcat-native, and
> patched tomcat to use it until java16 is available.
>
> Unfortunately unix domain socket support was removed from tomcat-native
> 2.0+, and now tomcat-native 2.0+ is appearing in distros, meaning that unix
> domain socket support just broke.
>
> https://tomcat.apache.org/native-doc/miscellaneous/changelog.html
>
> "Remove all API methods (and supporting code) that are not used by Tomcat
> 10.1.x to support the use of OpenSSL as a replacement for JSSE to provide
> TLS functionality. (markt)”
>
> Is there a way to fix this, or has tomcat just broken everything for
> anyone in a RHEL environment?
>

"tomcat" has broken nothing ;)

Assuming that you're using tomcat-native from EPEL 9 (because that's the
only place to get tomcat-native for RHEL at the moment), the only option is
to downgrade to the build from Aug 2022 (tomcat-native-1.2.35-1.el9). I
reopened a similar issue https://bugzilla.redhat.com/show_bug.cgi?id=2124703
to address this; please watch/engage there with any more questions/comments
(unless they are purely tomcat/tomcat-native related). Also, if you feel
that you want to document your unique problem in more detail, you can
comment or open a new bugzilla issue against Fedora EPEL 9 too.

Apologies for the inconvenience.


> Regards,
> Graham
> —
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>