[tomcat-jakartaee-migration] branch master updated: Convert the javax.jms package with the EE profile (Fixes #6). The previous fix c094325 targeted javax.jmx - Which is NOT touched by Jakarta EE it se

2020-12-29 Thread mgrigorov 
This is an automated email from the ASF dual-hosted git repository.

mgrigorov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new 7d180b2  Convert the javax.jms package with the EE profile (Fixes #6). 
The previous fix c094325 targeted javax.jmx - Which is NOT touched by Jakarta 
EE it seems
 new 78287b2  Merge pull request #7 from alitokmen/master
7d180b2 is described below

commit 7d180b26db201ffd3e7b00c62ff73d7853eea6e6
Author: alitokmen 
AuthorDate: Wed Dec 30 07:42:57 2020 +0100

Convert the javax.jms package with the EE profile (Fixes #6). The previous 
fix c094325 targeted javax.jmx - Which is NOT touched by Jakarta EE it seems
---
 src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java | 2 +-
 src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java 
b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
index 32781c1..78d4103 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
@@ -26,7 +26,7 @@ public enum EESpecProfile {
 
 
TOMCAT("javax([/\\.](annotation(?![/\\.]processing)|ejb|el|mail|persistence|security[/\\.]auth[/\\.]message|servlet|transaction(?![/\\.]xa)|websocket))"),
 
-
EE("javax([/\\.](activation|annotation(?![/\\.]processing)|decorator|ejb|el|enterprise|jmx|json|interceptor|inject|mail|persistence|"
+
EE("javax([/\\.](activation|annotation(?![/\\.]processing)|decorator|ejb|el|enterprise|jms|json|interceptor|inject|mail|persistence|"
 + 
"security[/\\.]auth[/\\.]message|servlet|transaction(?![/\\.]xa)|validation|websocket|ws[/\\.]rs|"
 + 
"xml[/\\.](bind|namespace|rpc|soap|stream|ws|XMLConstants)))");
 
diff --git a/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java 
b/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
index bab8010..1c97339 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
@@ -41,7 +41,7 @@ public class EESpecProfileTest {
 assertEquals("javax.activation", profile.convert("javax.activation"));
 assertEquals("javax.decorator", profile.convert("javax.decorator"));
 assertEquals("javax.enterprise", profile.convert("javax.enterprise"));
-assertEquals("javax.jmx", profile.convert("javax.jmx"));
+assertEquals("javax.jms", profile.convert("javax.jms"));
 assertEquals("javax.json", profile.convert("javax.json"));
 assertEquals("javax.interceptor", 
profile.convert("javax.interceptor"));
 assertEquals("javax.inject", profile.convert("javax.inject"));
@@ -73,7 +73,7 @@ public class EESpecProfileTest {
 assertEquals("jakarta.ejb", profile.convert("javax.ejb"));
 assertEquals("jakarta.el", profile.convert("javax.el"));
 assertEquals("jakarta.enterprise", 
profile.convert("javax.enterprise"));
-assertEquals("jakarta.jmx", profile.convert("javax.jmx"));
+assertEquals("jakarta.jms", profile.convert("javax.jms"));
 assertEquals("jakarta.json", profile.convert("javax.json"));
 assertEquals("jakarta.interceptor", 
profile.convert("javax.interceptor"));
 assertEquals("jakarta.inject", profile.convert("javax.inject"));


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

[GitHub] [tomcat-jakartaee-migration] martin-g commented on pull request #7: Convert the javax.jms package with the EE profile (Fixes #6). The previous fix c094325 targeted javax.jmx - Which is NOT to

2020-12-29 Thread GitBox 


martin-g commented on pull request #7:
URL: 
https://github.com/apache/tomcat-jakartaee-migration/pull/7#issuecomment-752351427


   Thank you, @alitokmen !



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.

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

[GitHub] [tomcat-jakartaee-migration] martin-g merged pull request #7: Convert the javax.jms package with the EE profile (Fixes #6). The previous fix c094325 targeted javax.jmx - Which is NOT touched

2020-12-29 Thread GitBox 


martin-g merged pull request #7:
URL: https://github.com/apache/tomcat-jakartaee-migration/pull/7


   



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.

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

[GitHub] [tomcat-jakartaee-migration] alitokmen opened a new pull request #7: Convert the javax.jms package with the EE profile (Fixes #6). The previous fix c094325 targeted javax.jmx - Which is NOT t

2020-12-29 Thread GitBox 


alitokmen opened a new pull request #7:
URL: https://github.com/apache/tomcat-jakartaee-migration/pull/7


   



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.

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: [tomcat-jakartaee-migration] branch master updated: Convert the javax.jms package with the EE profile (Fixes #6)

2020-12-29 Thread Rémy Maucherat 
On Tue, Dec 29, 2020 at 11:03 PM  wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> ebourg pushed a commit to branch master
> in repository
> https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>  new c094325  Convert the javax.jms package with the EE profile (Fixes
> #6)
> c094325 is described below
>
> commit c0943251b8fd3d7570d6c13cf461cd939aaf9ab0
> Author: Emmanuel Bourg 
> AuthorDate: Tue Dec 29 23:03:17 2020 +0100
>
> Convert the javax.jms package with the EE profile (Fixes #6)
> ---
>  src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java | 2 +-
>  src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
> b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
> index d4ae075..32781c1 100644
> --- a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
> +++ b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
> @@ -26,7 +26,7 @@ public enum EESpecProfile {
>
>
>  
> TOMCAT("javax([/\\.](annotation(?![/\\.]processing)|ejb|el|mail|persistence|security[/\\.]auth[/\\.]message|servlet|transaction(?![/\\.]xa)|websocket))"),
>
> -
> EE("javax([/\\.](activation|annotation(?![/\\.]processing)|decorator|ejb|el|enterprise|json|interceptor|inject|mail|persistence|"
> +
> EE("javax([/\\.](activation|annotation(?![/\\.]processing)|decorator|ejb|el|enterprise|jmx|json|interceptor|inject|mail|persistence|"
>  +
> "security[/\\.]auth[/\\.]message|servlet|transaction(?![/\\.]xa)|validation|websocket|ws[/\\.]rs|"
>  +
> "xml[/\\.](bind|namespace|rpc|soap|stream|ws|XMLConstants)))");
>
> diff --git
> a/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
> b/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
> index 0eba27f..bab8010 100644
> --- a/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
> +++ b/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
> @@ -41,6 +41,7 @@ public class EESpecProfileTest {
>  assertEquals("javax.activation",
> profile.convert("javax.activation"));
>  assertEquals("javax.decorator",
> profile.convert("javax.decorator"));
>  assertEquals("javax.enterprise",
> profile.convert("javax.enterprise"));
> +assertEquals("javax.jmx", profile.convert("javax.jmx"));
>

jmx -> jms typo ?

Rémy

 assertEquals("javax.json", profile.convert("javax.json"));
>  assertEquals("javax.interceptor",
> profile.convert("javax.interceptor"));
>  assertEquals("javax.inject", profile.convert("javax.inject"));
> @@ -72,6 +73,7 @@ public class EESpecProfileTest {
>  assertEquals("jakarta.ejb", profile.convert("javax.ejb"));
>  assertEquals("jakarta.el", profile.convert("javax.el"));
>  assertEquals("jakarta.enterprise",
> profile.convert("javax.enterprise"));
> +assertEquals("jakarta.jmx", profile.convert("javax.jmx"));
>  assertEquals("jakarta.json", profile.convert("javax.json"));
>  assertEquals("jakarta.interceptor",
> profile.convert("javax.interceptor"));
>  assertEquals("jakarta.inject", profile.convert("javax.inject"));
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

[tomcat-jakartaee-migration] branch master updated: Convert the javax.jms package with the EE profile (Fixes #6)

2020-12-29 Thread ebourg 
This is an automated email from the ASF dual-hosted git repository.

ebourg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new c094325  Convert the javax.jms package with the EE profile (Fixes #6)
c094325 is described below

commit c0943251b8fd3d7570d6c13cf461cd939aaf9ab0
Author: Emmanuel Bourg 
AuthorDate: Tue Dec 29 23:03:17 2020 +0100

Convert the javax.jms package with the EE profile (Fixes #6)
---
 src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java | 2 +-
 src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java 
b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
index d4ae075..32781c1 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
@@ -26,7 +26,7 @@ public enum EESpecProfile {
 
 
TOMCAT("javax([/\\.](annotation(?![/\\.]processing)|ejb|el|mail|persistence|security[/\\.]auth[/\\.]message|servlet|transaction(?![/\\.]xa)|websocket))"),
 
-
EE("javax([/\\.](activation|annotation(?![/\\.]processing)|decorator|ejb|el|enterprise|json|interceptor|inject|mail|persistence|"
+
EE("javax([/\\.](activation|annotation(?![/\\.]processing)|decorator|ejb|el|enterprise|jmx|json|interceptor|inject|mail|persistence|"
 + 
"security[/\\.]auth[/\\.]message|servlet|transaction(?![/\\.]xa)|validation|websocket|ws[/\\.]rs|"
 + 
"xml[/\\.](bind|namespace|rpc|soap|stream|ws|XMLConstants)))");
 
diff --git a/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java 
b/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
index 0eba27f..bab8010 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/EESpecProfileTest.java
@@ -41,6 +41,7 @@ public class EESpecProfileTest {
 assertEquals("javax.activation", profile.convert("javax.activation"));
 assertEquals("javax.decorator", profile.convert("javax.decorator"));
 assertEquals("javax.enterprise", profile.convert("javax.enterprise"));
+assertEquals("javax.jmx", profile.convert("javax.jmx"));
 assertEquals("javax.json", profile.convert("javax.json"));
 assertEquals("javax.interceptor", 
profile.convert("javax.interceptor"));
 assertEquals("javax.inject", profile.convert("javax.inject"));
@@ -72,6 +73,7 @@ public class EESpecProfileTest {
 assertEquals("jakarta.ejb", profile.convert("javax.ejb"));
 assertEquals("jakarta.el", profile.convert("javax.el"));
 assertEquals("jakarta.enterprise", 
profile.convert("javax.enterprise"));
+assertEquals("jakarta.jmx", profile.convert("javax.jmx"));
 assertEquals("jakarta.json", profile.convert("javax.json"));
 assertEquals("jakarta.interceptor", 
profile.convert("javax.interceptor"));
 assertEquals("jakarta.inject", profile.convert("javax.inject"));


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

[GitHub] [tomcat-jakartaee-migration] ebourg closed issue #6: Also rename JMS, Java Mail and other web.xml definitions

2020-12-29 Thread GitBox 


ebourg closed issue #6:
URL: https://github.com/apache/tomcat-jakartaee-migration/issues/6


   



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.

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

[GitHub] [tomcat-jakartaee-migration] alitokmen opened a new issue #6: Also rename JMS, Java Mail and other web.xml definitions

2020-12-29 Thread GitBox 


alitokmen opened a new issue #6:
URL: https://github.com/apache/tomcat-jakartaee-migration/issues/6


   Currently, the Jakarta EE migration tool seems to be leaving definitions in 
`web.xml` such as the below untouched:
   
   ```
 
   The test JMS queue
   jms/MyQueue
   javax.jms.Queue
   Container
 
   ```
   
   These also need to be adapted, to use the Jakarta EE equivalents.



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.

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

[GitHub] [tomcat] arjantijms commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


arjantijms commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752229567


   > It's hard to see how this concept can ever be a good idea.
   
   I personally think it's a brilliant idea. Then again, I'm probably biased ;)



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.

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

[GitHub] [tomcat] rmaucher commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


rmaucher commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752216538


   The plan was to keep the Servlet container implementation classes away from 
the webapps, so this is a request to do the opposite. It's hard to see how this 
concept can ever be a good idea.



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.

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

[GitHub] [tomcat] rmannibucau commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


rmannibucau commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752179364


   @markt-asf can we add a org.apache.catalina.api.Unwrappable and make the 
facade objects impl it? Would it be an option?



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.

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

[GitHub] [tomcat] rmannibucau commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


rmannibucau commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752178778


   @arjantijms maybe test the type and throw an exception with the request type 
when it is not a wrapper, some people do it :angel: ;)



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.

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

[GitHub] [tomcat] arjantijms commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


arjantijms commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752177666


   @rmannibucau oh yeah, of course, then it won't work. It'll throw an 
exception, so at least you know it won't work ;)
   
   `requestInitialized()` is really early though, so in most cases should be 
early enough. Just to be sure I'll take your suggestion and add a default 
unwrapFully:
   
   ```java
   @SuppressWarnings("unchecked")
   private  T unwrapFully(ServletRequest request) 
{
   ServletRequest currentRequest = request;
   while (currentRequest instanceof ServletRequestWrapper) {
   ServletRequestWrapper wrapper = (ServletRequestWrapper) 
currentRequest;
   currentRequest = wrapper.getRequest();
   }
   return (T) currentRequest;
   }
   ```
   
   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.

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

[GitHub] [tomcat] rmannibucau commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


rmannibucau commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752173068


   @arjantijms when you can't cast the request  because a filter/valve wrapped 
it (even if a bit nasty for valves it can happen) which is not uncommon for 
session distribution, security (potentially ran before jaspic) or framework 
(cdi for ex) setup. I know we are all tempated to say security first but 
actually it is common to put distribution and framework before just to ensure 
security context exists properly.



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.

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

[GitHub] [tomcat] arjantijms commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


arjantijms commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752169165


   >  being said the getRequest() does not really work in all cases in your 
example
   
   In what cases doesn't it work?
   
   > Guess a sane EE way to solve that is to propose to servlet spec to add an 
unwrap(Class type) method in most of its objects 
   
   Might not be a bad idea, 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.

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

[GitHub] [tomcat] arjantijms commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


arjantijms commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752074636


   > I share the concern that adding a getRequest() method may undermine the 
use of a facade.
   
   Isn't the facade intended as a service to the user to not accidentally 
access specifics? It's not a protection mechanism, is it? As with reflection 
one can get to the wrapped instance anyway.
   
   With a `getRequest()` method, the user has to do 2 very specific actions: 
cast to `RequestFacade` and then call that method. Compare to e.g. JPA's 
EntityManager, which is a facade for (mainly) Hibernate, but still has a method 
to get the underlying Hibernate specific manager.



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.

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

[GitHub] [tomcat] arjantijms commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


arjantijms commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752072520


   I'll investigate the application specific Valve. Can META-INF/context.xml 
exist on the class path as well, or does it have to be inside [war 
root]/META-INF?
   
   > Use reflection as several other integrators
   
   I'm now using reflection indeed:
   
   ```java
   private static Subject getSubject(HttpServletRequest httpServletRequest) 
{
   return (Subject) 
getRequest(httpServletRequest).getNote(REQ_JASPIC_SUBJECT_NOTE);
   }
   
   private static Request getRequest(HttpServletRequest servletRequest) {
   return getRequest((RequestFacade) servletRequest);
   }
   
   private static Request getRequest(RequestFacade facade) {
   try {
   Field requestField = 
RequestFacade.class.getDeclaredField("request");
   requestField.setAccessible(true);
   
   return (Request) requestField.get(facade);
   } catch (NoSuchFieldException | SecurityException | 
IllegalArgumentException | IllegalAccessException e) {
   throw new IllegalStateException(e);
   }
   }
   ```
   
   @markt-asf I'm working on a standalone implementation of Jakarta 
Authorization (JACC), and so far it works really well on Tomcat (and Piranha). 
It's actually interesting to see how the JACC way to match URL patterns and the 
Tomcat native way are really not that different.
   
   
   



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.

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

buildbot success in on tomcat-85-trunk

2020-12-29 Thread buildbot 
The Buildbot has detected a restored build on builder tomcat-85-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-85-trunk/builds/2577

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-85-commit' 
triggered this build
Build Source Stamp: [branch 8.5.x] 95ba57df171531fff8da664b4a3a31889f258582
Blamelist: remm 

Build succeeded!

Sincerely,
 -The Buildbot




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

[GitHub] [tomcat] markt-asf commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


markt-asf commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-752030086


   Valves can be installed via a WAR. You can specify an application specific 
Valve in a META-INF/context.xml file included in the WAR.
   I share the concern that adding a getRequest() method may undermine the use 
of a facade.



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.

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

buildbot success in on tomcat-9-trunk

2020-12-29 Thread buildbot 
The Buildbot has detected a restored build on builder tomcat-9-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-9-trunk/builds/588

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-9-commit' 
triggered this build
Build Source Stamp: [branch 9.0.x] c906bea693e56fd517b695704c88f1f5271fc553
Blamelist: remm 

Build succeeded!

Sincerely,
 -The Buildbot




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

buildbot failure in on tomcat-trunk

2020-12-29 Thread buildbot 
The Buildbot has detected a new failure on builder tomcat-trunk while building 
tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/5610

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-commit' 
triggered this build
Build Source Stamp: [branch master] 704e37a4fd21d16e48e45e78823f371598fbb02e
Blamelist: remm 

BUILD FAILED: failed compile_1

Sincerely,
 -The Buildbot




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

[Bug 65033] Tomcat 8.5.60/61 User authentication with JNDIRealm failure

2020-12-29 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=65033

--- Comment #9 from Satya  ---
Thanks for reply. we will test with Tomcat 8.5.62 once its released.

-- 
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] 01/02: Accurate version of the error test

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

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

commit 4246e2cc6586dfb3fa819f94c89cd6a6db0acdbc
Author: remm 
AuthorDate: Tue Dec 29 10:47:14 2020 +0100

Accurate version of the error test
---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index e6fe1f5..797f370 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -20,6 +20,8 @@ import java.lang.reflect.Field;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
@@ -112,8 +114,6 @@ public class TestJNDIRealm {
 Assert.assertEquals(ha1(), 
((GenericPrincipal)principal).getPassword());
 }
 
-volatile int count = 0;
-
 @Test
 public void testErrorRealm() throws Exception {
 Context context = new TesterContext();
@@ -124,13 +124,12 @@ public class TestJNDIRealm {
 realm.setConnectionURL("ldap://127.0.0.1:12345;);
 realm.start();
 
-count = 0;
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-Thread.sleep(10);
+final CountDownLatch latch = new CountDownLatch(3);
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
 
-Assert.assertEquals(3, count);
+Assert.assertTrue(latch.await(30, TimeUnit.SECONDS));
 }
 
 


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

[tomcat] 02/02: Remove lambdas

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

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

commit 95ba57df171531fff8da664b4a3a31889f258582
Author: remm 
AuthorDate: Tue Dec 29 10:55:12 2020 +0100

Remove lambdas
---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 797f370..033a292 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -117,7 +117,7 @@ public class TestJNDIRealm {
 @Test
 public void testErrorRealm() throws Exception {
 Context context = new TesterContext();
-JNDIRealm realm = new JNDIRealm();
+final JNDIRealm realm = new JNDIRealm();
 realm.setContainer(context);
 realm.setUserSearch("");
 // Connect to something that will fail
@@ -125,9 +125,16 @@ public class TestJNDIRealm {
 realm.start();
 
 final CountDownLatch latch = new CountDownLatch(3);
-(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+Runnable testThread = new Runnable() {
+@Override
+public void run() {
+realm.authenticate("foo", "bar");
+latch.countDown();
+}
+};
+(new Thread(testThread)).start();
+(new Thread(testThread)).start();
+(new Thread(testThread)).start();
 
 Assert.assertTrue(latch.await(30, TimeUnit.SECONDS));
 }


-
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 (59d9a28 -> 95ba57d)

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

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


from 59d9a28  65033: Fix JNDI realm error handling
 new 4246e2c  Accurate version of the error test
 new 95ba57d  Remove lambdas

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 test/org/apache/catalina/realm/TestJNDIRealm.java | 26 ++-
 1 file changed, 16 insertions(+), 10 deletions(-)


-
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: Accurate version of the error test

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm 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 c906bea  Accurate version of the error test
c906bea is described below

commit c906bea693e56fd517b695704c88f1f5271fc553
Author: remm 
AuthorDate: Tue Dec 29 10:47:14 2020 +0100

Accurate version of the error test
---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 62a7495..bbe471e 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -20,6 +20,8 @@ import java.lang.reflect.Field;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
@@ -112,8 +114,6 @@ public class TestJNDIRealm {
 Assert.assertEquals(ha1(), 
((GenericPrincipal)principal).getPassword());
 }
 
-volatile int count = 0;
-
 @Test
 public void testErrorRealm() throws Exception {
 Context context = new TesterContext();
@@ -124,13 +124,12 @@ public class TestJNDIRealm {
 realm.setConnectionURL("ldap://127.0.0.1:12345;);
 realm.start();
 
-count = 0;
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-Thread.sleep(10);
+final CountDownLatch latch = new CountDownLatch(3);
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
 
-Assert.assertEquals(3, count);
+Assert.assertTrue(latch.await(30, TimeUnit.SECONDS));
 }
 
 


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

[tomcat] branch master updated: Accurate version of the error test

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new 704e37a  Accurate version of the error test
704e37a is described below

commit 704e37a4fd21d16e48e45e78823f371598fbb02e
Author: remm 
AuthorDate: Tue Dec 29 10:47:14 2020 +0100

Accurate version of the error test
---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 8bc2a01..78412fa 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -20,6 +20,8 @@ import java.lang.reflect.Field;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
@@ -112,8 +114,6 @@ public class TestJNDIRealm {
 Assert.assertEquals(USER, principal.getName());
 }
 
-volatile int count = 0;
-
 @Test
 public void testErrorRealm() throws Exception {
 Context context = new TesterContext();
@@ -124,13 +124,12 @@ public class TestJNDIRealm {
 realm.setConnectionURL("ldap://127.0.0.1:12345;);
 realm.start();
 
-count = 0;
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
-Thread.sleep(10);
+final CountDownLatch latch = new CountDownLatch(3);
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); 
latch.countDown(); })).start();
 
-Assert.assertEquals(3, count);
+Assert.assertTrue(latch.await(30, TimeUnit.SECONDS));
 }
 
 


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

buildbot failure in on tomcat-85-trunk

2020-12-29 Thread buildbot 
The Buildbot has detected a new failure on builder tomcat-85-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-85-trunk/builds/2576

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-85-commit' 
triggered this build
Build Source Stamp: [branch 8.5.x] 59d9a285a3101ba8f333d55c704560b08fcd8e30
Blamelist: remm 

BUILD FAILED: failed compile_1

Sincerely,
 -The Buildbot




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

buildbot failure in on tomcat-9-trunk

2020-12-29 Thread buildbot 
The Buildbot has detected a new failure on builder tomcat-9-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-9-trunk/builds/587

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-9-commit' 
triggered this build
Build Source Stamp: [branch 9.0.x] 130843d0126f84e4027cdb858f77c55bb54ffe3f
Blamelist: remm 

BUILD FAILED: failed compile_1

Sincerely,
 -The Buildbot




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

[Bug 65033] Tomcat 8.5.60/61 User authentication with JNDIRealm failure

2020-12-29 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=65033

Remy Maucherat  changed:

   What|Removed |Added

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

--- Comment #8 from Remy Maucherat  ---
I added a test case for this. The fix will be in Tomcat 10.0.1, 9.0.42 and
8.5.62.

-- 
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 8.5.x updated: 65033: Fix JNDI realm error handling

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm 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 59d9a28  65033: Fix JNDI realm error handling
59d9a28 is described below

commit 59d9a285a3101ba8f333d55c704560b08fcd8e30
Author: remm 
AuthorDate: Tue Dec 29 09:44:41 2020 +0100

65033: Fix JNDI realm error handling

Connecting to a failed server when pooling was not enabled would cause
threads to hang in authenticate since unlocking was not done. Closing
the connection was correctly present in that case before the refactoring
that added pooling.
---
 java/org/apache/catalina/realm/JNDIRealm.java | 15 ---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 21 +
 webapps/docs/changelog.xml|  4 
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java 
b/java/org/apache/catalina/realm/JNDIRealm.java
index daa76bf..1eedab6 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1309,10 +1309,11 @@ public class JNDIRealm extends RealmBase {
 // Ensure that we have a directory context available
 connection = get();
 
-// Occasionally the directory context will timeout.  Try one more
-// time before giving up.
 try {
 
+// Occasionally the directory context will timeout.  Try one 
more
+// time before giving up.
+
 // Authenticate the specified username if possible
 principal = authenticate(connection, username, credentials);
 
@@ -1358,6 +1359,10 @@ public class JNDIRealm extends RealmBase {
 // Log the problem for posterity
 containerLog.error(sm.getString("jndiRealm.exception"), e);
 
+// close the connection so we know it will be reopened.
+close(connection);
+closePooledConnections();
+
 // Return "not authenticated" for this request
 if (containerLog.isDebugEnabled())
 containerLog.debug("Returning null principal.");
@@ -2200,8 +2205,12 @@ public class JNDIRealm extends RealmBase {
 protected void close(JNDIConnection connection) {
 
 // Do nothing if there is no opened connection
-if (connection.context == null)
+if (connection == null || connection.context == null) {
+if (connectionPool == null) {
+singleConnectionLock.unlock();
+}
 return;
+}
 
 // Close tls startResponse if used
 if (tls != null) {
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 5ffac18..e6fe1f5 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -112,6 +112,27 @@ public class TestJNDIRealm {
 Assert.assertEquals(ha1(), 
((GenericPrincipal)principal).getPassword());
 }
 
+volatile int count = 0;
+
+@Test
+public void testErrorRealm() throws Exception {
+Context context = new TesterContext();
+JNDIRealm realm = new JNDIRealm();
+realm.setContainer(context);
+realm.setUserSearch("");
+// Connect to something that will fail
+realm.setConnectionURL("ldap://127.0.0.1:12345;);
+realm.start();
+
+count = 0;
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+Thread.sleep(10);
+
+Assert.assertEquals(3, count);
+}
+
 
 private JNDIRealm buildRealm(String password) throws 
javax.naming.NamingException,
 NoSuchFieldException, IllegalAccessException, LifecycleException {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6755a79..0737cd0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -149,6 +149,10 @@
 Avoid uncaught InaccessibleObjectException on Java 16 trying to clear
 references threads. (remm)
   
+  
+65033: Fix JNDI realm error handling when connecting to a
+failed server when pooling was not enabled. (remm)
+  
 
   
   


-
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: 65033: Fix JNDI realm error handling

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm 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 130843d  65033: Fix JNDI realm error handling
130843d is described below

commit 130843d0126f84e4027cdb858f77c55bb54ffe3f
Author: remm 
AuthorDate: Tue Dec 29 09:44:41 2020 +0100

65033: Fix JNDI realm error handling

Connecting to a failed server when pooling was not enabled would cause
threads to hang in authenticate since unlocking was not done. Closing
the connection was correctly present in that case before the refactoring
that added pooling.
---
 java/org/apache/catalina/realm/JNDIRealm.java | 15 ---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 21 +
 webapps/docs/changelog.xml|  4 
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java 
b/java/org/apache/catalina/realm/JNDIRealm.java
index 3d952c0..7e2d578 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1301,10 +1301,11 @@ public class JNDIRealm extends RealmBase {
 // Ensure that we have a directory context available
 connection = get();
 
-// Occasionally the directory context will timeout.  Try one more
-// time before giving up.
 try {
 
+// Occasionally the directory context will timeout.  Try one 
more
+// time before giving up.
+
 // Authenticate the specified username if possible
 principal = authenticate(connection, username, credentials);
 
@@ -1350,6 +1351,10 @@ public class JNDIRealm extends RealmBase {
 // Log the problem for posterity
 containerLog.error(sm.getString("jndiRealm.exception"), e);
 
+// close the connection so we know it will be reopened.
+close(connection);
+closePooledConnections();
+
 // Return "not authenticated" for this request
 if (containerLog.isDebugEnabled())
 containerLog.debug("Returning null principal.");
@@ -2192,8 +2197,12 @@ public class JNDIRealm extends RealmBase {
 protected void close(JNDIConnection connection) {
 
 // Do nothing if there is no opened connection
-if (connection.context == null)
+if (connection == null || connection.context == null) {
+if (connectionPool == null) {
+singleConnectionLock.unlock();
+}
 return;
+}
 
 // Close tls startResponse if used
 if (tls != null) {
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 00180e1..62a7495 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -112,6 +112,27 @@ public class TestJNDIRealm {
 Assert.assertEquals(ha1(), 
((GenericPrincipal)principal).getPassword());
 }
 
+volatile int count = 0;
+
+@Test
+public void testErrorRealm() throws Exception {
+Context context = new TesterContext();
+JNDIRealm realm = new JNDIRealm();
+realm.setContainer(context);
+realm.setUserSearch("");
+// Connect to something that will fail
+realm.setConnectionURL("ldap://127.0.0.1:12345;);
+realm.start();
+
+count = 0;
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+Thread.sleep(10);
+
+Assert.assertEquals(3, count);
+}
+
 
 private JNDIRealm buildRealm(String password) throws 
javax.naming.NamingException,
 NoSuchFieldException, IllegalAccessException, LifecycleException {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index da0a0d0..6fed570 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -142,6 +142,10 @@
 Avoid uncaught InaccessibleObjectException on Java 16 trying to clear
 references threads. (remm)
   
+  
+65033: Fix JNDI realm error handling when connecting to a
+failed server when pooling was not enabled. (remm)
+  
 
   
   


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

[tomcat] branch master updated: 65033: Fix JNDI realm error handling

2020-12-29 Thread remm 
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new 55f2f35  65033: Fix JNDI realm error handling
55f2f35 is described below

commit 55f2f355c778dae68a4a154cc6eaf10e997872c9
Author: remm 
AuthorDate: Tue Dec 29 09:44:41 2020 +0100

65033: Fix JNDI realm error handling

Connecting to a failed server when pooling was not enabled would cause
threads to hang in authenticate since unlocking was not done. Closing
the connection was correctly present in that case before the refactoring
that added pooling.
---
 java/org/apache/catalina/realm/JNDIRealm.java | 15 ---
 test/org/apache/catalina/realm/TestJNDIRealm.java | 21 +
 webapps/docs/changelog.xml|  4 
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java 
b/java/org/apache/catalina/realm/JNDIRealm.java
index e7543d8..7e09d5c 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1301,10 +1301,11 @@ public class JNDIRealm extends RealmBase {
 // Ensure that we have a directory context available
 connection = get();
 
-// Occasionally the directory context will timeout.  Try one more
-// time before giving up.
 try {
 
+// Occasionally the directory context will timeout.  Try one 
more
+// time before giving up.
+
 // Authenticate the specified username if possible
 principal = authenticate(connection, username, credentials);
 
@@ -1350,6 +1351,10 @@ public class JNDIRealm extends RealmBase {
 // Log the problem for posterity
 containerLog.error(sm.getString("jndiRealm.exception"), e);
 
+// close the connection so we know it will be reopened.
+close(connection);
+closePooledConnections();
+
 // Return "not authenticated" for this request
 if (containerLog.isDebugEnabled())
 containerLog.debug("Returning null principal.");
@@ -2192,8 +2197,12 @@ public class JNDIRealm extends RealmBase {
 protected void close(JNDIConnection connection) {
 
 // Do nothing if there is no opened connection
-if (connection.context == null)
+if (connection == null || connection.context == null) {
+if (connectionPool == null) {
+singleConnectionLock.unlock();
+}
 return;
+}
 
 // Close tls startResponse if used
 if (tls != null) {
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java 
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 845bc61..8bc2a01 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -112,6 +112,27 @@ public class TestJNDIRealm {
 Assert.assertEquals(USER, principal.getName());
 }
 
+volatile int count = 0;
+
+@Test
+public void testErrorRealm() throws Exception {
+Context context = new TesterContext();
+JNDIRealm realm = new JNDIRealm();
+realm.setContainer(context);
+realm.setUserSearch("");
+// Connect to something that will fail
+realm.setConnectionURL("ldap://127.0.0.1:12345;);
+realm.start();
+
+count = 0;
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+(new Thread(() -> { realm.authenticate("foo", "bar"); count++; 
})).start();
+Thread.sleep(10);
+
+Assert.assertEquals(3, count);
+}
+
 
 private JNDIRealm buildRealm(String password) throws 
javax.naming.NamingException,
 NoSuchFieldException, IllegalAccessException, LifecycleException {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7576672..2373340 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -142,6 +142,10 @@
 Avoid uncaught InaccessibleObjectException on Java 16 trying to clear
 references threads. (remm)
   
+  
+65033: Fix JNDI realm error handling when connecting to a
+failed server when pooling was not enabled. (remm)
+  
 
   
   


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

[GitHub] [tomcat] rmannibucau commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request

2020-12-29 Thread GitBox 


rmannibucau commented on pull request #399:
URL: https://github.com/apache/tomcat/pull/399#issuecomment-751987522


   @arjantijms I see, my point is that the facade is about preventing the user 
to access the internals (request here) or any API not from the spec so I don't 
think it should be broken. It is done for all servlet code (look for all 
*Facade instances) and it generally prevents to access any internal from a spec 
code (you see it the other way for your case but it is designed the other way). 
I see a few options to help - there are probably more:
   
   1. Add an "app valve" in tomcat code, it would reuse tomcat reflection 
factory to instantiate lazily - when app loader exists - a valve. it is close 
to 
https://github.com/apache/tomee/blob/master/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/valve/LazyValve.java
 but only using tomcat internal. It enables you to write a valve in your war 
without tomcat tuning. Similar solution would enable a valve to fail to be 
created from the context.xml but it would be retried when the app loader exists 
(I prefer previous option which is clearer on when it must work). I know it is 
not tomcat habit to enable internals from the app but I agree it is not 
uncommon so having some wrappers enabling it can help - tomee also has the real 
wrapper since you mentionned it.
   2. Use reflection as several other integrators
   3. Drop jaspic from tomcat and use plain servlet filters (big +1 from me 
since it would also make jaspic optional for tomcat embed)
   4. Use GenericPrincipal (or TomcatPrincipal/ a new JaspicWrappingPrincipal) 
to host the subject instead of a note
   5. add in base authenticator an attribute to set it in a request attribute 
too (likely the easiest)



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.

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