[tomcat] branch 7.0.x updated: Guard new escape routines for null values

2021-05-18 Thread fschumacher
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/7.0.x by this push:
 new 7d709b5  Guard new escape routines for null values
7d709b5 is described below

commit 7d709b57592103e28a6a66e9a4017b8af4d515bd
Author: Felix Schumacher 
AuthorDate: Sat May 15 14:06:21 2021 +0200

Guard new escape routines for null values

NPE in JNDIRealm, when userRoleAttribute is not set.
Plus add path to UnboundID SDK to the Eclipse and Intellij
classpath settings.

Bugzilla Id: 63508
---
 java/org/apache/catalina/realm/JNDIRealm.java | 6 ++
 webapps/docs/changelog.xml| 8 
 2 files changed, 14 insertions(+)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java 
b/java/org/apache/catalina/realm/JNDIRealm.java
index 6425194..887d6d1 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -2769,6 +2769,9 @@ public class JNDIRealm extends RealmBase {
  * @return String the escaped/encoded result
  */
 protected String doFilterEscaping(String inString) {
+if (inString == null) {
+return null;
+}
 StringBuilder buf = new StringBuilder(inString.length());
 for (int i = 0; i < inString.length(); i++) {
 char c = inString.charAt(i);
@@ -2861,6 +2864,9 @@ public class JNDIRealm extends RealmBase {
  * @return  The string representation of the attribute value
  */
 protected String doAttributeValueEscaping(String input) {
+if (input == null) {
+return null;
+}
 int len = input.length();
 StringBuilder result = new StringBuilder();
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ef125b2..1994d8c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,14 @@
   issues do not "pop up" wrt. others).
 -->
 
+  
+
+  
+63508: NPE in JNDIRealm when no 
userRoleAttribute
+is given. (fschumacher)
+  
+
+  
   
 
   

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



Re: JSP code generation optimizations

2021-05-18 Thread Mark Thomas

On 18/05/2021 15:42, Christopher Schultz wrote:

Mark,

On 5/17/21 14:47, Mark Thomas wrote:

Hi all,

I am looking at some of the optimizations requested in [1]

One of the examples is show below. Code like this is used when the JSP 
calls a tag.



private boolean _jspx_meth_mytag_005fhelloWorld_005f0(
 jakarta.servlet.jsp.PageContext _jspx_page_context)
 throws java.lang.Throwable {
 jakarta.servlet.jsp.PageContext pageContext = _jspx_page_context;
 //  mytag:helloWorld
 
}

The question is, can the generated code above be changed to something 
like this:


private boolean _jspx_meth_mytag_005fhelloWorld_005f0(
 final jakarta.servlet.jsp.PageContext pageContext)
 throws java.lang.Throwable {
 //  mytag:helloWorld
 
}


To what end? Are you trying to remove a line of code? Are you trying to 
remove another local variable? Improve performance?


Yes, yes and yes. :)

This was one small part of a group of changes that made a measurable 
difference at scale:

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

How much this specific change contributed is TBD. I suspect only a small 
amount.


I am assuming that the aliasing of _jspx_page_context to pageContext 
is so that _jspx_page_context always references the original even if 
the tag decides to do something like:


pageContext = new MyCustomPageContextImpl();

The question is, is behaviour like this allowed?

I can't see anything in the spec that disallows this.

I don't think the aliasing is a defence against malicious apps as it 
is only the implicit objects that are protected this way. Internal 
objects (those name _jsp*) are unprotected.


Given that the original authors (who were on the JSP EG at the time) 
added this aliasing for the implicit objects, it looks like the 
intention was to allow applications to manipulate/replace the implicit 
objects if they wish - without breaking Jasper.


If the above assumptions are correct, what do we want to do about [1]? 
The aliasing is often unnecessary but will sometimes be essential. Do 
we reject this optimization request? Do we add a configuration option? 
Do we try and make this aspect of the code generation pluggable 
somehow? Something else?


If you could add an option like "make global-local-aliases final" and 
then just add "final" to the declaration of that local variable when 
it's "true" (and default==true), that would be a Good Thing. Release it 
and see who, if anyone, complains. It's easy to disable, and we'll get 
information on how much of a needed feature it is. I'd only make the 
default==true in 10.x as to not disturb anyone unnecessarily.


Even that was starting to look a little invasive. At this point I think 
I have done all I can for BZ 65124.


Mark

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



[Bug 65311] NioBlockingSelector.BlockPoller.run() may not "wake up" in time

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65311

--- Comment #3 from Mark Thomas  ---
I'd have no objections if you wanted to back-port it now.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 65124] Inefficient generated JSP code

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65124

--- Comment #14 from Mark Thomas  ---
Created attachment 37867
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37867=edit
Sample plug-in for 

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 65124] Inefficient generated JSP code

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65124

--- Comment #13 from Mark Thomas  ---
I've spent some time this afternoon looking at issue 1. Tomcat's tagPlugin
mechanism is the supported way to approach this. It lets you generate your own
optimised code for tags. It is pretty flexible so you could have one plugin per
tag or one plugin for a group of tags.

There are some example implementations in org.apache.jasper.tagplugins.jstl to
give you an idea of what is possible. The JSP samples in the examples webapp
also include some tag plugins although it is worth noting that these are not
enabled to use the plug-ins as the WEB-INF/tagPlugins.xml file is not present.

As an example of how you could reduce simple  calls to a single
line of Java see the sample plugin attached.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Coty Sutherland
On Tue, May 18, 2021 at 7:34 AM Mark Thomas  wrote:

> All,
>
> Things are starting to move forward for Jakarta EE 10 so I think it is
> time for us to create the 10.1.x branch. At the same time, I'd like to
> switch our primary development branches from master to main for all our
> repos.
>
> We would, therefore, end up with the following for the Tomcat repo:
>
> main   - 10.1.x development
> 10.0.x - 10.0.x development/maintenance
> 9.0.x  - 9.0.x development/maintenance
> 8.5.x  - 8.5.x development/maintenance
> 7.0.x  - 7.0.x development/maintenance
>
> There are some git commands each committer will need to run locally for
> each repo to switch from master to main.
>
> I have also been looking into how we can "retire" the 7.0.x branch when
> the time comes (after end of June). I'd like to suggest the following:
> - tag the HEAD of the 7.0.x branch as "7.0.x-archive"
> - delete the 7.0.x branch
>
> That way it won't appear in the list of branches but it is trivial to
> recreate if we need it.
>
> I'd like to get the master->main rename completed and the 10.1.x
> development branch created towards the end of this week (unless there
> are objections or things we need to discuss further).
>
> Comments?
>

+1, happy to hear there's some movement on the spec :D


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


[Bug 65311] NioBlockingSelector.BlockPoller.run() may not "wake up" in time

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65311

--- Comment #2 from Remy Maucherat  ---
Too bad I had to revert the NioBlockingSelector removal, I was too worried of
the initial regression I never tried to backport it again. But nobody has
complained about it in Tomcat 10, so it's likely less risky now.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 65311] NioBlockingSelector.BlockPoller.run() may not "wake up" in time

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65311

Christopher Schultz  changed:

   What|Removed |Added

Summary|NioBlockingSelector.BlockPo |NioBlockingSelector.BlockPo
   |ller.run() may not be   |ller.run() may not "wake
   |wakeup in time  |up" in time

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: JSP code generation optimizations

2021-05-18 Thread Christopher Schultz

Mark,

On 5/17/21 14:47, Mark Thomas wrote:

Hi all,

I am looking at some of the optimizations requested in [1]

One of the examples is show below. Code like this is used when the JSP 
calls a tag.



private boolean _jspx_meth_mytag_005fhelloWorld_005f0(
     jakarta.servlet.jsp.PageContext _jspx_page_context)
     throws java.lang.Throwable {
     jakarta.servlet.jsp.PageContext pageContext = _jspx_page_context;
     //  mytag:helloWorld
     
}

The question is, can the generated code above be changed to something 
like this:


private boolean _jspx_meth_mytag_005fhelloWorld_005f0(
     final jakarta.servlet.jsp.PageContext pageContext)
     throws java.lang.Throwable {
     //  mytag:helloWorld
     
}


To what end? Are you trying to remove a line of code? Are you trying to 
remove another local variable? Improve performance?


I am assuming that the aliasing of _jspx_page_context to pageContext is 
so that _jspx_page_context always references the original even if the 
tag decides to do something like:


pageContext = new MyCustomPageContextImpl();

The question is, is behaviour like this allowed?

I can't see anything in the spec that disallows this.

I don't think the aliasing is a defence against malicious apps as it is 
only the implicit objects that are protected this way. Internal objects 
(those name _jsp*) are unprotected.


Given that the original authors (who were on the JSP EG at the time) 
added this aliasing for the implicit objects, it looks like the 
intention was to allow applications to manipulate/replace the implicit 
objects if they wish - without breaking Jasper.


If the above assumptions are correct, what do we want to do about [1]? 
The aliasing is often unnecessary but will sometimes be essential. Do we 
reject this optimization request? Do we add a configuration option? Do 
we try and make this aspect of the code generation pluggable somehow? 
Something else?


If you could add an option like "make global-local-aliases final" and 
then just add "final" to the declaration of that local variable when 
it's "true" (and default==true), that would be a Good Thing. Release it 
and see who, if anyone, complains. It's easy to disable, and we'll get 
information on how much of a needed feature it is. I'd only make the 
default==true in 10.x as to not disturb anyone unnecessarily.


-chris

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



Re: Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Raymond Augé
+1

I think it's a great plan.

Ray

On Tue., May 18, 2021, 8:16 a.m. Romain Manni-Bucau, 
wrote:

> +1 to move forward even if jakarta is not yet adopted (there is a single
> time direction - at least at our scale ;))
> -0 to break all clones and related toolings/automotion for hypothetical
> reasons, didn't pay in all projects which did AFAIK
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github <
> https://github.com/rmannibucau> |
> LinkedIn  | Book
> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
>
>
> Le mar. 18 mai 2021 à 14:13, Emmanuel Bourg  a écrit :
>
> > Le 2021-05-18 14:10, Emmanuel Bourg a écrit :
> >
> > >> Comments?
> > >
> > > I don't think the 7.0.x branch should be removed, there is no harm
> > > keeping it.
> > >
> > > As for the master->main change I think that's a waste of time for all
> > > of us. i don't buy the argument that "master" is offensive, but by
> > > moving awa
> >
> > Grr message sent too quickly, sorry.
> >
> > I don't want to reopen the debate, please ignore my comment.
> >
> > Emmanuel Bourg
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
> >
>


Re: Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Romain Manni-Bucau
+1 to move forward even if jakarta is not yet adopted (there is a single
time direction - at least at our scale ;))
-0 to break all clones and related toolings/automotion for hypothetical
reasons, didn't pay in all projects which did AFAIK

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le mar. 18 mai 2021 à 14:13, Emmanuel Bourg  a écrit :

> Le 2021-05-18 14:10, Emmanuel Bourg a écrit :
>
> >> Comments?
> >
> > I don't think the 7.0.x branch should be removed, there is no harm
> > keeping it.
> >
> > As for the master->main change I think that's a waste of time for all
> > of us. i don't buy the argument that "master" is offensive, but by
> > moving awa
>
> Grr message sent too quickly, sorry.
>
> I don't want to reopen the debate, please ignore my comment.
>
> Emmanuel Bourg
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Emmanuel Bourg

Le 2021-05-18 14:10, Emmanuel Bourg a écrit :


Comments?


I don't think the 7.0.x branch should be removed, there is no harm 
keeping it.


As for the master->main change I think that's a waste of time for all
of us. i don't buy the argument that "master" is offensive, but by
moving awa


Grr message sent too quickly, sorry.

I don't want to reopen the debate, please ignore my comment.

Emmanuel Bourg

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



Re: Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Emmanuel Bourg

Le 2021-05-18 13:18, Mark Thomas a écrit :

All,

Things are starting to move forward for Jakarta EE 10 so I think it is
time for us to create the 10.1.x branch. At the same time, I'd like to
switch our primary development branches from master to main for all
our repos.

We would, therefore, end up with the following for the Tomcat repo:

main   - 10.1.x development
10.0.x - 10.0.x development/maintenance
9.0.x  - 9.0.x development/maintenance
8.5.x  - 8.5.x development/maintenance
7.0.x  - 7.0.x development/maintenance

There are some git commands each committer will need to run locally
for each repo to switch from master to main.

I have also been looking into how we can "retire" the 7.0.x branch
when the time comes (after end of June). I'd like to suggest the
following:
- tag the HEAD of the 7.0.x branch as "7.0.x-archive"
- delete the 7.0.x branch

That way it won't appear in the list of branches but it is trivial to
recreate if we need it.

I'd like to get the master->main rename completed and the 10.1.x
development branch created towards the end of this week (unless there
are objections or things we need to discuss further).

Comments?


I don't think the 7.0.x branch should be removed, there is no harm 
keeping it.


As for the master->main change I think that's a waste of time for all of 
us. i don't buy the argument that "master" is offensive, but by moving 
awa


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



Re: Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Rémy Maucherat
On Tue, May 18, 2021 at 1:34 PM Mark Thomas  wrote:

> All,
>
> Things are starting to move forward for Jakarta EE 10 so I think it is
> time for us to create the 10.1.x branch. At the same time, I'd like to
> switch our primary development branches from master to main for all our
> repos.
>

Ah good news. I sort of wasn't expecting anything anymore ;)


>
> We would, therefore, end up with the following for the Tomcat repo:
>
> main   - 10.1.x development
> 10.0.x - 10.0.x development/maintenance
> 9.0.x  - 9.0.x development/maintenance
> 8.5.x  - 8.5.x development/maintenance
> 7.0.x  - 7.0.x development/maintenance
>

+1

>
> There are some git commands each committer will need to run locally for
> each repo to switch from master to main.
>
> I have also been looking into how we can "retire" the 7.0.x branch when
> the time comes (after end of June). I'd like to suggest the following:
> - tag the HEAD of the 7.0.x branch as "7.0.x-archive"
> - delete the 7.0.x branch
>
> That way it won't appear in the list of branches but it is trivial to
> recreate if we need it.
>

Good idea !


>
> I'd like to get the master->main rename completed and the 10.1.x
> development branch created towards the end of this week (unless there
> are objections or things we need to discuss further).
>
> Comments?
>

Rémy

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


Time to create Tomcat 10.1.x and master->main migration

2021-05-18 Thread Mark Thomas

All,

Things are starting to move forward for Jakarta EE 10 so I think it is 
time for us to create the 10.1.x branch. At the same time, I'd like to 
switch our primary development branches from master to main for all our 
repos.


We would, therefore, end up with the following for the Tomcat repo:

main   - 10.1.x development
10.0.x - 10.0.x development/maintenance
9.0.x  - 9.0.x development/maintenance
8.5.x  - 8.5.x development/maintenance
7.0.x  - 7.0.x development/maintenance

There are some git commands each committer will need to run locally for 
each repo to switch from master to main.


I have also been looking into how we can "retire" the 7.0.x branch when 
the time comes (after end of June). I'd like to suggest the following:

- tag the HEAD of the 7.0.x branch as "7.0.x-archive"
- delete the 7.0.x branch

That way it won't appear in the list of branches but it is trivial to 
recreate if we need it.


I'd like to get the master->main rename completed and the 10.1.x 
development branch created towards the end of this week (unless there 
are objections or things we need to discuss further).


Comments?

Mark

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



[tomcat] branch master updated: Remove experimental code

2021-05-18 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt 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 d731971  Remove experimental code
d731971 is described below

commit d73197196a578881fd99de0e4bf91e106c194668
Author: Mark Thomas 
AuthorDate: Tue May 18 11:31:37 2021 +0100

Remove experimental code
---
 java/org/apache/jasper/compiler/Generator.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/org/apache/jasper/compiler/Generator.java 
b/java/org/apache/jasper/compiler/Generator.java
index 92ccaf6..65098c6 100644
--- a/java/org/apache/jasper/compiler/Generator.java
+++ b/java/org/apache/jasper/compiler/Generator.java
@@ -1845,7 +1845,7 @@ class Generator {
 
 // Initialize local variables used in this method.
 if (!isTagFile) {
-out.printil("final jakarta.servlet.jsp.PageContext 
pageContext = _jspx_page_context;");
+out.printil("jakarta.servlet.jsp.PageContext pageContext = 
_jspx_page_context;");
 }
 // Only need to define out if the tag has a non-empty body or
 // uses ... nodes

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



[Bug 65124] Inefficient generated JSP code

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65124

--- Comment #12 from Mark Thomas  ---
Regarding issue 3:

The call to _jspx_page_context.getOut() is now only made when required.

The variable aliasing is more difficult. For more background see this thread
https://markmail.org/thread/7hksrkahpvjg3ukz but the short version is: it is
intentional; removing it isn't an option; and making it configurable is
relatively risky/invasive for the associated benefit. Therefore, we do not
currently plan to make any changes in this area.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: JSP code generation optimizations

2021-05-18 Thread Mark Thomas

On 17/05/2021 21:42, Rémy Maucherat wrote:

On Mon, May 17, 2021 at 8:47 PM Mark Thomas  wrote:


Hi all,

I am looking at some of the optimizations requested in [1]

One of the examples is show below. Code like this is used when the JSP
calls a tag.


private boolean _jspx_meth_mytag_005fhelloWorld_005f0(
  jakarta.servlet.jsp.PageContext _jspx_page_context)
  throws java.lang.Throwable {
  jakarta.servlet.jsp.PageContext pageContext = _jspx_page_context;
  //  mytag:helloWorld
  
}

The question is, can the generated code above be changed to something
like this:

private boolean _jspx_meth_mytag_005fhelloWorld_005f0(
  final jakarta.servlet.jsp.PageContext pageContext)
  throws java.lang.Throwable {
  //  mytag:helloWorld
  
}


I am assuming that the aliasing of _jspx_page_context to pageContext is
so that _jspx_page_context always references the original even if the
tag decides to do something like:

pageContext = new MyCustomPageContextImpl();

The question is, is behaviour like this allowed?

I can't see anything in the spec that disallows this.

I don't think the aliasing is a defence against malicious apps as it is
only the implicit objects that are protected this way. Internal objects
(those name _jsp*) are unprotected.

Given that the original authors (who were on the JSP EG at the time)
added this aliasing for the implicit objects, it looks like the
intention was to allow applications to manipulate/replace the implicit
objects if they wish - without breaking Jasper.

If the above assumptions are correct, what do we want to do about [1]?
The aliasing is often unnecessary but will sometimes be essential. Do we
reject this optimization request? Do we add a configuration option? Do
we try and make this aspect of the code generation pluggable somehow?
Something else?

Thoughts very welcome.



Well, although I do see how this makes the generated code a little bit
smaller, this is a rather small optimization (even with 1000s of
occurrences). Since you've identified a risk, and it seems correct to me
(I'm almost certain this was on purpose and because the idea is not
forbidden in the specification), I would say we should skip that one.


That works for me.

_jspx_page_context is referenced in multiple places in Generator. I 
can't see a way to make that configurable that isn't relatively invasive 
/ risky for the benefit it brings.


If the EG were to declare that implicit variables should be declared as 
final then that would change things - but I don't see that happening.


Mark



[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=65124


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



[tomcat] branch master updated: Partial fix for BZ 65124. Only generate local out variable when required

2021-05-18 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt 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 4aab46d  Partial fix for BZ 65124. Only generate local out variable 
when required
4aab46d is described below

commit 4aab46d425e26dd6448665c74f0956e01c9fa8fb
Author: Mark Thomas 
AuthorDate: Tue May 18 11:22:04 2021 +0100

Partial fix for BZ 65124. Only generate local out variable when required

https://bz.apache.org/bugzilla/show_bug.cgi?id=65124
---
 java/org/apache/jasper/compiler/Generator.java | 8 ++--
 webapps/docs/changelog.xml | 5 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/jasper/compiler/Generator.java 
b/java/org/apache/jasper/compiler/Generator.java
index 4c228e8..92ccaf6 100644
--- a/java/org/apache/jasper/compiler/Generator.java
+++ b/java/org/apache/jasper/compiler/Generator.java
@@ -1845,9 +1845,13 @@ class Generator {
 
 // Initialize local variables used in this method.
 if (!isTagFile) {
-out.printil("jakarta.servlet.jsp.PageContext pageContext = 
_jspx_page_context;");
+out.printil("final jakarta.servlet.jsp.PageContext 
pageContext = _jspx_page_context;");
+}
+// Only need to define out if the tag has a non-empty body or
+// uses ... nodes
+if (!n.hasEmptyBody() || n.getNamedAttributeNodes().size() > 
0) {
+out.printil("jakarta.servlet.jsp.JspWriter out = 
_jspx_page_context.getOut();");
 }
-out.printil("jakarta.servlet.jsp.JspWriter out = 
_jspx_page_context.getOut();");
 generateLocalVariables(out, n);
 }
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 20a366a..173e8cd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -131,6 +131,11 @@
 Refactor use of internal ChildInfo class to use compile
 time type checking rather than run time type checking. (markt)
   
+  
+65124: Partial fix. When generating Java source code to call
+a tag handler, only define the local variable JspWriter 
out
+when it is going to be used. (markt)
+  
 
   
 

-
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: Fix BZ 65311. Correct race condition that could lead to short delay

2021-05-18 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt 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 b42b6fe  Fix BZ 65311. Correct race condition that could lead to short 
delay
b42b6fe is described below

commit b42b6fedefea8f1a8f26bc006e8aeee6497c7411
Author: Mark Thomas 
AuthorDate: Tue May 18 10:56:49 2021 +0100

Fix BZ 65311. Correct race condition that could lead to short delay

https://bz.apache.org/bugzilla/show_bug.cgi?id=65311
---
 java/org/apache/tomcat/util/net/NioBlockingSelector.java | 6 ++
 webapps/docs/changelog.xml   | 5 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/NioBlockingSelector.java 
b/java/org/apache/tomcat/util/net/NioBlockingSelector.java
index 5aaa90e..4b61440 100644
--- a/java/org/apache/tomcat/util/net/NioBlockingSelector.java
+++ b/java/org/apache/tomcat/util/net/NioBlockingSelector.java
@@ -320,11 +320,9 @@ public class NioBlockingSelector {
 events();
 int keyCount = 0;
 try {
-int i = wakeupCounter.get();
-if (i>0)
+if (wakeupCounter.getAndSet(-1) > 0) {
 keyCount = selector.selectNow();
-else {
-wakeupCounter.set(-1);
+} else {
 keyCount = selector.select(1000);
 }
 wakeupCounter.set(0);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c9b3a5c..1922cfe 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,11 @@
 an error occurs on an HTTP/1.1 connection being upgraded to HTTP/2 or 
on
 a pushed HTTP/2 stream. (markt)
   
+  
+65311: Fix a race condition in the
+NioBlockingSelector that could cause a delay to select
+operations. (markt)
+  
 
   
   

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



[Bug 65311] NioBlockingSelector.BlockPoller.run() may not be wakeup in time

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65311

Mark Thomas  changed:

   What|Removed |Added

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

--- Comment #1 from Mark Thomas  ---
Thanks for the report.

Fixed in:
- 9.0.x for 9.0.47 onwards
- 8.5.x for 8.5.67 onwards

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[tomcat] branch 9.0.x updated: Fix BZ 65311. Correct race condition that could lead to short delay

2021-05-18 Thread markt
This is an automated email from the ASF dual-hosted git repository.

markt 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 dc442e5  Fix BZ 65311. Correct race condition that could lead to short 
delay
dc442e5 is described below

commit dc442e5bab823803ffb92adaa0dc5ed7e83f796f
Author: Mark Thomas 
AuthorDate: Tue May 18 10:56:49 2021 +0100

Fix BZ 65311. Correct race condition that could lead to short delay

https://bz.apache.org/bugzilla/show_bug.cgi?id=65311
---
 java/org/apache/tomcat/util/net/NioBlockingSelector.java | 4 +---
 webapps/docs/changelog.xml   | 5 +
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/NioBlockingSelector.java 
b/java/org/apache/tomcat/util/net/NioBlockingSelector.java
index a60b7aa..79f0797 100644
--- a/java/org/apache/tomcat/util/net/NioBlockingSelector.java
+++ b/java/org/apache/tomcat/util/net/NioBlockingSelector.java
@@ -323,11 +323,9 @@ public class NioBlockingSelector {
 events();
 int keyCount = 0;
 try {
-int i = wakeupCounter.get();
-if (i > 0) {
+if (wakeupCounter.getAndSet(-1) > 0) {
 keyCount = selector.selectNow();
 } else {
-wakeupCounter.set(-1);
 keyCount = selector.select(1000);
 }
 wakeupCounter.set(0);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7ded262..2246907 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,11 @@
 an error occurs on an HTTP/1.1 connection being upgraded to HTTP/2 or 
on
 a pushed HTTP/2 stream. (markt)
   
+  
+65311: Fix a race condition in the
+NioBlockingSelector that could cause a delay to select
+operations. (markt)
+  
 
   
   

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



[GitHub] [tomcat-jakartaee-migration] rmaucher commented on issue #20: Skipping files

2021-05-18 Thread GitBox


rmaucher commented on issue #20:
URL: 
https://github.com/apache/tomcat-jakartaee-migration/issues/20#issuecomment-843021850


   It would still be interesting to get a zip/war that triggers this issue. 
Most likely it has a duplicate entry that gets ignored when expanding (I agree: 
who cares ...) but since we rewrite it and the JVM checks for that condition, 
it fails. It is annoying to ignore or avoid this problem, looking at the 
ZipOutputStream code. It would need catching the exception and checking the 
message, which is obviously not robust at all.
   Expanding first as a workaround seems like a more practical solution.


-- 
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] ShamithaSIlva commented on issue #20: Skipping files

2021-05-18 Thread GitBox


ShamithaSIlva commented on issue #20:
URL: 
https://github.com/apache/tomcat-jakartaee-migration/issues/20#issuecomment-843006708


   > > Failed to migrate archive [xyz.war]. Using the "-zipInMemory" option may 
help.
   > > Exception in thread "main" java.util.zip.ZipException: duplicate entry: 
WEB-INF/classes/xyz_api_activityStreamXYZ_getActivityStreamData_gson$_run_closure1.class
   > > at java.util.zip.ZipOutputStream.putNextEntry(Unknown Source)
   > > at 
org.apache.tomcat.jakartaee.Migration.migrateArchiveStreaming(Migration.java:228)
   > 
   > Ok, so it really means we're trying to add a duplicate entry in the zip 
(looked up in the JVM source). Do you also have problems with your war if you 
unpack it yourself, then migrate it with the tool using the defaults ? 
Generally trying to migrate compressed items is slower and riskier.
   > If you can end up providing a test war, it could help, the most likely is 
that its compression is likely a bit weird. How is it created ?
   
   Already extracted war worked. Thanks guys!


-- 
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] ShamithaSIlva closed issue #20: Skipping files

2021-05-18 Thread GitBox


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


   


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



[Bug 65311] New: NioBlockingSelector.BlockPoller.run() may not be wakeup in time

2021-05-18 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=65311

Bug ID: 65311
   Summary: NioBlockingSelector.BlockPoller.run() may not be
wakeup in time
   Product: Tomcat 8
   Version: 8.5.31
  Hardware: PC
OS: Linux
Status: NEW
  Severity: normal
  Priority: P2
 Component: Connectors
  Assignee: dev@tomcat.apache.org
  Reporter: some51...@163.com
  Target Milestone: 

org.apache.tomcat.util.net.NioBlockingSelector.BlockPoller: 

227  public void wakeup() {
228 if (wakeupCounter.addAndGet(1)==0) selector.wakeup();
229  }

287  public void run() {
288 while (run) {
289 try {
290 events();
291 int keyCount = 0;
292 try {
293 int i = wakeupCounter.get();
294 if (i>0)
295 keyCount = selector.selectNow();
296 else {
297 wakeupCounter.set(-1);
298 keyCount = selector.select(1000);
299 }
300 wakeupCounter.set(0);

when wakeupCounter is 0, selector.select(1000) may not be wakeup in time.
to resolve this bug, we can see org.apache.tomcat.util.net.NioEndpoint.Poller:

782  public void run() {
783 // Loop until destroy() is called
784 while (true) {
785  
786 boolean hasEvents = false;
787  
788 try {
789 if (!close) {
790 hasEvents = events();
791 if (wakeupCounter.getAndSet(-1) > 0) {
792 //if we are here, means we have other
stuff to do
793 //do a non blocking select
794 keyCount = selector.selectNow();
795 } else {
796 keyCount =
selector.select(selectorTimeout);
797 }
798 wakeupCounter.set(0);
799 }

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



[GitHub] [tomcat-jakartaee-migration] rmaucher commented on issue #20: Skipping files

2021-05-18 Thread GitBox


rmaucher commented on issue #20:
URL: 
https://github.com/apache/tomcat-jakartaee-migration/issues/20#issuecomment-842957167


   > Failed to migrate archive [xyz.war]. Using the "-zipInMemory" option may 
help.
   > Exception in thread "main" java.util.zip.ZipException: duplicate entry: 
WEB-INF/classes/xyz_api_activityStreamXYZ_getActivityStreamData_gson$_run_closure1.class
   > at java.util.zip.ZipOutputStream.putNextEntry(Unknown Source)
   > at 
org.apache.tomcat.jakartaee.Migration.migrateArchiveStreaming(Migration.java:228)
   
   Ok, so it really means we're trying to add a duplicate entry in the zip 
(looked up in the JVM source). Do you also have problems with your war if you 
unpack it yourself, then migrate it with the tool using the defaults ? 
Generally trying to migrate compressed items is slower and riskier.
   If you can end up providing a test war, it could help, the most likely is 
that its compression is likely a bit weird. How is it created ?


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