[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
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
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
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
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)
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)
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
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
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
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
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
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
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
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
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
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] rmannibucau commented on pull request #399: Add getRequest method to RequestFacade, to get the wrapped Request
rmannibucau commented on pull request #399: URL: https://github.com/apache/tomcat/pull/399#issuecomment-752093847 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 (or Unwrappable instance). It would solve a lot of issues with such pattern - being said the getRequest() does not really work in all cases in your example - and enables to traverse proxies/wrappers. Wdyt? 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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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