Re: JDK 9 EA Build 159 and JDK 8u152 is available on java.net

2017-03-19 Thread Rémy Maucherat
2017-03-03 12:23 GMT+01:00 Rory O'Donnell :

>
> Hi Mark,
>
> *JDK 9 Early Access* b159   is available
> on java.net, summary of  changes are listed here <
> http://download.java.net/java/jdk9/changes/jdk-9+159.html>.
>
> Can you confirm fix in b159 for  JDK-8175261 : Per-protocol cache setting
> not working for JAR URLConnection
>
> There have been a number of fixes to bugs reported by Open Source projects
> since the last availability email  :
>
>  * b158 - JDK-8173028 : Incorrect processing of supplementary-plane
>characters in text fields
>  * b158 - JDK-8172967 : [macosx] Exception while working with layout
>for text containing unmappable character
>  * b158 - JDK-8173804 : javadoc throws UnsupportedOperationException:
>should not happen
>  * b157 - JDK-8174073 : NPE caused by @link reference to class
>  * b156 - JDK-8172726 : ForkJoin common pool retains a reference to the
>thread context class loader
>
> The following changeset is included in jdk-9+158:
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/8b0d55e02f54
>
> If you have a user-defined Policy implementation that grants
> FilePermission on ${user.dir}/-, reading a file in the current directory
> using its base name will fail.  Still the same solution: Ensure that the
> path used in permission granting has the same style as the one how you
> access the file.
>
> Setting -Djdk.security.filePermCompat=true will take you back to the
> jdk-9+140 behavior.
> Setting -Djdk.io.permissionsUseCanonicalPath=true will take you back to
> the jdk8 behavior.
> Feedback is welcome on jdk9-...@openjdk.java.net
>
> *JDK 8u152 **Early Access b01  *is
> available on java.net
>
> Other areas of interest
>
>  * JDK 9 Developer Guide [1]
>  * JDK 9 Migration Guide [2]
>  * JDK Cryptographic Roadmap [3]
>
> Finaly, Dalibor and I gave a presentation at FOSDEM the video is available
> here [*4*]
>

This BZ https://bz.apache.org/bugzilla/show_bug.cgi?id=60560 caught my
attention about support for System.inheritedChannel. NIO is supported, but
for some reason NIO2 support was never added. Due to its limitations (only
one channel) the API isn't particularly helpful so many it's because it is
considered almost deprecated. Any comments ?

Thanks,
Rémy

>
> Rgds,Rory
>
> [1] http://docs.oracle.com/javase/9/javase-docs.htm
> [2] https://docs.oracle.com/javase/9/migrate/toc.htm#JSMIG-GUID-
> 7744EF96-5899-4FB2-B34E-86D49B2E89B6
> [3] https://www.java.com/en/jre-jdk-cryptoroadmap.html
> [4] https://fosdem.org/2017/schedule/event/outreach/
>
> --
> Rgds,Rory O'Donnell
> Quality Engineering Manager
> Oracle EMEA , Dublin, Ireland
>
>


Re: Read events suspend/resume logic in websocket impl to achieve backpressure

2017-03-21 Thread Rémy Maucherat
2017-03-20 15:41 GMT+01:00 Violeta Georgieva :

> Hi,
>
> 2017-02-27 16:50 GMT+02:00 Mark Thomas :
> >
> > On 27/02/17 11:55, Violeta Georgieva wrote:
> >
> > 
> >
> > >> A new patch is available based on the provided comments.
> > >> Can you please review it.
> > >
> > > Any feedback for the latest changes
> >
> > Sorry for the delay.
> >
> > On a minor/style point, I'd prefer SUSPENDED rather than READ_SUSPENDED
> > since the socket won't be eligible for read or write.
>
> Ok
>
> > Thinking some more about that, could that cause problems? Does the patch
> > need to ensure write operations aren't attempted? Non-blocking writes
> > should be OK but a blocking write would be problematic.
>
> I was thinking something
> around org.apache.tomcat.websocket.WsRemoteEndpointBasic.send* methods.
> If the reading is suspended then these methods will do nothing and log
> error.
> What do you think?
>
> > I think there is still a timing / concurrency issue around resume().
> > Consider the following sequence:
> > - incoming message is being processed in WsFrameServer
> > - suspend() is called on another thread
> > - while loop ends and onDataAvailable returns
> > - resume() is called on another thread
> > - then WsHttpUpgradeHandler checks if the thread is suspended
> >
> > The problem is between onDataAvailable returning and
> > WsHttpUpgradeHandler checking if the thread is suspended. If resume() is
> > called and processed during that admittedly narrow gap, the socket will
> > end up in the Poller twice which - from past experience - will cause
> > problems.
>
> I fixed that. The PR https://github.com/apache/tomcat/pull/42 is updated.
>
> A long time ago there were two extensions proposed for the "comet" API
(still in the wiki somehow: https://wiki.apache.org/tomcat/WhatIsComet ).
As part of them was suspend/resume which do exactly what you propose:
suspend read events for a while. While the Servlet API additions also looks
very similar, it decided to be less flexible and in particular there's no
suspend/resume (conceptually with the Servlet API, you'd have to unset the
listeners which isn't allowed).

So, having looked at the history, it's hard for me to really complain about
this being added.

Rémy


Re: Question about r1780601

2017-03-21 Thread Rémy Maucherat
2017-03-21 17:45 GMT+01:00 Violeta Georgieva :

> Hi,
>
> I was checking some back ports related to bug 60897.
> This [1] was not back ported to the older versions.
> Do we really want it only for Tomcat 9?
>

It can now be backported since nobody complained about it, although it will
increase code size, causing more reports like the BZ. I broke things once
with the fix, hence no new backport.

Rémy

>
> Regards,
> Violeta
>
> [1] https://svn.apache.org/viewvc?view=revision&revision=1780601
>


Re: [VOTE] Release Apache Tomcat 9.0.0.M19

2017-03-28 Thread Rémy Maucherat
2017-03-27 14:56 GMT+02:00 Mark Thomas :

> The proposed Apache Tomcat 9.0.0.M19 release is now available for voting.
>
> This is a milestone release for the 9.0.x branch. It should be
> noted that, as a milestone release:
> - Servlet 4.0 is not finalised
> - The EGs have not started work on JSP 2.4, EL 3.1 or WebSocket 1.2/2.0
>
> The major changes compared to the 9.0.0.M18 release are:
>
> - Various HTTP/2 improvements
>
> - Fixes for sendfile related issues that could cause subsequent requests
>   to experience IllegalStateExceptions
>
> - Servlet 4.0 updates
>
>
> Along with lots of other bug fixes and improvements
>
> For full details, see the changelog:
> http://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/changelog.xml
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.0.M19/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1125/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tags/TOMCAT_9_0_0_M19/
>
> The proposed 9.0.0.M19 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M19
>
> Rémy


Re: [VOTE] Release Apache Tomcat 8.5.13

2017-03-29 Thread Rémy Maucherat
2017-03-27 17:13 GMT+02:00 Mark Thomas :

> The proposed Apache Tomcat 8.5.13 release is now available for voting.
>
> The major changes compared to the 8.5.12 release are:
>
> - Various HTTP/2 improvements
>
> - Fixes for sendfile related issues that could cause subsequent requests
>   to experience IllegalStateExceptions
>
> - Servlet 4.0 updates
>
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.5.13/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1126/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.5.x/tags/TOMCAT_8_5_13/
>
> The proposed 8.5.13 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.5.13
>
> Rémy


URI encoding

2017-03-30 Thread Rémy Maucherat
Hi,

On the Servlet EG mailing list, I noticed a request for comments on URI
encoding, which adds a per webapp parameter for encoding and, most
importantly, that it also affects the URI. The problem is that decoding is
still needed to first map the URI to the webapp, making this added feature
a problem. OTOH, if decoding twice is needed, only people who use this
parameter would pay the price.

Comments ?

Rémy


Re: [VOTE] Release Apache Tomcat 8.0.43

2017-03-30 Thread Rémy Maucherat
2017-03-28 17:28 GMT+02:00 Violeta Georgieva :

> The proposed Apache Tomcat 8.0.43 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.0.43/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1127/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.0.x/tags/TOMCAT_8_0_43/
>
> The proposed 8.0.43 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.0.43
>
> Rémy


Re: [VOTE] Release Apache Tomcat 7.0.77

2017-03-30 Thread Rémy Maucherat
2017-03-28 18:45 GMT+02:00 Violeta Georgieva :

> The proposed Apache Tomcat 7.0.77 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-7/v7.0.77/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1128/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc7.0.x/tags/TOMCAT_7_0_77/
>
> The proposed 7.0.77 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 7.0.77 Stable
>
> Rémy


Re: Hang in TestSsl testRenegotiateWorks for NIO2 (was Re: [VOTE] Release Apache Tomcat 8.0.43)

2017-04-07 Thread Rémy Maucherat
2017-04-06 21:57 GMT+02:00 Rainer Jung :

> I am not much close, but r1781988 broke it:
>
> "Ensure that executor thread pools used with connectors pre-start the
> configured minimum number of idle threads."
>
>  The change itself is OK, but as a consequence the test is now executing
> with more threads in the executor:
>
> Index: java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
> ===
> --- java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java (revision
> 1781987)
> +++ java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java (revision
> 1781988)
> @@ -63,19 +63,23 @@
>
>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long
> keepAliveTime, TimeUnit unit, BlockingQueue workQueue,
> RejectedExecutionHandler handler) {
>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
> workQueue, handler);
> +prestartAllCoreThreads();
>  }
>
>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long
> keepAliveTime, TimeUnit unit, BlockingQueue workQueue,
> ThreadFactory threadFactory,
>  RejectedExecutionHandler handler) {
>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
> workQueue, threadFactory, handler);
> +prestartAllCoreThreads();
>  }
>
>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long
> keepAliveTime, TimeUnit unit, BlockingQueue workQueue,
> ThreadFactory threadFactory) {
>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
> workQueue, threadFactory, new RejectHandler());
> +prestartAllCoreThreads();
>  }
>
>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long
> keepAliveTime, TimeUnit unit, BlockingQueue workQueue) {
>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
> workQueue, new RejectHandler());
> +prestartAllCoreThreads();
>  }
>
>  public long getThreadRenewalDelay() {
>
>
> So at least that's why I observed it first in 8.0.42, the version that
> change was included.
>
> But I'm not close to the root cause.
>
> Reproducability on my slow Solaris 10 Sparc system is very good, about 80%
> of all attempts show the hang.
>
> This test is skipped with NIO. With NIO2, it works for me, so it's not,
but when I had a look then, I became convinced it's impossible to guarantee
the process will work with async IO and the SSL engine. Basically, it's
really a blocking IO thing, preferably all integrated like with java.io.
However, the feature should be kept since usually the user will not be in
the worst case scenario, unlike the test.

So if this bothers you, the easiest at this point is to skip the test for
NIO2 in Tomcat 8.0.

Rémy


Re: Hang in TestSsl testRenegotiateWorks for NIO2 (was Re: [VOTE] Release Apache Tomcat 8.0.43)

2017-04-07 Thread Rémy Maucherat
2017-04-07 14:49 GMT+02:00 Rainer Jung :

> Am 07.04.2017 um 11:22 schrieb Rémy Maucherat:
>
>> 2017-04-06 21:57 GMT+02:00 Rainer Jung :
>>
>> I am not much close, but r1781988 broke it:
>>>
>>> "Ensure that executor thread pools used with connectors pre-start the
>>> configured minimum number of idle threads."
>>>
>>>  The change itself is OK, but as a consequence the test is now executing
>>> with more threads in the executor:
>>>
>>> Index: java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
>>> ===
>>> --- java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
>>> (revision
>>> 1781987)
>>> +++ java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
>>> (revision
>>> 1781988)
>>> @@ -63,19 +63,23 @@
>>>
>>>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize,
>>> long
>>> keepAliveTime, TimeUnit unit, BlockingQueue workQueue,
>>> RejectedExecutionHandler handler) {
>>>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
>>> workQueue, handler);
>>> +prestartAllCoreThreads();
>>>  }
>>>
>>>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize,
>>> long
>>> keepAliveTime, TimeUnit unit, BlockingQueue workQueue,
>>> ThreadFactory threadFactory,
>>>  RejectedExecutionHandler handler) {
>>>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
>>> workQueue, threadFactory, handler);
>>> +prestartAllCoreThreads();
>>>  }
>>>
>>>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize,
>>> long
>>> keepAliveTime, TimeUnit unit, BlockingQueue workQueue,
>>> ThreadFactory threadFactory) {
>>>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
>>> workQueue, threadFactory, new RejectHandler());
>>> +prestartAllCoreThreads();
>>>  }
>>>
>>>  public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize,
>>> long
>>> keepAliveTime, TimeUnit unit, BlockingQueue workQueue) {
>>>  super(corePoolSize, maximumPoolSize, keepAliveTime, unit,
>>> workQueue, new RejectHandler());
>>> +prestartAllCoreThreads();
>>>  }
>>>
>>>  public long getThreadRenewalDelay() {
>>>
>>>
>>> So at least that's why I observed it first in 8.0.42, the version that
>>> change was included.
>>>
>>> But I'm not close to the root cause.
>>>
>>> Reproducability on my slow Solaris 10 Sparc system is very good, about
>>> 80%
>>> of all attempts show the hang.
>>>
>>> This test is skipped with NIO. With NIO2, it works for me, so it's not,
>>>
>> but when I had a look then, I became convinced it's impossible to
>> guarantee
>> the process will work with async IO and the SSL engine. Basically, it's
>> really a blocking IO thing, preferably all integrated like with java.io.
>> However, the feature should be kept since usually the user will not be in
>> the worst case scenario, unlike the test.
>>
>> So if this bothers you, the easiest at this point is to skip the test for
>> NIO2 in Tomcat 8.0.
>>
>
> Hi Rémy
>
> what about:
>
> Index: build.xml
> ===
> --- build.xml   (revision 1790439)
> +++ build.xml   (working copy)
> @@ -1480,6 +1480,8 @@
>   unless="${test.openssl.exists}" />
>  
>   if="${test.excludePerformance}" />
> +
> +
>
>  
>
>
>
> so one can exclude tests by listing them in property "test.exclude". For
> me on Solaris TestSsl hangs the test suite, so I would prefer to
> skip/exclude it, all others currently do not have a problem with it. So we
> can keep the test enabled for now and I will disable it in my environment
> using the new test.exclude property. Once more people or Gump etc. report
> failures we can officially skip it.
>
> WDYT?
>
> +1, this option won't hurt obviously.

Right now, the code is:
 protected static boolean isRenegotiationSupported(Tomcat tomcat) {
String protocol =
tomcat.getConnector().getProtocolHandlerClassName();
if (protocol.contains("Apr")) {
// Disabled by default in 1.1.20 windows binary (2010-07-27)
return false;
}
if (protocol.contains("NioProtocol") ||
(protocol.contains("Nio2Protocol") && isMacOs())) {
// Doesn't work on all platforms - see BZ 56448.
return false;
}

return true;
}

So if we know it doesn't work on Solaris, it can be filtered out there.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M20

2017-04-13 Thread Rémy Maucherat
2017-04-12 22:11 GMT+02:00 Mark Thomas :

> The proposed Apache Tomcat 9.0.0.M20 release is now available for voting.
>
> This is a milestone release for the 9.0.x branch. It should be
> noted that, as a milestone release:
> - Servlet 4.0 is not finalised
> - The EGs have not started work on JSP 2.4, EL 3.1 or WebSocket 1.2/2.0
>
> The major changes compared to the 9.0.0.M19 release are:
>
> - Correct a regression that broke JMX operations (including the Manager
>   web application) if the operation took parameters
>
> - Add JMX support for Tribes components
>
> - Calls to isReady() no longer throw exceptions after timeouts for async
>   servlets
>
>
> Along with lots of other bug fixes and improvements
>
> For full details, see the changelog:
> http://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/changelog.xml
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.0.M20/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1131/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tags/TOMCAT_9_0_0_M20/
>
> The proposed 9.0.0.M20 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M20
>
> Rémy


Re: [VOTE] Release Apache Tomcat 8.5.14

2017-04-13 Thread Rémy Maucherat
2017-04-13 15:12 GMT+02:00 Mark Thomas :

> The proposed Apache Tomcat 8.5.14 release is now available for voting.
>
> The major changes compared to the 8.5.13 release are:
>
> - Correct a regression that broke JMX operations (including the Manager
>   web application) if the operation took parameters
>
> - Add JMX support for Tribes components
>
> - Calls to isReady() no longer throw exceptions after timeouts for async
>   servlets
>
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.5.14/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1132/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.5.x/tags/TOMCAT_8_5_14/
>
> The proposed 8.5.14 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.5.14
>
> Rémy


Re: TC 8.5 and Log4J2 via Juli: Wrong location info

2017-04-17 Thread Rémy Maucherat
2017-04-15 23:32 GMT+02:00 Rainer Jung :

> Coming back to this.
>
> Reminder:
>
> - in TC 8.5 we removed explicit support for Log4J(2)
>
> - we direct users at the Log4J2 JUL bridge
>
> - the Log4J2 JUL bridge works by adding the additional Log4J2 jar files to
> the CLASSPATH and setting LOGGING_MANAGER to -Djava.util.logging.manager=or
> g.apache.logging.log4j.jul.LogManager
>
> Now there's a problem: the log4j2 implementation of retrieving location
> info (e.g. %C, %l, %F, %L) from the stack isn't aware of our juli sitting
> between the real jul classes and log4J2 and so always results in
> org.apache.juli.logging.DirectJDKLog as location of the log event instead
> of the real class containing the log statement.
>
> Of course location info patterns are not recommended for performance
> reasons, nevertheless I think they should work correctly for debugging
> purposes.
>
> One suggested solution was:
>
> - add a org.apache.juli.logging.Log impl that delegates to Log4J2 (instead
> of the JUL bridge)
>
> and
>
> - add a Log4jLogEventFactory that additionally skips our DirectJDKLog when
> looking for the location info in the stack.
>
> I implemented this. See the patch for 8.5 at
>
> http://home.apache.org/~rjung/patches/tc8.5.x-juli-log4j2.patch
>
> The way I did it, is adding back a tomcat-juli-adapters.jar, which
> contains the above two classes. Adding this instead of the Log4J2 JUL
> bridge to the CLASSPATH (plus the Log4J2 api and core jars, but no changes
> to LOGGING_MANAGER) result in Log4J2 being used and logging with correct
> location info.
>
> Would that make a useful addition for TC 8.5 (and 9)?
>
> Ok, so the dependency from extras to log4j is removed just to be added
back 5 minutes later as a real dependency. Interesting !
Personally, I'm probably going to vote "no".

Rémy


Re: [ANN] Snapshots now published to Nexus after every CI build for 8.5.x and 9.0.x

2017-04-20 Thread Rémy Maucherat
2017-04-20 16:20 GMT+02:00 Mark Thomas :

> Hi,
>
> With some help from the infra team, I have just finished configuring the
> CI system to publish snapshot builds to Nexus after every CI run for
> 9.0.x and 8.5.x.
>
> The snapshots include:
> - all the individual JARs
> - all the embedded JARs
> - the .tar.gz and .zip distributions
>
> In other words, we have a resource we can point users that want to test
> fixes to where they can grab a build of the latest source without having
> to build it themselves (I still think we should encourage them to build
> it themselves in the first instance).
>
> It may turn out that publishing a snapshot after every release causes
> some sort of resource issue for infra. If that is the case, we can drop
> it down to once a day.
>

That sounds excellent !

Rémy

>
> Enjoy!
>
> Mark
>
> Join us at TomcatCon in Miami for 3 days of Apache Tomcat content:
> https://tomcat.apache.org/conference.html
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: [VOTE] Release Apache Tomcat 8.0.44

2017-05-11 Thread Rémy Maucherat
2017-05-10 20:41 GMT+02:00 Violeta Georgieva :

> The proposed Apache Tomcat 8.0.44 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.0.44/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1136/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.0.x/tags/TOMCAT_8_0_44/
>
> The proposed 8.0.44 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.0.44
>
> Rémy


Re: [VOTE] Release Apache Tomcat 7.0.78

2017-05-11 Thread Rémy Maucherat
2017-05-10 17:47 GMT+02:00 Violeta Georgieva :

> The proposed Apache Tomcat 7.0.78 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-7/v7.0.78/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1135/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc7.0.x/tags/TOMCAT_7_0_78/
>
> The proposed 7.0.78 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 7.0.78 Stable
>
> Rémy


Re: try to release taglibs-standard-1.2.6

2017-05-16 Thread Rémy Maucherat
2017-04-30 11:10 GMT+02:00 jean-frederic clere :

> Hi,
>
> I will try to tag and propose taglibs-standard-1.2.6 for release
> tomorrow, any comments?
>

Since this didn't happen, I plan to add a pre_126_bz60950 tag instead.
Would that be acceptable ?

Rémy

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


Re: try to release taglibs-standard-1.2.6

2017-05-16 Thread Rémy Maucherat
2017-05-16 14:43 GMT+02:00 Konstantin Kolinko :

> 2017-05-16 14:56 GMT+03:00 Rémy Maucherat :
> > 2017-04-30 11:10 GMT+02:00 jean-frederic clere :
> >
> >> Hi,
> >>
> >> I will try to tag and propose taglibs-standard-1.2.6 for release
> >> tomorrow, any comments?
> >>
> >
> > Since this didn't happen, I plan to add a pre_126_bz60950 tag instead.
> > Would that be acceptable ?
>
> I'd prefer more explicit "1.2.6" in the name.
>
> Maybe just name it a "release candidate", e.g.:
> https://svn.apache.org/repos/asf/tomcat/taglibs/standard/tag
> s/taglibs-standard-1.2.6-RC1/
>
> Ok, I will probably do that. I could attempt to make the real 1.2.6
release, but the RELEASING.txt looks incomplete (pom.xml would presumably
need to be updated, CHANGES.txt as well), and I would have trouble making
the binaries available and signing them. I would of course feel a lot more
optimistic if I was at ApacheCon with a lot of "support" people around :)

Rémy


Re: Proposal to remove AjpApr connector

2017-05-27 Thread Rémy Maucherat
2017-05-26 6:46 GMT-05:00 Mark Thomas :

> On 25/05/17 18:04, Christopher Schultz wrote:
> > All,
> >
> > At ApacheCon, a few of us were talking about things that could be
> > removed in upcoming versions of Tomcat. The issue of connectors came up,
> > and I was thinking that there doesn't seem to be a reason to have an
> > AjpApr connector any more.
> >
> > The APR flavor of the AJP connector was only useful when BIO was the
> > only IO strategy available, but now NIO has been available for some
> > time. APR really only gives a benefit when used with OpenSSL for TLS,
> > and since AJP doesn't use crypto, I think it's no longer necessary.
> >
> > I think we could even remove it as of Tomcat 9 if we want.
> >
> > What do others think?
>
> It is only 8 lines of code. It is probably simpler just to keep it than
> to take the time to document why that particular combination is no
> longer available.
>
> However, if you expand the proposal to removing the HTTP APR/native
> connector as well then that gets a lot more interesting.
>
> Now we have NIO/NIO2 + OpenSSL there is much less of a requirement for
> APR/native.
>
> On the plus side, it is probably (mariginally?) faster in some scenarios.
>

I can confirm it is faster, although it marginal ;)

>
> On the down side, it is less stable (we still get the odd crash report)
> and it is ~1200 lines of code.
>
> Dropping the APR/native connectors also opens up the possibility of a
> significantly trimmed down native library - or possibly even going
> directly to OpenSSL.
>

Yes, there was an experiment we did on that, the code reduction is
significant, although it doesn't bring any tangible benefit to the user (=
it's not faster or any easier to use).

>
> It feels a bit late to do this for 9.0.x although we code if we wanted
> to. It is more of an option for 10.0.x.
>
> My current thinking is that we should drop APR/native for 10.0.x. What
> do others think?
>
>
+1 for 10.

Rémy


Re: svn commit: r1767304 - in /tomcat/trunk/java/org/apache: coyote/AbstractProtocol.java tomcat/util/net/AbstractEndpoint.java tomcat/util/net/AprEndpoint.java tomcat/util/net/Nio2Endpoint.java tomca

2016-10-31 Thread Rémy Maucherat
2016-10-31 16:22 GMT+01:00 :

> Author: markt
> Date: Mon Oct 31 15:22:07 2016
> New Revision: 1767304
>
> URL: http://svn.apache.org/viewvc?rev=1767304&view=rev
> Log:
> Follow up to r1767250: rename backlog to acceptCount in ProcotolHandler
> and Endpoint
>
> Have you been able to notice any actual use of this setting ? I remember
it never made any noticeable difference for me.

Rémy


Re: [VOTE] Release Apache Tomcat 8.5.7

2016-11-03 Thread Rémy Maucherat
2016-11-03 15:49 GMT+01:00 Mark Thomas :

> On 03/11/2016 14:43, Felix Schumacher wrote:
> > Hi all,
> >
> > change r1767357 broke ajp connector with an executor.
> >
> > Tomcat fails to set the thread priority, since the ajp connector reports
> a default priority of -1 and Thread#setPriority will throw an IAE in line
> 583 of AbstractProtocol#start.
> >
> > To reproduce this enable the commented executor in server.xml and add
> that executor to the ajp connector.
> >
> > A second rather minor point is a white space policy violation in
> webapps/docs/config/http2.xml:160.
>
> That almost certainly means 9.0.x is broken as well.
>
> I can re-roll once this is fixed.
>
> I'll be able to get to this in a few hours. If someone beats me to the
> fix, that would be great :)
>
> Bad luck ... AbstractProtocol.start should use Thread.NORM_PRIORITY if
getThreadPriority() is -1. I won't commit it since you'll have a lot of
tagging to do, so it won't make any difference :(

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M13

2016-11-04 Thread Rémy Maucherat
2016-11-03 22:40 GMT+01:00 Mark Thomas :

> The proposed 9.0.0.M13 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M13
>
> Looks fine to me with my testing.

Rémy


Re: [VOTE] Release Apache Tomcat 8.5.8

2016-11-04 Thread Rémy Maucherat
2016-11-03 22:50 GMT+01:00 Mark Thomas :

> The proposed 8.5.8 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.5.8
>
> Testing is ok.

Rémy


Re: svn commit: r1768283 - in /tomcat/trunk/java/org/apache/catalina: Realm.java realm/DataSourceRealm.java

2016-11-06 Thread Rémy Maucherat
2016-11-05 23:03 GMT+01:00 :

> Author: markt
> Date: Sat Nov  5 22:03:30 2016
> New Revision: 1768283
>
> URL: http://svn.apache.org/viewvc?rev=1768283&view=rev
> Log:
> Keep checkstyle happy
>

Thanks, unfortunately I had disabled checkstyle at some point to do other
things.

Rémy

>
> Modified:
> tomcat/trunk/java/org/apache/catalina/Realm.java
> tomcat/trunk/java/org/apache/catalina/realm/DataSourceRealm.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/Realm.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
> catalina/Realm.java?rev=1768283&r1=1768282&r2=1768283&view=diff
> 
> ==
> --- tomcat/trunk/java/org/apache/catalina/Realm.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/Realm.java Sat Nov  5 22:03:30
> 2016
> @@ -237,7 +237,7 @@ public interface Realm {
>   * Return the availability of the realm for authentication.
>   * @return true if the realm is able to perform
> authentication
>   */
> -default public boolean isAvailable() {
> +public default boolean isAvailable() {
>  return true;
>  }
>  }
>
> Modified: tomcat/trunk/java/org/apache/catalina/realm/DataSourceRealm.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
> catalina/realm/DataSourceRealm.java?rev=1768283&r1=1768282&r2=1768283&
> view=diff
> 
> ==
> --- tomcat/trunk/java/org/apache/catalina/realm/DataSourceRealm.java
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/realm/DataSourceRealm.java Sat
> Nov  5 22:03:30 2016
> @@ -393,7 +393,7 @@ public class DataSourceRealm extends Rea
>  connectionSuccess = true;
>  return connection;
>  } catch (Exception e) {
> -connectionSuccess = false;
> +connectionSuccess = false;
>  // Log the problem for posterity
>  containerLog.error(sm.getString("dataSourceRealm.exception"),
> e);
>  }
>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: [VOTE] Release Apache Tomcat 8.0.39

2016-11-10 Thread Rémy Maucherat
2016-11-09 10:41 GMT+01:00 Violeta Georgieva :

> The proposed Apache Tomcat 8.0.39 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.0.39/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1107/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.0.x/tags/TOMCAT_8_0_39/
>
> The proposed 8.0.39 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.0.39
>
> No problem with the tests.

Rémy


Re: [VOTE] Release Apache Tomcat 7.0.73

2016-11-10 Thread Rémy Maucherat
2016-11-07 23:15 GMT+01:00 Violeta Georgieva :

> The proposed Apache Tomcat 7.0.73 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-7/v7.0.73/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1106/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc7.0.x/tags/TOMCAT_7_0_73/
>
> The proposed 7.0.73 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 7.0.73 Stable
>
> Tests are ok for me.

Rémy


Re: [VOTE] Release Apache Tomcat 6.0.48

2016-11-14 Thread Rémy Maucherat
2016-11-07 21:57 GMT+01:00 Violeta Georgieva :

> The proposed Apache Tomcat 6.0.48 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-6/v6.0.48/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1105/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc6.0.x/tags/TOMCAT_6_0_48/
>
> The proposed 6.0.48 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 6.0.48 Stable
>
> Rémy


Re: Tomcat Regression still haunts projects with URL Encoding?

2016-11-19 Thread Rémy Maucherat
2016-11-18 23:51 GMT+01:00 Marc Mercer :

> Anyone willing to help get this looked into, updated and fixed/validated
> once and for all (across the board)?
>
> Very funny. There's no good way to do it (although personally, I would
have said dispatcher paths are not encoded), so the best advice if you
don't want problems is: don't use URLs that need encoding. If you still
want to do it, there are two possibilities: encode/decode or don't. So
there's now a setting for that as you can see in the commit you linked.

Rémy


Re: Plans for 9.0.0.M14 and 8.5.9

2016-12-01 Thread Rémy Maucherat
2016-12-01 15:20 GMT+01:00 Mark Thomas :

> Hi,
>
> It is the start of a new month so I'm planning to tag 9.0.0.M14 and
> 8.5.9 soon(ish). As usual, I want to work through the open bug reports
> and any other known issues first.
>

IMO 8.5.8 is not really usable due to 60372 so +1 for a release.

>
> I suspect the actual tags will be next week at the earliest. Rémy
> mentioned Async timeouts + HTTP/2 as a possible root cause of a bug and,
> while the OP wasn't using HTTP/2, I took a quick look and there appears
> to be some work required in that area. On the basis anything async
> related is rarely quick and simple, my guess is that it will take into
> next week to fix this along with the open bugs.
>
> The stream had a big setSocketWrapper(null) so that's why I asked, but
actually it's the same at the end of a regular HTTP/1.1 request when
recycling the processor. So there's a race between that and the removal in
the waitingProcessor set which occurs during processing. So it could be
enough to skip over if the socketWrapper is null in AstractProcessor.
doTimeoutAsync.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M15

2016-12-06 Thread Rémy Maucherat
2016-12-05 15:47 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 9.0.0.M15 release is now available for voting.
>
> This is a milestone release for the 9.0.x branch. It should be
> noted that, as a milestone release:
> - Servlet 4.0 is not finalised
> - The EGs have not started work on JSP 2.4, EL 3.1 or WebSocket 1.2/2.0
>
> The major changes compared to the 9.0.0.M13 release are:
>
> - Improvements to SPNEGO authentication. Patches provided by Michael
>   Osipov.
>
> - Correct regression in I/O buffer handling.
>
> - Improve handling of varargs in UEL expressions. Based on a patch by
>   Ben.
>
> Along with lots of other bug fixes and improvements
>
> For full details, see the changelog:
> http://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/changelog.xml
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.0.M15/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1108/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tags/TOMCAT_9_0_0_M15/
>
> The proposed 9.0.0.M15 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M15
>
> Rémy


Re: [VOTE] Release Apache Tomcat 8.5.9

2016-12-06 Thread Rémy Maucherat
2016-12-05 21:44 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 8.5.9 release is now available for voting.
>
> The major changes compared to the 8.5.8 release are:
>
>
> - Improvements to SPNEGO authentication. Patches provided by Michael
>   Osipov.
>
> - Correct regression in I/O buffer handling.
>
> - Improve handling of varargs in UEL expressions. Based on a patch by
>   Ben.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.5.9/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1109/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.5.x/tags/TOMCAT_8_5_9/
>
> The proposed 8.5.9 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.5.8
>
> Tests work for me.

Rémy


Re: websocket and custom instantiator

2016-12-13 Thread Rémy Maucherat
2016-12-12 22:03 GMT+01:00 Romain Manni-Bucau :

> Hi guys,
>
> on 9M15 it seems the default server configurator just uses a newInstance
> and not the instance manager. Any reason? Is it too hard to catch close and
> destroy instances properly?
>
> Issue is it makes hard to integrate with an IoC without needing some
> boilerplate in onopen() and onclose().
>

It gets registered a little bit later in WsSession.

Ok for encoder/decoder, this is not done, but is it certain these objects
are injectable ?

Rémy


>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github  rmannibucau> |
> LinkedIn  | JavaEE Factory
> 
>


Re: Incoming refactoring

2016-12-14 Thread Rémy Maucherat
2016-12-14 9:50 GMT+01:00 Mark Thomas :

> The failure happens on current trunk too so I don't think this is
> related to the refactoring. However, I am going to investigate this
> failure first - before I apply the refactoring.
>
> What is the intermittent failure ?
https://ci.apache.org/builders/tomcat-trunk seems happy (usually it's not).

Rémy


Re: Incoming refactoring

2016-12-14 Thread Rémy Maucherat
2016-12-14 10:07 GMT+01:00 Mark Thomas :

> On 14/12/2016 08:54, Rémy Maucherat wrote:
> > 2016-12-14 9:50 GMT+01:00 Mark Thomas :
> >
> >> The failure happens on current trunk too so I don't think this is
> >> related to the refactoring. However, I am going to investigate this
> >> failure first - before I apply the refactoring.
> >>
> >> What is the intermittent failure ?
> > https://ci.apache.org/builders/tomcat-trunk seems happy (usually it's
> not).
>
> TestHttp11Processor.test57621b
>
> Fails maybe 1 time in 10. It looks like something is going wrong
> resetting the input buffer between requests.
>
> Ah this test is very specific. The runnable is doing what a dispatch would
do but it doesn't have the code to wait until the container thread has
returned:

public synchronized boolean asyncDispatch() {
if (!ContainerThreadMarker.isContainerThread() && state ==
AsyncState.STARTING) {
state = AsyncState.DISPATCH_PENDING;
return false;
} else {
return doDispatch();
}
}

vs:

public synchronized void asyncRun(Runnable runnable) {
... [set env]
processor.getExecutor().execute(runnable);
 ... [unset env]
}

So if this is allowed, it would run into the same concurrency issues that
were fixed for dispatch.

Rémy


Re: Tomcat can report that a request-processing thread was started by an application?

2016-12-22 Thread Rémy Maucherat
2016-12-22 15:31 GMT+01:00 Christopher Schultz :

> All,
>
> I was browsing the Tomcat-related questions on SO and I noticed this one:
>
> http://stackoverflow.com/questions/41223141/threads-in-tomcat-memory-leaks
>
> The poster shows a snip of his error log which says this:
>
> дек 19, 2016 12:25:10 AM
> org.apache.catalina.loader.WebappClassLoaderBase clearReferencesThreads
> SEVERE: The web application [/someContext] appears to have started a
> thread named [http-apr-8081-exec-10] but has failed to stop it. This is
> very likely to create a memory leak.
>
> That seems like an incorrect -- and very confusing -- error message.
>
> The use-case was undeploying an application while a request was being
> handled (your basic Thread.sleep() to hold a request in-flight in order
> to undeploy during that time). This was with Tomcat 7 (no specific
> version number).
>
> It also seems like in that case, Tomcat may forcibly-change the TCCL of
> the in-flight request-processing thread, bus it's not quite clear to me
> precisely what's going on.
>
> It's mostly as expected. The servlet unloading waits for an unload delay
(2s) then moves on. The thread is then flagged as a possible leak, which is
true. The odd thing is that there's code to identify a request thread, and
it's not reported as one (the code looks like it will work, so I don't
understand). Maybe this check was not present in the version that is being
used.

Rémy


Re: Final 6.0.x release?

2017-01-02 Thread Rémy Maucherat
2017-01-01 12:11 GMT+01:00 Mark Thomas :

> Happy New Year!
>

+1

>
> I'm starting to think about the tasks that need to be completed now
> 6.0.x has reached EOL.
>
> Looking at the changelog for 6.0.x we probably do need a final 6.0.x
> release to pick up the 60497 fix. Any takers?
>
> Based on how we handled the 5.5.x EOL and assuming we want to handle
> 6.0.x the same way, there isn't much to do now. Assuming we do have a
> final 6.0.x release, we can remind users of the EOL (and the planned
> actions below) as part of the release announcement.
>

+1 to the EOL plan.

>
> I'll go through the open 6.0.x bugs and close / move them as appropriate
> shortly.
>
> The tasks for ~ 3 months time are:
> - remove the 6.0.x download pages
> - remove the latest 6.0.x release from the mirror system
> - move the 6.0.x branch in svn from /tomcat/tc6.0.x to
> /tomcat/archive/tc6.0.x
> - remove the links to the 6.0.x documentation from
> tomcat.apache.org (the docs remain)
> - The bugzilla project for 6.0.x will be made read-only
>
> +1

Rémy


Re: buildbot failure in on tomcat-trunk

2017-01-03 Thread Rémy Maucherat
2017-01-03 17:57 GMT+01:00 Mark Thomas :

> On 03/01/2017 16:23, build...@apache.org wrote:
> > The Buildbot has detected a new failure on builder tomcat-trunk while
> building . Full details are available at:
> > https://ci.apache.org/builders/tomcat-trunk/builds/2004
> >
> > Buildbot URL: https://ci.apache.org/
> >
> > Buildslave for this Build: silvanus_ubuntu
> >
> > Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-commit'
> triggered this build
> > Build Source Stamp: [branch tomcat/trunk] 1777147
> > Blamelist: markt
> >
> > BUILD FAILED: failed compile_1
>
> Odd. The tests timed out. It could be random but my refactoring earlier
> today has to be a suspect root cause.
>
> I'll run the complete test suite locally and see what results I get.
>
> It was only a simple NPE, fixed now.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M16

2017-01-06 Thread Rémy Maucherat
2017-01-06 0:37 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 9.0.0.M16 release is now available for voting.
>
> This is a milestone release for the 9.0.x branch. It should be
> noted that, as a milestone release:
> - Servlet 4.0 is not finalised
> - The EGs have not started work on JSP 2.4, EL 3.1 or WebSocket 1.2/2.0
>
> The major changes compared to the 9.0.0.M15 release are:
>
> - HTTP/2 fixes and improvements
>
> - Simpler JSP file encoding detector that delegates XML prolog
>   encoding detection to the JRE rather than using a custom XML
>   parser.
>
> - Improve the logic that selects an address to use to unlock the
>   Acceptor to take account of platforms what do not listen on all
>   local addresses when configured with an address of 0.0.0.0 or ::
>
> Along with lots of other bug fixes and improvements
>
> For full details, see the changelog:
> http://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/changelog.xml
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.0.M16/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1110/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tags/TOMCAT_9_0_0_M16/
>
> The proposed 9.0.0.M16 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M16
>
> Rémy


Re: [VOTE] Release Apache Tomcat 8.5.10

2017-01-06 Thread Rémy Maucherat
2017-01-06 0:38 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 8.5.10 release is now available for voting.
>
> The major changes compared to the 8.5.9 release are:
>
> - HTTP/2 fixes and improvements
>
> - Simpler JSP file encoding detector that delegates XML prolog
>   encoding detection to the JRE rather than using a custom XML
>   parser.
>
> - Improve the logic that selects an address to use to unlock the
>   Acceptor to take account of platforms what do not listen on all
>   local addresses when configured with an address of 0.0.0.0 or ::
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.5.10/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.5.x/tags/TOMCAT_8_5_10/
>
> The proposed 8.5.10 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.5.10
>
> Rémy


Re: [VOTE] Release Apache Tomcat 8.5.10

2017-01-06 Thread Rémy Maucherat
2017-01-06 13:58 GMT+01:00 Violeta Georgieva :

> I'm observing the exception below:
>

Maybe but I fail to see the actual problem in the generated code, and it's
not normal anyway that it makes a difference when the change is to replace
 with  .
However, I should have used tagHandlerVar rather than
n.getTagHandlerPoolName() for the boolean variable, for sure.

Rémy

>
> [junit] An error occurred at line: 20 in the jsp file:
> /bug48nnn/bug48616.jsp
> [junit] Duplicate local variable
> _005fjspx_005ftagPool_005fbugs_005fBug48616b_005fnobody_reused
> [junit] 17: <%@ taglib prefix="bugs" uri="http://tomcat.apache.org/
> bugs"
> %>
> [junit] 18: 
> [junit] 19: 
> [junit] 20:   
> [junit] 21: 
> [junit]
> [junit]
> [junit] Stacktrace:] with root cause
> [junit]  org.apache.jasper.JasperException: Unable to compile class
> for
> JSP:
> [junit]
> [junit] An error occurred at line: 20 in the jsp file:
> /bug48nnn/bug48616.jsp
> [junit] Duplicate local variable
> _005fjspx_005ftagPool_005fbugs_005fBug48616b_005fnobody_reused
> [junit] 17: <%@ taglib prefix="bugs" uri="http://tomcat.apache.org/
> bugs"
> %>
> [junit] 18: 
> [junit] 19: 
> [junit] 20:   
> [junit] 21: 
> [junit]
> [junit]
> [junit] Stacktrace:
> [junit] at
> org.apache.jasper.compiler.DefaultErrorHandler.javacError(
> DefaultErrorHandler.java:103)
> [junit] at
> org.apache.jasper.compiler.ErrorDispatcher.javacError(
> ErrorDispatcher.java:212)
> [junit] at
> org.apache.jasper.compiler.JDTCompiler.generateClass(JDTCompiler.java:457)
> [junit] at
> org.apache.jasper.compiler.Compiler.compile(Compiler.java:377)
> [junit] at
> org.apache.jasper.compiler.Compiler.compile(Compiler.java:349)
>
>
> I succeeded to modify one of our tests in order to reproduce the issue.
>
>
> test.entry=org.apache.jasper.compiler.TestScriptingVariabler
>
>
> Index: bug48616.jsp
> ===
> --- bug48616.jsp (revision 1777593)
> +++ bug48616.jsp (working copy)
> @@ -17,5 +17,5 @@
>  <%@ taglib prefix="bugs" uri="http://tomcat.apache.org/bugs"; %>
>  
>  
> -  
> +  
>  
> \ No newline at end of file
>
>
>
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
>


Re: [VOTE] Release Apache Tomcat 8.5.10

2017-01-06 Thread Rémy Maucherat
2017-01-06 14:44 GMT+01:00 Violeta Georgieva :

> 2017-01-06 15:24 GMT+02:00 Rémy Maucherat :
> >
> > 2017-01-06 13:58 GMT+01:00 Violeta Georgieva :
> >
> > > I'm observing the exception below:
> > >
> >
> > Maybe but I fail to see the actual problem in the generated code, and
> it's
> > not normal anyway that it makes a difference when the change is to
> replace
> >  with  .
> > However, I should have used tagHandlerVar rather than
> > n.getTagHandlerPoolName() for the boolean variable, for sure.
>
> Thanks with that change I do not see the exception anymore.
>
> Yes, it's much more logical with this more unique variable name (it's the
tag instance that gets reused, not the tag pool) and the compiler is no
longer complaining, but I still don't understand the error.

Rémy


Re: [VOTE] Release Apache Tomcat 8.5.10

2017-01-06 Thread Rémy Maucherat
2017-01-06 16:52 GMT+01:00 Violeta Georgieva :

> With r1775598 changes, the declaration of the scripting variable now is in
> try/finally block and it is not visible outside this block.
>
> http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/a
> pache/jasper/compiler/Generator.java?view=markup&pathrev=1775598#l2713
>
> This is independent of the previous issue, so you can either remove the
try/finally for simple tags, or fix it if you have a test on hand,

Rémy


Re: svn commit: r1777647 - in /tomcat/trunk: java/org/apache/jasper/compiler/Generator.java webapps/docs/changelog.xml

2017-01-06 Thread Rémy Maucherat
2017-01-06 17:55 GMT+01:00 Violeta Georgieva :

> 2017-01-06 18:50 GMT+02:00 :
> >
> > Author: remm
> > Date: Fri Jan  6 16:50:50 2017
> > New Revision: 1777647
> >
> > URL: http://svn.apache.org/viewvc?rev=1777647&view=rev
> > Log:
> > Revert try/finally for simple tags.
>
> I just succeeded to extract a test case.
> Can you look at the patch below and give me your feedback?
>

I thought it was better to revert it for now. It's possible this can work,
but not sure.

Rémy

>
>
>
> Index: java/org/apache/jasper/compiler/Generator.java
> ===
> --- java/org/apache/jasper/compiler/Generator.java (revision 1777614)
> +++ java/org/apache/jasper/compiler/Generator.java (working copy)
> @@ -2655,6 +2655,8 @@
>  // Declare AT_BEGIN scripting variables
>  declareScriptingVars(n, VariableInfo.AT_BEGIN);
>  saveScriptingVars(n, VariableInfo.AT_BEGIN);
> +// Declare AT_END scripting variables
> +declareScriptingVars(n, VariableInfo.AT_END);
>
>  String tagHandlerClassName =
> tagHandlerClass.getCanonicalName();
>  writeNewInstance(tagHandlerVar, tagHandlerClassName);
> @@ -2709,8 +2711,7 @@
>  // Synchronize AT_BEGIN scripting variables
>  syncScriptingVars(n, VariableInfo.AT_BEGIN);
>
> -// Declare and synchronize AT_END scripting variables
> -declareScriptingVars(n, VariableInfo.AT_END);
> +// Synchronize AT_END scripting variables
>  syncScriptingVars(n, VariableInfo.AT_END);
>
>  out.popIndent();
> Index: test/webapp/bug48nnn/bug48616b.jsp
> ===
> --- test/webapp/bug48nnn/bug48616b.jsp (revision 1777614)
> +++ test/webapp/bug48nnn/bug48616b.jsp (working copy)
> @@ -26,3 +26,6 @@
>  
>
>  
> +<%
> +  out.println(X);
> +%>
> \ No newline at end of file
> Index: test/webapp/WEB-INF/tags/bug42390.tag
> ===
> --- test/webapp/WEB-INF/tags/bug42390.tag (revision 1777614)
> +++ test/webapp/WEB-INF/tags/bug42390.tag (working copy)
> @@ -14,5 +14,5 @@
>See the License for the specific language governing permissions and
>limitations under the License.
>  --%>
> -<%@ variable name-given="X" scope="AT_BEGIN" %>
> +<%@ variable name-given="X" scope="AT_END" %>
>  
> \ No newline at end of file
>


Re: [VOTE] Release Apache Tomcat 9.0.0.M17

2017-01-16 Thread Rémy Maucherat
2017-01-10 22:28 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 9.0.0.M17 release is now available for voting.
>
> This is a milestone release for the 9.0.x branch. It should be
> noted that, as a milestone release:
> - Servlet 4.0 is not finalised
> - The EGs have not started work on JSP 2.4, EL 3.1 or WebSocket 1.2/2.0
>
> The major changes compared to the 9.0.0.M15 release are:
>
> - HTTP/2 fixes and improvements
>
> - Improve the logic that selects an address to use to unlock the
>   Acceptor to take account of platforms what do not listen on all
>   local addresses when configured with an address of 0.0.0.0 or ::
>
> - Fix a bug in APR/native socket close handling that resulted in
>   unexpected errors for upgraded connections
>
> Along with lots of other bug fixes and improvements
>
> For full details, see the changelog:
> http://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/changelog.xml
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.0.M17/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1112/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tags/TOMCAT_9_0_0_M17/
>
> The proposed 9.0.0.M17 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M17
>
> +1, sorry for the Jasper regressions.

Rémy


Re: [VOTE] Release Apache Tomcat 8.5.11

2017-01-16 Thread Rémy Maucherat
2017-01-10 22:28 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 8.5.11 release is now available for voting.
>
> The major changes compared to the 8.5.9 release are:
>
> - HTTP/2 fixes and improvements
>
> - Improve the logic that selects an address to use to unlock the
>   Acceptor to take account of platforms what do not listen on all
>   local addresses when configured with an address of 0.0.0.0 or ::
>
> - Fix a bug in APR/native socket close handling that resulted in
>   unexpected errors for upgraded connections
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.5.11/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1113/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.5.x/tags/TOMCAT_8_5_11/
>
> The proposed 8.5.11 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.5.11
>
> Rémy


Re: svn commit: r1779370 - /tomcat/trunk/java/org/apache/tomcat/util/buf/ByteBufferUtils.java

2017-01-20 Thread Rémy Maucherat
2017-01-18 19:46 GMT+01:00 :

> Author: markt
> Date: Wed Jan 18 18:46:27 2017
> New Revision: 1779370
>
> URL: http://svn.apache.org/viewvc?rev=1779370&view=rev
> Log:
> Java 9 can throw a Java 9 specific exception here
> (InaccessibleObjectException) so tweak the handling
>

This isn't supposed to happen following r1762753. If this no longer works,
it is necessary to find a new hack that fixes it or scream at Oracle, since
otherwise direct buffers cannot be used at all (they will leak memory
really fast).

Rémy


>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/buf/ByteBufferUtils.java
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/buf/
> ByteBufferUtils.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
> tomcat/util/buf/ByteBufferUtils.java?rev=1779370&r1=1779369&r2=1779370&
> view=diff
> 
> ==
> --- tomcat/trunk/java/org/apache/tomcat/util/buf/ByteBufferUtils.java
> (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/buf/ByteBufferUtils.java Wed
> Jan 18 18:46:27 2017
> @@ -22,6 +22,7 @@ import java.nio.ByteBuffer;
>
>  import org.apache.juli.logging.Log;
>  import org.apache.juli.logging.LogFactory;
> +import org.apache.tomcat.util.ExceptionUtils;
>  import org.apache.tomcat.util.res.StringManager;
>
>  public class ByteBufferUtils {
> @@ -46,9 +47,11 @@ public class ByteBufferUtils {
>  cleanMethodLocal = cleanerObject.getClass().
> getMethod("clean");
>  }
>  cleanMethodLocal.invoke(cleanerObject);
> -} catch (IllegalAccessException | IllegalArgumentException
> -| InvocationTargetException | NoSuchMethodException |
> SecurityException e) {
> -log.warn(sm.getString("byteBufferUtils.cleaner"), e);
> +} catch (Throwable t) {
> +// Use throwable as when running on Java 9 we may see a Java 9
> +// specific exception (InaccessibleObjectException) here.
> +ExceptionUtils.handleThrowable(t);
> +log.warn(sm.getString("byteBufferUtils.cleaner"), t);
>  cleanerMethodLocal = null;
>  cleanMethodLocal = null;
>  }
>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: Java 9 and reflection uses in Apache Tomcat

2017-01-20 Thread Rémy Maucherat
2017-01-20 13:14 GMT+01:00 Mark Thomas :

> Rory,
>
> We have a handful of instances where Tomcat currently needs to use
> reflection which are blocked when running on Java 9.
>
> Users can work-around these but it would probably be better if an
> alternative API was identified / provided to achieve the same results
> without reflection.
>
> There are currently three instances I am aware of.
>
> 1. Cleaning direct ByteBuffers
> To avoid OOME, we need to be able to trigger cleaning of a direct
> ByteBuffers. The code in question is at [1]. We need a way to trigger
> DirectByteBuffer.cleaner().clean() via a public API.
>

Technically this is: https://bugs.openjdk.java.net/browse/JDK-8171377
Essentially, we must now use
sun.misc.Unsafe.getUnsafe.invokeCleaner(byteBuffer). The problem is that we
cannot use reflection (Unsafe.getUnsafe is explicitly filtered out) and
there's also a security check which may or may not necessitate some extra
configuration.

>
> 2. Cleaning thread local related memory leaks
> To fix application created memory leaks, we need to be able to clear
> thread locals associated with a given class loader from a thread.
> To be able to warn users they have a memory leak in their application,
> we need to be able to list the thread locals associated with a class
> loader for a thread. The relevant code is at [2].
>
> 3. Cleaning RMI related memory leaks
> To fix application created memory leaks, we need to be able to clear RMI
> objects associated with a given class loader.
> To be able to warn users they have a memory leak in their application,
> we need to be able to list the RMI targets associated with a given
> classloader. The relevant code is at [3].
>
> Any suggestions for alternative public APIs to achieve the same ends
> gratefully received.
>

+1

Rémy

>
> Kind regards,
>
> Mark
>
>
> [1]
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/
> ByteBufferUtils.java?diff_format=h&view=annotate#l38
>
> [2]
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/
> WebappClassLoaderBase.java?diff_format=h&view=annotate#l1823
>
> [3]
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/
> WebappClassLoaderBase.java?diff_format=h&view=annotate#l2058
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: [VOTE] Release Apache Tomcat 8.0.41

2017-01-23 Thread Rémy Maucherat
2017-01-19 0:07 GMT+01:00 Violeta Georgieva :

> The proposed Apache Tomcat 8.0.41 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-8/v8.0.41/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1118/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc8.0.x/tags/TOMCAT_8_0_41/
>
> The proposed 8.0.41 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.0.41
>
> Rémy


Re: [VOTE] Release Apache Tomcat 7.0.75

2017-01-23 Thread Rémy Maucherat
2017-01-18 22:45 GMT+01:00 Violeta Georgieva :

> The proposed Apache Tomcat 7.0.75 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-7/v7.0.75/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1117/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc7.0.x/tags/TOMCAT_7_0_75/
>
> The proposed 7.0.75 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 7.0.75 Stable
>
> Rémy


Re: [VOTE] Release Apache Tomcat 6.0.50

2017-01-25 Thread Rémy Maucherat
2017-01-22 16:59 GMT+01:00 Violeta Georgieva :

> The proposed Apache Tomcat 6.0.50 release is now available for voting.
>
> Note: This is the last Tomcat 6 release.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-6/v6.0.50/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1119/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc6.0.x/tags/TOMCAT_6_0_50/
>
> The proposed 6.0.50 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 6.0.50 Stable
>
> Rémy


Re: HTTP response reason phrases

2017-01-25 Thread Rémy Maucherat
2017-01-24 20:08 GMT+01:00 Christopher Schultz :

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA256
>
> All,
>
> I'm cross-posting dev@ and users@, but please only reply to dev@ if
> you'd like to get involved in this discussion.
>
> I'd like to openly-discuss r1702765 [1]. There have been some
> complaints on both users@ and dev@ and some BZ issues filed against
> Tomcat 8.5 and 9.0 for removing the reason phrase. It happened in
> r1702765 with no referenced BZ issue and was first released with
> Tomcat 8.5.0 -- the initial release of Tomcat 8.5 -- as well as 9.0.0.M1
> .
>
> This issue doesn't really affect me, but some recent conversations
> about the "stability" (in terms of "things not changing") of Tomcat
> have me thinking about the implications of making this change in
> Tomcat 8.5.x and not in just Tomcat 9.0.x.
>
> It is well-known within this community that Tomcat 8.0.x and Tomcat
> 8.5.x are distinct versions, but since the major version number is the
> same, many have expectations that nothing serious is going to change.
> Of course, 8.5.x has *many* serious changes to it with respect to
> Tomcat 8.0.x. But this one seems to be tripping a lot of people up.
>

- 8.5 is already changing a number of things (since it's supposed to be 9,
so it's normal).
- the clients we're talking about are ancient.
- I only saw "a few" people affected, not "a lot".

It's better to avoid breaking things, but I don't see the benefit of
reverting it or bothering with an option.

Rémy

>
> Those who are filing bugs, etc. are quite adamant that the reason
> phrase is "required" for certain things. To be sure, the reason phrase
> will only be required by non-compliant clients, and so technically the
> client is at fault, here.
>
> I'm wondering what the wider community thinks about this change and
> whether or not we should consider reverting it for Tomcat 8.5.x.
>
> Again, please reply to the dev@ list, since that's where this
> discussion belongs. I just wanted to make sure I reached the widest
> audience possible to begin a discussion.
>
> Thanks,
> - -chris
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=r1702765
> -BEGIN PGP SIGNATURE-
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBCAAGBQJYh6ZCAAoJEBzwKT+lPKRYnJgQAIe57jJw2iUk86I63NGsqPnY
> IKWjZoMuqd8vlczn8/FF/drWMV7ObYsrhsYmJATR5iVI+/xdEXB7n8cMO7B7+ryV
> Sxe1Tcmh/tBNJ83a8C+zSHWvnIELYRonHm9syApa7onPKcsoEe6MTrsdL1M+An9U
> 9IvXtH3BfYKAynze5pkNS6I+ILjgWvNSclJFHmDNHWmRPyqdob4OtMWkSSU3qRBX
> FfbEk9IMrEbIit6CH75dw9xfaUDDRudnw3MBkKaV8VOLUoykvSMK1w9GdufO4ohP
> Dw/+l6CkXl8xCSRcNwXrDdJcisT9gN6Ey7+g7zrgAcg62RP3ftrQMCzT2VDQV3b4
> IlZfTi+vEdsKKzGUdH+OLbN0+hiW0bnuxJmTG2zQSGwKsIh78aFdPKShv4u22XKB
> xfcKn9c6XGUHH88j0ZVSOLh2AmORCvuDfQNA3NJCOceRwQsV1OHAda65fFlkjyiz
> Q/yMMV8VlblGJRItN1nEwheIs9ru3MokRBhaXQ78ehSkRxbkIPawP6ZSiojmv/80
> aKx3/T413GOK4e18sK3XFHP4NowkR7VR/a1R5Py7L2kpzhMJcc4bstYuE9hugfiN
> BaECAT66qahCmP0xVoiFEB2A0+sD0wRKZ6K1gPCarPdLKh6cX4poRcMK4i0jgG2a
> cao/Frb1y8JDm8maw1Q8
> =oQCi
> -END PGP SIGNATURE-
>
> -
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
>


Re: [VOTE] Release Apache Tomcat 6.0.50

2017-01-26 Thread Rémy Maucherat
2017-01-26 12:10 GMT+01:00 Mark Thomas :

> - One TCK test fails because an associated JSP no longer compiles
>   because it reaches the 64k method limit. The JSP has a very large
>   number of nested tags.
>
> I'm going to take another look at the JSP Generation to see if there is
> anything we can do to make even a small improvement.
>
> :( It could be an option to simply revert the try/finally change in all
branches except 8.5 and 9, after all nobody complained about it.

Rémy


Re: [VOTE] Release Apache Tomcat 6.0.50

2017-01-26 Thread Rémy Maucherat
2017-01-26 15:29 GMT+01:00 Mark Thomas :

> On 26 January 2017 13:53:22 GMT+00:00, Konstantin Kolinko <
> knst.koli...@gmail.com> wrote:
> >2017-01-26 16:00 GMT+03:00 Rémy Maucherat :
> >> 2017-01-26 12:10 GMT+01:00 Mark Thomas :
> >>
> >>> - One TCK test fails because an associated JSP no longer compiles
> >>>   because it reaches the 64k method limit. The JSP has a very large
> >>>   number of nested tags.
> >>>
> >>> I'm going to take another look at the JSP Generation to see if there
> >is
> >>> anything we can do to make even a small improvement.
> >>>
> >>> :( It could be an option to simply revert the try/finally change in
> >all
> >> branches except 8.5 and 9, after all nobody complained about it.
> >
> >1. I think that try/finally change fixed a real bug. It is better to
> >keep it.
> >
> >
> >2. Looking at java code generated for JSPs by 6.0.50, the additional
> >code is in helper methods ("_jspx_meth_"*), not in the main big
> >_jspService() method.
>
> If you use tags with bodies that contain scriptlets then the code appears
> in the main service method.
>
> >There is not much of that additional code. Just a try/catch and a
> >boolean variable.
> >So I also do not see what can be improved.
> >
> >A "catch (Throwable t)" block at the end of _jspService() could be
> >extracted into a helper method as non-trivial code, but it is only a
> >dozen of lines and it is present only once.
>
> We might be able to save a few bytes with some helper methods but it won't
> be much, if any.
>
> >It needs some actual numbers - how big of a JSP can be compiled by
> >Tomcat.
>
> It took around 250 tags with a scriplet on a single page to trigger the
> problem. I need to test how much difference the try/finally fix made.
>

Yes, the problem is that once you add a scriptlet, there can be a variable
declaration in it (well, of course there is, people don't use scriptlets to
do clean stuff) so there needs to be full visibility of it to build. We had
been trying to remove scriptlets for w very long time, it never worked, we
still have people immediately asking for support of the new Java language
features in JSPs, so brand new code being written right now still features
scriptlets.
Also sad, the original size limits of the JVM were rather low, and never
changed.

Sorry for the complaints ...

Rmy

>
> >3. There are some Jasper options that produce a more compact java
> >code. Especially the option to generates a single out.write() call for
> >a sequence of text instead of separate call for each line of text. A
> >bugzilla report mentions it.
> >
> > 
> >mappedfile
> > false
> >  
> >
> >http://tomcat.apache.org/tomcat-6.0-doc/jasper-howto.html
> >
> >I wonder whether the mentioned TCK test will pass with that option.
>
> I'll take a look.
>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: svn commit: r1780530 - /tomcat/trunk/java/org/apache/jasper/compiler/Generator.java

2017-01-27 Thread Rémy Maucherat
2017-01-27 10:55 GMT+01:00 :

> Author: markt
> Date: Fri Jan 27 09:55:04 2017
> New Revision: 1780530
>
> URL: http://svn.apache.org/viewvc?rev=1780530&view=rev
> Log:
> More refactoring of generated code so tags require less code
> Extract setters into a separate method when tag handling is in-lined in
> _jspService()
>

I am now not convinced this is valid, since it breaks visibility with
expressions. Unfortunately :(

Rémy


>
> Modified:
> tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
>
> Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
> jasper/compiler/Generator.java?rev=1780530&r1=1780529&r2=1780530&view=diff
> 
> ==
> --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
> +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Fri Jan
> 27 09:55:04 2017
> @@ -1796,7 +1796,8 @@ class Generator {
>  // to a method.
>  ServletWriter outSave = null;
>  Node.ChildInfo ci = n.getChildInfo();
> -if (ci.isScriptless() && !ci.hasScriptingVars()) {
> +boolean hasNoScriptingElement = ci.isScriptless() &&
> !ci.hasScriptingVars();
> +if (hasNoScriptingElement) {
>  // The tag handler and its body code can reside in a
> separate
>  // method if it is scriptless and does not have any
> scripting
>  // variable defined.
> @@ -1891,14 +1892,14 @@ class Generator {
>
>
>  if (n.implementsSimpleTag()) {
> -generateCustomDoTag(n, handlerInfo, tagHandlerVar);
> +generateCustomDoTag(n, handlerInfo, tagHandlerVar,
> hasNoScriptingElement);
>  } else {
>  /*
>   * Classic tag handler: Generate code for start element,
> body,
>   * and end element
>   */
>  generateCustomStart(n, handlerInfo, tagHandlerVar,
> tagEvalVar,
> -tagPushBodyCountVar);
> +tagPushBodyCountVar, hasNoScriptingElement);
>
>  // visit body
>  String tmpParent = parent;
> @@ -1926,7 +1927,7 @@ class Generator {
>  tagPushBodyCountVar);
>  }
>
> -if (ci.isScriptless() && !ci.hasScriptingVars()) {
> +if (hasNoScriptingElement) {
>  // Generate end of method
>  if (methodNesting > 0) {
>  out.printil("return false;");
> @@ -2366,10 +2367,9 @@ class Generator {
>  }
>  }
>
> -private void generateCustomStart(Node.CustomTag n,
> -TagHandlerInfo handlerInfo, String tagHandlerVar,
> -String tagEvalVar, String tagPushBodyCountVar)
> -throws JasperException {
> +private void generateCustomStart(Node.CustomTag n,
> TagHandlerInfo handlerInfo,
> +String tagHandlerVar, String tagEvalVar, String
> tagPushBodyCountVar,
> +boolean hasNoScriptingElement) throws JasperException {
>
>  Class tagHandlerClass =
>  handlerInfo.getTagHandlerClass();
> @@ -2407,7 +2407,7 @@ class Generator {
>  out.pushIndent();
>
>  // includes setting the context
> -generateSetters(n, tagHandlerVar, handlerInfo, false);
> +generateSetters(n, tagHandlerVar, handlerInfo, false,
> hasNoScriptingElement);
>
>  if (n.implementsTryCatchFinally()) {
>  out.printin("int[] ");
> @@ -2625,9 +2625,8 @@ class Generator {
>  restoreScriptingVars(n, VariableInfo.AT_BEGIN);
>  }
>
> -private void generateCustomDoTag(Node.CustomTag n,
> -TagHandlerInfo handlerInfo, String tagHandlerVar)
> -throws JasperException {
> +private void generateCustomDoTag(Node.CustomTag n,
> TagHandlerInfo handlerInfo,
> +String tagHandlerVar, boolean hasNoScriptingElement)
> throws JasperException {
>
>  Class tagHandlerClass =
>  handlerInfo.getTagHandlerClass();
> @@ -2643,7 +2642,7 @@ class Generator {
>  String tagHandlerClassName = tagHandlerClass.
> getCanonicalName();
>  writeNewInstance(tagHandlerVar, tagHandlerClassName);
>
> -generateSetters(n, tagHandlerVar, handlerInfo, true);
> +generateSetters(n, tagHandlerVar, handlerInfo, true,
> hasNoScriptingElement);
>
>  // Set the body
>  if (findJspBody(n) == null) {
> @@ -3149,9 +3148,55 @@ class Generator {
>  }
>
>  private void generateSetters(Node.CustomTag n, String
> tagHandlerVar,
> -TagHandlerInfo handlerInfo, boolean simpleTag)
> +TagHandlerInfo handlerInfo, boolean simp

Re: [VOTE] Release Apache Tomcat 6.0.50

2017-01-27 Thread Rémy Maucherat
2017-01-26 22:13 GMT+01:00 Mark Thomas :

> On 26/01/2017 14:38, Rémy Maucherat wrote:
> > 2017-01-26 15:29 GMT+01:00 Mark Thomas :
> >> On 26 January 2017 13:53:22 GMT+00:00, Konstantin Kolinko <
> >> knst.koli...@gmail.com> wrote:
>
> 
>
> >>> It needs some actual numbers - how big of a JSP can be compiled by
> >>> Tomcat.
> >>
> >> It took around 250 tags with a scriplet on a single page to trigger the
> >> problem. I need to test how much difference the try/finally fix made.
> >>
>
> Working with Tomcat 6 (9 won't be that much different), before the
> try/finally clean-up code was added a JSP page could handle 299
> instances of the foo tag (from the examples webapp) before the method
> got too big. After adding the try/finally clean-up, that drops to 228.
>
> I've been looking at the generated code for a while. One obvious
> optimisation had no effect (I'm guessing the compiler found it anyway).
>
> With a couple a small changes, I got the maximum tag count back up to 249.
>
> To take this further, I think the next step is to take the setters and,
> if the tag processed in _jspService(), move the setters for that tag
> into a separate method.
>
> I intend to explore this in 9.0.x with a view to back-porting if it works.
>
> If the try/finally fix is supposed to be important, it would be good to
use this opportunity to resurrect my try/finally patch for simple tags now
that the release cycle is done (otherwise, they have the same bug). With
the addition of Violeta's idea though.

Rémy


Re: svn commit: r1781620 - in /tomcat/tc6.0.x/trunk: ./ java/org/apache/jasper/compiler/ java/org/apache/jasper/runtime/ webapps/docs/

2017-02-03 Thread Rémy Maucherat
2017-02-04 0:40 GMT+01:00 Mark Thomas :

> On 03/02/17 23:21, ma...@apache.org wrote:
>
>> Author: markt
>> Date: Fri Feb  3 23:21:00 2017
>> New Revision: 1781620
>>
>> URL: http://svn.apache.org/viewvc?rev=1781620&view=rev
>> Log:
>> Refactor generated JSP code so tags require less code
>>
>
> With this commit and the following configuration options, the TCK test
> that was failing due to the _jspService() method being to big now passes.
>
> - development - false
> - mappedfile - false
> - enablePooling - false
> - trimSpaces - true
>
> Awesome you found a way to improve it !

Rémy


Re: Read events suspend/resume logic in websocket impl to achieve backpressure

2017-02-07 Thread Rémy Maucherat
2017-02-06 20:55 GMT+01:00 Violeta Georgieva :

> Hi,
>
> Currently JSR356 provides possibility to add message handlers in order to
> receive web socket
> messages but there is no way to instruct the web socket implementation to
> suspend for a while
> the incoming messages (backpressure) so that the application is able to
> process the already delivered messages.
> The other web containers (Jetty, Undertow) supports such functionality so I
> would like to introduce it in Tomcat.
> Here [1] I prepared one possible implementation.
>
> What do you think about this feature and the proposed implementation?
>

I don't understand why this is that useful (it has to be used in a smart
way that improves scalability by the application, I'm not convinced this
can happen) but more importantly it's a proprietary API.

Rémy

>
> Regards,
> Violeta
> [1] https://github.com/violetagg/tomcat/commits/ws-suspend-resume
>


Re: Read events suspend/resume logic in websocket impl to achieve backpressure

2017-02-08 Thread Rémy Maucherat
2017-02-08 0:51 GMT+01:00 Mark Thomas :

> On 06/02/17 19:55, Violeta Georgieva wrote:
>
>> Hi,
>>
>> Currently JSR356 provides possibility to add message handlers in order to
>> receive web socket
>> messages but there is no way to instruct the web socket implementation to
>> suspend for a while
>> the incoming messages (backpressure) so that the application is able to
>> process the already delivered messages.
>> The other web containers (Jetty, Undertow) supports such functionality so
>> I
>> would like to introduce it in Tomcat.
>> Here [1] I prepared one possible implementation.
>>
>> What do you think about this feature and the proposed implementation?
>>
>
> I suggest you go ahead and commit (and back-port) the formatting updates.
> They all look good and getting those out of the way will make the diff
> easier to read.
>
> I'm currently undecided on this.
>
> I understand the requirement but rather than have proprietary methods
> added to various WebSocket implementations, I would have preferred to see a
> reactive wrapper provided for Java WebSocket that would have used
> Server->Client WebSocket messages to communicate back pressure to the
> client.
>
> However, that doesn't work if the aim is to feed 'uncontrolled' WebSocket
> clients into a reactive server side framework. Blocking is going to be only
> option to apply back-pressure and better to do that just on the client
> rather than on the client and the server - which means this feature is
> required in some form.
>
> I guess that makes me reluctantly in favour of it in principle but I'd
> very much prefer to review a patch proposal minus the reformatting.
>
> This new solution says that it should be the network that takes care of
the congestion. Personally, I prefer thinking the application should
decouple its IO from the actual processing and take care of the backlog
itself. One example: with some APIs, we've noticed that when a server
disconnects or become unresponsive, they become notified with an error like
... Never. Or, even better, as you say, the server should send something to
the client to actually give it information, but it's all proprietary then
(client + server).

Overall, I don't really see the point of retrofitting this new stuff into
already old things like websockets, while HTTP/2 should have this server ->
client "control" capability (minus an actual API to use it from Servlets, I
guess).

If this is still included, I suppose you will make sure the performance
doesn't go down BTW.

Rémy


Re: [VOTE] Release Apache Tomcat Native 1.2.11

2017-02-08 Thread Rémy Maucherat
2017-02-07 21:30 GMT+01:00 Mark Thomas :

> Version 1.2.10 includes the following change:
>
> - Update minimum recommended OpenSSL version to 1.0.2k
> - Windows binaries built with OpenSSL 1.0.2k
> - Better documentation for building on Windows
>   (including with FIPS enabled OpenSSL)
>
> The proposed release artefacts can be found at [1],
> and the build was done using tag [2].
>
> The Apache Tomcat Native 1.2.11 is
>  [X] Stable, go ahead and release
>  [ ] Broken because of ...
>

Tested on Linux, where everything is always good, though.

Rémy

>
> Thanks,
>
> Mark
>
>
> [1]
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-connect
> ors/native/1.2.11/
> [2] https://svn.apache.org/repos/asf/tomcat/native/tags/TOMCAT_N
> ATIVE_1_2_11
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: InstanceListener, InstanceEvent: Unused?

2016-01-04 Thread Rémy Maucherat
2016-01-04 17:14 GMT+01:00 Mark Thomas :

> Given that the Tomcat code doesn't contain any classes that implement
> InstanceListener is looks to be unused internally. I'm wondering if
> anyone else is using it. If not, there is a chunk of code we could
> remove including a bunch of (admittedly simple/fast) calls made on every
> request.
>
> Could we deprecate this in 8.0.x and remove it in 9.0.x?
>
> Tomcat had these hooks for integration in enterprise environments (but it
wasn't done very well and was incomplete). I tried to improve that with
simpler hooks (the instance manager and the thread listener). OTOH, as was
said previously, this one is used to do RunAs most likely [it needs to set
the proper security context when the servlet is invoked], so since someone
from TomEE is there to comment, what's the plan to do this feature without
it ?

Rémy


Re: InstanceListener, InstanceEvent: Unused?

2016-01-04 Thread Rémy Maucherat
2016-01-04 17:14 GMT+01:00 Mark Thomas :

> Given that the Tomcat code doesn't contain any classes that implement
> InstanceListener is looks to be unused internally. I'm wondering if
> anyone else is using it. If not, there is a chunk of code we could
> remove including a bunch of (admittedly simple/fast) calls made on every
> request.
>
> Could we deprecate this in 8.0.x and remove it in 9.0.x?
>
> So about the proposal: +1 to try to remove InstanceListener, a more
specialized hook could be added instead if really needed.

Rémy


Re: Upcoming release plans

2016-01-04 Thread Rémy Maucherat
2016-01-04 23:46 GMT+01:00 Mark Thomas :

> The changelog for 9.0.x is getting quite long so I'd like to do a
> release. A 8.0.x release wouldn't hurt either.
>
> I recall that Remy said it would be useful to have a Tomcat-Native
> release first before the next 9.0.x release so my current thinking is:
>

Yes, a change has been made to be able to detect the end of the handshake
during a renegotiation (this method also looks better during the initial
handshake but is incompatible with 1.2.3 - it will likely not detect the
handshake is done with the older native, so handshake will never complete).

>
> - tag Tomcat-Native 1.2.4 tomorrow and start the release process
> - tidy up any remaining 9.0.x and 8.0.x issues while the 1.2.4 vote
>   progresses
> - update 9.0.x and 8.0.x to Tomcat-Native 1.2.4 once released
>

Updating 8 to the new 1.2.4 native isn't mandatory.

- tag 9.0.x and start the release process
> - tag 8.0.x and start the release process
>
> Depending on timing the last two items may be in parallel or series.
>
> Rémy


Re: Upcoming release plans

2016-01-05 Thread Rémy Maucherat
2016-01-04 23:46 GMT+01:00 Mark Thomas :

> The changelog for 9.0.x is getting quite long so I'd like to do a
> release.
>

At this time the APR connector remains the default one when native is used.
I would think it should stay that way for this build, although direct
OpenSSL works for me [at some small performance cost compared to full APR].

Rémy


Re: [VOTE] Release Apache Tomcat Native 1.2.4

2016-01-05 Thread Rémy Maucherat
2016-01-05 16:46 GMT+01:00 Mark Thomas :

> Version 1.2.4 includes the following change:
>
> - Renegotiation improvements
>
> The proposed release artefacts can be found at [1],
> and the build was done using tag [2].
>
> The Apache Tomcat Native 1.2.4 is
>  [X] Stable, go ahead and release
>  [ ] Broken because of ...
>
> Rémy


Re: [VOTE-RESTARTED] Release Apache Tomcat Native 1.2.4

2016-01-06 Thread Rémy Maucherat
2016-01-06 13:56 GMT+01:00 Mark Thomas :

> On 05/01/2016 15:46, Mark Thomas wrote:
> > Version 1.2.4 includes the following change:
> >
> > - Renegotiation improvements
> >
> > The proposed release artefacts can be found at [1],
> > and the build was done using tag [2].
> >
> > The Apache Tomcat Native 1.2.4 is
> >  [X] Stable, go ahead and release
> >  [ ] Broken because of ...
>
> Restarting the vote now I have corrected the binaries so they are linked
> to OpenSSL 1.0.2e.
>
> The tag and the source archives are unchanged.
>
> Still stable, I'm using OpenSSL from my OS.

Rémy


Re: [PROPOSAL] Manager behaviour clarification for 9.0.x

2016-01-08 Thread Rémy Maucherat
2016-01-08 13:33 GMT+01:00 Mark Thomas :

> My preference is for 1. It is less work, Tomcat currently never changes
> the Context and I can't see a viable use case for wanting to do so.
>

No problem, that's the best solution.

Rémy


Re: svn commit: r1723752 - in /tomcat/trunk/java/org/apache: catalina/filters/ catalina/ha/ catalina/ha/backend/ catalina/ha/deploy/ catalina/ha/session/ catalina/ha/tcp/ catalina/session/ catalina/tr

2016-01-08 Thread Rémy Maucherat
2016-01-08 20:00 GMT+01:00 Konstantin Kolinko :

>  /**
> - * Return the member that represents this node.
> - *
> - * @return Member
> + * @return the member that represents this node.
>   */
>  public Member getLocalMember();
>
>  /**
> - * Return response status code that is used to reject denied request.
> + * @return response status code that is used to reject denied request.
>   */
>
>
> The first sentence of description of a method goes into summary - into
> the table that lists class methods.
>
> E.g.:
>
> http://tomcat.apache.org/tomcat-8.0-doc/api/org/apache/catalina/filters/RequestFilter.html#method_summary
>
> If everything is moved into @return tag, the method summary becomes
> empty and that table would list just the method names.
>
> I think that a simple
> @return Member
> @return Member or null
> @return Boolean
> @return value
> will fix the javadoc tool warning and is better than moving all method
> description into @return tag.
>
> This was done like that elsewhere and I like it.

Rémy


Re: svn commit: r1724094 - /tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java

2016-01-11 Thread Rémy Maucherat
2016-01-11 18:35 GMT+01:00 :

> Author: markt
> Date: Mon Jan 11 17:35:29 2016
> New Revision: 1724094
>
> URL: http://svn.apache.org/viewvc?rev=1724094&view=rev
> Log:
> Whoops. Fix regression in r1724015.
> Code was used although I can't see why a simple AtomicInteger wasn't
> sufficient.
>
> The Netty code used this. I'm not convinced either, but of course if
there's any problem it will crash.

Rémy


Re: svn commit: r1724394 - /tomcat/trunk/java/org/apache/catalina/util/RequestUtil.java

2016-01-13 Thread Rémy Maucherat
2016-01-13 11:54 GMT+01:00 :

> Author: markt
> Date: Wed Jan 13 10:54:41 2016
> New Revision: 1724394
>
> URL: http://svn.apache.org/viewvc?rev=1724394&view=rev
> Log:
> Javadoc
>
>
> I'm doing the javadoc to help out with the todo list while waiting for
more interesting releases / testing results. I'm sure I'll get to these
eventually down the list, unless my javadoc is acting weird.

Rémy


Re: svn commit: r1725201 - /tomcat/trunk/java/org/apache/catalina/session/StandardManager.java

2016-01-18 Thread Rémy Maucherat
2016-01-18 10:17 GMT+01:00 :

> Author: markt
> Date: Mon Jan 18 09:17:13 2016
> New Revision: 1725201
>
> URL: http://svn.apache.org/viewvc?rev=1725201&view=rev
> Log:
> Refactor handling of failed loading of persisted sessions.
> Old behaviour:
>  - sessions loaded up to point where error occurred
>  - serialized session data deleted
>  - web app started
> i.e. session data after the failure was lost
> New behaviour
>  - serialized session data deleted only if all sessions loaded without
> error
>  - web application only starts if all sessions loaded without error
>
> Honestly I'm not a big fan of session saving, I consider it causes as much
issues as it does resolve. But as a result, I think it should be best
effort, so I prefer the old behavior.

Rémy


Re: svn commit: r1725201 - /tomcat/trunk/java/org/apache/catalina/session/StandardManager.java

2016-01-18 Thread Rémy Maucherat
2016-01-18 10:17 GMT+01:00 :

> Author: markt
> Date: Mon Jan 18 09:17:13 2016
> New Revision: 1725201
>
> URL: http://svn.apache.org/viewvc?rev=1725201&view=rev
> Log:
> Refactor handling of failed loading of persisted sessions.
> Old behaviour:
>  - sessions loaded up to point where error occurred
>  - serialized session data deleted
>  - web app started
> i.e. session data after the failure was lost
> New behaviour
>  - serialized session data deleted only if all sessions loaded without
> error
>  - web application only starts if all sessions loaded without error
>
> So I think it would be best to stat over on this. The old behavior could
perhaps be improved though, like skipping the sessions with errors but
restoring as much as possible.

IMO this is not a real production feature at all, this is really a very old
trick. You should either use a cluster or have the session act as a read
cache (while everything is immediately persisted to the backend). Maybe
this feature could be abandoned by default actually ? [but I am worried we
could find many people relying on it ;) ]

Rémy


Re: svn commit: r1725497 - in /tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool: DataSourceProxy.java FairBlockingQueue.java MultiLockFairBlockingQueue.java

2016-01-19 Thread Rémy Maucherat
2016-01-19 13:48 GMT+01:00 :

> Author: markt
> Date: Tue Jan 19 12:48:31 2016
> New Revision: 1725497
>
> URL: http://svn.apache.org/viewvc?rev=1725497&view=rev
> Log:
> Fix a couple of IDE warnings triggered by adding the missing Javadoc
> Fix the remaining Javadoc warnings
>
> Modified:
>
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSourceProxy.java
>
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
>
> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/MultiLockFairBlockingQueue.java
>
> For some reason the javadoc warnings are not completely consistent. I got
a warning about type parameters in some other area, but not here.

Rémy


Re: svn commit: r1725687 - in /tomcat/trunk/java/org/apache/jasper/compiler: ELNode.java Generator.java Node.java

2016-01-20 Thread Rémy Maucherat
2016-01-20 10:09 GMT+01:00 :

> Author: markt
> Date: Wed Jan 20 09:09:08 2016
> New Revision: 1725687
>
> URL: http://svn.apache.org/viewvc?rev=1725687&view=rev
> Log:
> Javadoc
>
> Modified:
> tomcat/trunk/java/org/apache/jasper/compiler/ELNode.java
> tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
> tomcat/trunk/java/org/apache/jasper/compiler/Node.java
>
> How do you get a report on these javadoc errors ? They don't appear with
my javadoc (from Java 8) nor on the CI runs [which have test failures after
the lifecycle refactoring].

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M2

2016-01-21 Thread Rémy Maucherat
2016-01-21 13:38 GMT+01:00 Mark Thomas :

> The proposed 9.0.0.M2 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M2
>
> The testsuite looks ok for me.

Once this is released, maybe we can start discussing the contents of that
possible new 8.x branch.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M2

2016-01-23 Thread Rémy Maucherat
2016-01-23 17:51 GMT+01:00 Martin Grigorov :

> Hi,
> 23-Jan-2016 17:36:58.900 SEVERE [main]
> org.apache.coyote.AbstractProtocol.init Failed to initialize end point
> associated with ProtocolHandler ["https-apr-8443"]
>  java.lang.NullPointerException
> at
> org.apache.tomcat.util.net.AprEndpoint.bind(AprEndpoint.java:366)
>


> Line 366 is: for (String protocol : sslHostConfig.getEnabledProtocols()) {
> sshHostConfig is used earlier, so it seems the result of
> #getEnabledProtocols() is null.
>
> My conf/server.conf looks like:
>  protocol="org.apache.coyote.http11.Http11AprProtocol"
>maxThreads="150" SSLEnabled="true" >
>  />
> 
>  certificateKeyFile="/tmp/tc9.0.0.M2/private-key.pem"
>  certificateFile="/tmp/tc9.0.0.M2/cert.pem"
>  type="RSA" />
> 
> 
>
> I just uncommented it and changed the paths to the certificate files.
>
> I run Ubuntu 15.10, Apr 1.5.2, Openssl 1.0.2e, Tomcat Native 1.2.4.
> Please let me know if you need more information!
>
> Ok, well, it doesn't work ... Maybe it would be a great time to give a
shot to the NIO(2) + OpenSSL support ! :)

Besides that, the JSSE code went into a refactoring, and the APR connector
didn't get it. The enabled protocols are now parsed in the superclass of
the SSLUtil and the set on the host config, that doesn't happen with APR. I
don't really understand why the APR connector didn't continue using
sslHostConfig.getProtocols.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M2

2016-01-24 Thread Rémy Maucherat
2016-01-23 20:02 GMT+01:00 Martin Grigorov :

> Hi Rémy,
> What changes I should apply to be able to test HTTP2 ?
> I've changed the protocol:
>  protocol="org.apache.coyote.http11.Http11Nio2Protocol"
>maxThreads="150" SSLEnabled="true" >
>  />
> 
>   certificateFile="/tmp/tc9.0.1/cert.pem"
>  type="RSA" />
> 
> 
>
> and created ~/.keystore.
> Now I can start Tomcat and test my apps successfully with HTTP 1.1.
> The "Server" response header is "Apache-Coyote/1.1". And my plugin for
> Google Chrome says that SPDY/HTTP2 is not enabled.
>
> So I'm using:
  
This tells the listener to not use the APR connector even if APR is present.

And a connector like:
 





This will use NIO + OpenSSL. You don't have to use a keystore, you can
replace it with your certificateKeyFile="/tmp/tc9.0.1/private-key.pem"
certificateFile="/tmp/tc9.0.1/cert.pem"

To use other combinations and force OpenSSL, you can use
sslImplementationName="org.apache.tomcat.util.net.openssl.OpenSSLImplementation"
on the Connector element.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M2

2016-01-24 Thread Rémy Maucherat
2016-01-24 15:27 GMT+01:00 Rainer Jung :

>
> APR+SSL seemed not to have worked in the unit tests.
>
> Yes, it doesn't work. I put in a quick fix which works for me but I am
unsure of the relationship with the refactoring (r1724234).

Rémy


Re: AJP13 and flush handling

2016-01-26 Thread Rémy Maucherat
2016-01-26 10:57 GMT+01:00 Rainer Jung :

> For applications for which timely streaming is important, that's a good
> default behavior. The flush goes through until the client. But for
> traditional applications it might be better to let the web server buffer
> and decide when to flush. E.g. mod_proxy_ajp has an implementation to wait
> a short time for more data and then auto-flush.
>

I think it could work since AJP is not compatible with upgrade.

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M2

2016-01-26 Thread Rémy Maucherat
2016-01-26 9:13 GMT+01:00 Martin Grigorov :

>
> With Rémy's help I was able to test my apps with HTTP2 without APR.
> My conf/server.conf looks like:
>
>  SSLEngine="on" *aprPreferred="false"* />
>  protocol="org.apache.coyote.http11.Http11Nio2Protocol"
>maxThreads="150" SSLEnabled="true"
>
> *sslImplementationName="org.apache.tomcat.util.net.openssl.OpenSSLImplementation"*
> >
>  />
> 
> 
>
> Thanks, Rémy!
>

No problem. Since you apparently really want to use NIO2 + SSL, let us know
if you run into:
https://bz.apache.org/bugzilla/show_bug.cgi?id=58721

Rémy


Re: svn commit: r1727355 - /tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java

2016-01-28 Thread Rémy Maucherat
2016-01-28 15:53 GMT+01:00 Mark Thomas :

> On 28/01/2016 14:16, Konstantin Kolinko wrote:
> > 2016-01-28 17:04 GMT+03:00  :
> >> Author: markt
> >> Date: Thu Jan 28 14:04:00 2016
> >> New Revision: 1727355
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1727355&view=rev
> >> Log:
> >> Switch OutputBuffer to a local Map of C2BConvertors. Loading testing
> with HTTP/2 and default Tomcat home page suggests an improvement of a few
> percent in throughput.
> >
> > How much memory is wasted?
>
> I wouldn't call that memory 'wasted'. I'd call it 'used' to gain
> somewhere in the region of a 2% to 4% throughput increase.
>
> > Every connection (incl. keep-alive ones) will have its own map of
> > encoders?  Encoders are not reused across different connections?
>
> Using the same load test as I used for the performance numbers, I see a
> retained size of less than 1MB all of which is eligible for GC. There
> isn't much reuse in HTTP/2.
>
> If I switch to HTTP/1.1 and run the same test I end up with ~60
> instances using ~16k. Scale that up to 2000 concurrent connections and
> you get ~550k so I am not worried about memory use in this case.
>
> I didn't measure the performance gain of this change for HTTP/1.1 but
> I'd expect there to be some benefit although not as much as HTTP/2
> simply because HTTP/2 uses more concurrency for broadly the same load.
>
> I have always used local encoders and decoders, nobody ever complained
about memory use so it's most likely not a problem.

Rémy


Re: Timing for 9.0.0.M3

2016-01-29 Thread Rémy Maucherat
2016-01-29 15:36 GMT+01:00 Konstantin Kolinko :

> 2016-01-28 22:58 GMT+03:00 Mark Thomas :
> > All,
> >
> > Now we believe that today's OpenSSL issues don't affect us, I plan to
> > tag 9.0.0.M3 fairly soon. I want to check a few things around the
> > Encoder caching and the other connectors first so I'll probably tag
> > 9.0.0.M3 tomorrow morning. Once that release vote is ready to go, I'll
> > start on 8.0.x.
>
>
> I think redirection fail logging issue needs to be addressed
> https://bz.apache.org/bugzilla/show_bug.cgi?id=58768
>
> More generally, I think BZs should be moved to the most recent branches
when it is certain they are still relevant.

Rémy


Re: svn commit: r1727992 - in /tomcat/trunk: java/org/apache/tomcat/util/net/SecureNioChannel.java webapps/docs/changelog.xml

2016-02-01 Thread Rémy Maucherat
2016-02-01 20:47 GMT+01:00 :

> Author: markt
> Date: Mon Feb  1 19:47:13 2016
> New Revision: 1727992
>
> URL: http://svn.apache.org/viewvc?rev=1727992&view=rev
> Log:
> Fix a consistent unit test failure on OSX (no idea why it started to
> appear now)
> Handle the case where the required TLS buffer increases after the
> connection has been initiated.
>

Well, the design is so wrong. BTW, what is the
getSession().getApplicationBufferSize() value here ? And that's with
OpenSSL or JSSE ?

Rémy

>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
> tomcat/trunk/webapps/docs/changelog.xml
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java?rev=1727992&r1=1727991&r2=1727992&view=diff
>
> ==
> --- tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
> (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java Mon
> Feb  1 19:47:13 2016
> @@ -558,18 +558,33 @@ public class SecureNioChannel extends Ni
>  if (unwrap.getStatus() == Status.BUFFER_UNDERFLOW) {
>  break;
>  }
> -} else if (unwrap.getStatus() == Status.BUFFER_OVERFLOW &&
> read > 0) {
> -//buffer overflow can happen, if we have read data, then
> -//empty out the dst buffer before we do another read
> -break;
> +} else if (unwrap.getStatus() == Status.BUFFER_OVERFLOW) {
> +if (read > 0) {
> +// Buffer overflow can happen if we have read data.
> Return
> +// so the destination buffer can be emptied before
> another
> +// read is attempted
> +break;
> +} else {
> +// The SSL session has increased the required buffer
> size
> +// since the buffer was created.
> +if (dst ==
> socket.getSocketBufferHandler().getReadBuffer()) {
> +// This is the normal case for this code
> +socket.getSocketBufferHandler().expand(
> +
> sslEngine.getSession().getApplicationBufferSize());
> +dst =
> socket.getSocketBufferHandler().getReadBuffer();
> +} else {
> +// Can't expand the buffer as there is no way to
> signal
> +// to the caller that the buffer has been
> replaced.
> +throw new IOException(
> +
> sm.getString("channel.nio.ssl.unwrapFail", unwrap.getStatus()));
> +}
> +}
>  } else {
> -//here we should trap BUFFER_OVERFLOW and call expand on
> the buffer
> -//for now, throw an exception, as we initialized the
> buffers
> -//in the constructor
> +// Something else went wrong
>  throw new
> IOException(sm.getString("channel.nio.ssl.unwrapFail", unwrap.getStatus()));
>  }
> -} while ( (netInBuffer.position() != 0)); //continue to
> unwrapping as long as the input buffer has stuff
> -return (read);
> +} while (netInBuffer.position() != 0); //continue to unwrapping
> as long as the input buffer has stuff
> +return read;
>  }
>
>  /**
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1727992&r1=1727991&r2=1727992&view=diff
>
> ==
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Mon Feb  1 19:47:13 2016
> @@ -99,6 +99,10 @@
>  New configuration option ajpFlush for the AJP
> connectors
>  to disable the sending of AJP flush packets. (rjung)
>
> +  
> +Handle the case in the NIO connector where the required TLS
> buffer sizes
> +increase after the connection has been initiated. (markt)
> +  
>  
>
>  
>
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: svn commit: r1727992 - in /tomcat/trunk: java/org/apache/tomcat/util/net/SecureNioChannel.java webapps/docs/changelog.xml

2016-02-02 Thread Rémy Maucherat
2016-02-02 1:01 GMT+01:00 Mark Thomas :

> > Well, the design is so wrong.
>
> I'm not a fan of the solution either but I couldn't see a better way to
> resize the buffer.
>
> I thought about:
> - custom exception - rejected since exceptions are slow and flow control
>   via exception is bad
> - custom return value (-2, Integer.MIN_VALUE or similar) - rejected
>   because it is non-standard and would require changes up the call-stack
>   to handle it.
> - increasing the default buffer size - rejected as the smaller buffer
>   is enough in nearly all cases
>
> Anything else I thought of required invasive API changes. A related
> issue is the read(ByteBuffer) provides no way to expose the expanded
> ByteBuffer to the caller but that method is part of the ByteChannel API.
>
> Suggestions welcome.
>

Will think about that :)
At some point I'll likely have to add another read buffer in
SecureNio2Channel since I would like more flexibility ...

>
> > BTW, what is the
> > getSession().getApplicationBufferSize() value here ? And that's with
> > OpenSSL or JSSE ?
>
> Roughly 16k or 32k for JSSE with current Oracle Java 8 as far as I can
> tell. Something, I didn't figure out what, was triggering a switch to
> the larger buffer size after we had initialised the buffers.
>
> The OpenSSL implementation only ever uses 16k so it shouldn't hit this
> code.
>
> IMO there's something wrong with the behavior since my JDK 8 source is
(for JSSE):

@Override
public synchronized int getPacketBufferSize() {
return acceptLargeFragments ?
Record.maxLargeRecordSize : Record.maxRecordSize;
}

@Override
public synchronized int getApplicationBufferSize() {
return getPacketBufferSize() - Record.headerSize;
}

And the fields from Record are static (obviously) and final. The value
returned thus shouldn't be able to change.

Rémy


Re: svn commit: r1727992 - in /tomcat/trunk: java/org/apache/tomcat/util/net/SecureNioChannel.java webapps/docs/changelog.xml

2016-02-02 Thread Rémy Maucherat
2016-02-02 10:46 GMT+01:00 Mark Thomas :

> > And the fields from Record are static (obviously) and final. The value
> > returned thus shouldn't be able to change.
>
> But acceptLargeFragments can change via a call to
> SSLSessionImpl.expandBufferSizes().
>
> Correct, thanks, the code has the full explanation actually.

acceptLargeFragments could default to true, which will then cause
getApplicationBufferSize to have the compatible non compliant value:
/**
 * Use large packet sizes now or follow RFC 2246 packet sizes (2^14)
 * until changed.
 *
 * In the TLS specification (section 6.2.1, RFC2246), it is not
 * recommended that the plaintext has more than 2^14 bytes.
 * However, some TLS implementations violate the specification.
 * This is a workaround for interoperability with these stacks.
 *
 * Application could accept large fragments up to 2^15 bytes by
 * setting the system property jsse.SSLEngine.acceptLargeFragments
 * to "true".
 */
private boolean acceptLargeFragments =
Debug.getBooleanProperty("jsse.SSLEngine.acceptLargeFragments",
false);

I don't plan to port the fix to NIO2 at the moment, since it is a
compatibility flag that can be adjusted by users and it would remove my
buffer flexibility :(

Rémy


Re: [VOTE] Release Apache Tomcat 9.0.0.M3

2016-02-03 Thread Rémy Maucherat
2016-02-02 1:20 GMT+01:00 Mark Thomas :

> The proposed Apache Tomcat 9.0.0.M3 release is now available for voting.
>
> This is a milestone release for the 9.0.x branch. It should be
> noted that, as a milestone release:
> - Servlet 4.0 is not finalised
> - The EGs have not started work on JSP 2.4, EL 3.1 or WebSocket 1.2/2.0
>
> The major changes compared to the 9.0.0.M1 branch are:
> - Ability to use OpenSSL with JSSE configuration
> - Update to Tomcat-native 1.2.4 to pick up Windows binaries based on
>   OpenSSL 1.0.2e
> - Allow HTTP redirects to be relative
> - Lots of bug fixes
>
> For full details, see the changelog:
> http://svn.us.apache.org/repos/asf/tomcat/trunk/webapps/docs/changelog.xml
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-9/v9.0.0.M3/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1062/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tags/TOMCAT_9_0_0_M3/
>
> The proposed 9.0.0.M3 release is:
> [ ] Broken - do not release
> [X] Alpha - go ahead and release as 9.0.0.M3
>
> Works for me.

Following this release, I can try to flip the
AprLifecycleListener.aprPreferred flag to false, which will make things
default to NIO+OpenSSL over APR when native is installed [also for non SSL,
the default will then be NIO even when native is installed].

Rémy


Re: [VOTE] Release Apache Tomcat 8.0.32

2016-02-04 Thread Rémy Maucherat
2016-02-03 10:05 GMT+01:00 Mark Thomas :

> The proposed 8.0.32 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 8.0.32
>
> Rémy


Re: svn commit: r1728676 - in /tomcat/trunk: java/org/apache/catalina/core/AprLifecycleListener.java webapps/docs/changelog.xml

2016-02-05 Thread Rémy Maucherat
2016-02-05 16:10 GMT+01:00 Mark Thomas :

> > Switch APR default behavior, as previously announced. Will revert if
> there's a major problem, and will update the SSL docs otherwise.
>
> Any objection to me trying to come up with a different name for this?
> Every time I look at it I seem to spend a couple minutes figuring out
> what true and false mean.
>

The flag means you prefer using the APR connector (or not) :)

>
> I'm thinking of something like:
> preferOpenSslForTls
>

That's not what the flag does. For non SSL, the NIO connector is now always
the default, even if native is installed. For SSL, NIO+OpenSSL is the
default if native is installed, and NIO+JSSE otherwise.

>
> It has the inverse meaning to the current setting but I think it is
> clearer.
>
> Rémy


Re: svn commit: r1728676 - in /tomcat/trunk: java/org/apache/catalina/core/AprLifecycleListener.java webapps/docs/changelog.xml

2016-02-05 Thread Rémy Maucherat
2016-02-05 18:28 GMT+01:00 Mark Thomas :

> Ah. Not sure how I managed to get confused on this. The name makes
> perfect sense in that case.
>
> It must be a bad name for the flag since you got confused by what it does,
so you can change it. I objected because the proposed name was wrong.
Ideas: useAprConnector (not that good, the connector can still be used even
if the flag is false), aprConnectorAsDefault (maybe, but a bit long),
defaultToAprConnector (sounds average).

Rémy


Re: svn commit: r1728676 - in /tomcat/trunk: java/org/apache/catalina/core/AprLifecycleListener.java webapps/docs/changelog.xml

2016-02-06 Thread Rémy Maucherat
2016-02-06 0:02 GMT+01:00 Christopher Schultz 
:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Rémy,
>
> On 2/5/16 12:54 PM, Rémy Maucherat wrote:
> > 2016-02-05 18:28 GMT+01:00 Mark Thomas :
> >
> >> Ah. Not sure how I managed to get confused on this. The name
> >> makes perfect sense in that case.
> >>
> > It must be a bad name for the flag since you got confused by what
> > it does, so you can change it.
>
> +1
>
> It took me a minute to remember that the choice was between APR and
> NIO, not between OpenSSL and JSSE.
>
> Incidentally... how does one choose NIO+JSSE when APR+OpenSSL (or
> NIO+OpenSSL) is available?
>

Yes.

>
> > I objected because the proposed name was wrong. Ideas:
> > useAprConnector (not that good, the connector can still be used
> > even if the flag is false), aprConnectorAsDefault (maybe, but a
> > bit long), defaultToAprConnector (sounds average).
>
> How about connectorForTLS="NIO|NIO2|APR" ?
>

I would like the default to use a single connector for "SSL" and non "SSL".

>
> One can always use "protocol" to be explicit, no?
>

Yes.

Rémy


Re: [VOTE] Release Apache Tomcat 6.0.45

2016-02-08 Thread Rémy Maucherat
2016-02-01 19:52 GMT+01:00 jean-frederic clere :

> The proposed Apache Tomcat 6.0.45 release is now available for voting.
>
> It can be obtained from:
> https://dist.apache.org/repos/dist/dev/tomcat/tomcat-6/v6.0.45/
> The Maven staging repo is:
> https://repository.apache.org/content/repositories/orgapachetomcat-1061/
> The svn tag is:
> http://svn.apache.org/repos/asf/tomcat/tc6.0.x/tags/TOMCAT_6_0_45/
>
> The proposed 6.0.45 release is:
> [ ] Broken - do not release
> [X] Stable - go ahead and release as 6.0.45 Stable
>
> Rémy


Re: JASPIC progress

2016-02-09 Thread Rémy Maucherat
2016-02-09 15:04 GMT+01:00 Mark Thomas :

> I've been working with the JASPIC test suite Arjan recommended[1]. There
> are a few wrinkles but I've got things working well enough that I can
> test the JASPIC code I'm working on.
>
> I've reached the point where I need to hook in to the authenticators and
> this had raised some interesting questions.
>
> The samples use a SevletContextListener to register the JASPIC
> AuthConfigProvider.
>
> The problem is that SevletContextListener events fire after the
> authenticator has been configured.
>
> As I thought about this some more, I realised that there is nothing in
> the Servlet Container profile in the JASPIC spec (that I have been able
> to find) about when AuthConfigProvider registration takes place. This
> means that AuthConfigProvider registrations and de-registrations could
> take place while the web application is running.
>
> I am currently leaning towards a refactoring of AuthenticatorBase along
> the following lines:
> - implement authenticate() and have it delegate to a new protected
>   method doAuthenticate()
> - have authenticate() check (i.e. on every request) for a JASPIC config
>   and use it if present
> - cache what I can (for speed) and use a RegistrationListener to track
>   updates
>
> The refactoring does mean that any custom authenticator will not support
> JASPIC unless it is updated to over-ride doAuthenticate() rather than
> authenticate().
>
> I'm concerned that looking for a JASPIC configuration on every request
> could slow things down. I'll test this and, if it does, I'll make JASPIC
> support something that has to be explicitly enabled for a Context.
>
> Thoughts? Comments?
>

Thanks for the report. However, the more I thought about it, the more I was
convinced JASPIC is useless [besides Arjan asking for it, there's still
nobody actually requesting it as a Tomcat feature], so I don't think it is
a good idea to introduce complexity or degrade performance to have it. I
would go with the last option: require explicit configuration on the
Context.

Rémy

>
> Mark
>
>
> [1] https://github.com/javaee-samples/javaee7-samples
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>


Re: JASPIC progress

2016-02-09 Thread Rémy Maucherat
2016-02-09 15:26 GMT+01:00 Mark Thomas :

> On 09/02/2016 14:18, Rémy Maucherat wrote:
> > 2016-02-09 15:04 GMT+01:00 Mark Thomas :
>
> 
>
> >> Thoughts? Comments?
> >>
> >
> > Thanks for the report. However, the more I thought about it, the more I
> was
> > convinced JASPIC is useless [besides Arjan asking for it, there's still
> > nobody actually requesting it as a Tomcat feature], so I don't think it
> is
> > a good idea to introduce complexity or degrade performance to have it. I
> > would go with the last option: require explicit configuration on the
> > Context.
>
> Thanks for the feedback. I share you concerns regarding performance.
>
> In terms of demand, no-one is asking for it directly but it does provide
> a way to add SAML support (BZ 54503) and I suspect there is demand for
> OAuth as well.
>
> Ok that sound good. In that case since it is relatively specific and not
trivial to setup, so an extra configuration in Tomcat to enable JASPIC
shouldn't be a major issue.

Rémy


Re: JASPIC requirements for request and response wrapping

2016-02-11 Thread Rémy Maucherat
2016-02-11 12:59 GMT+01:00 Mark Thomas :

> JASPIC requires that an authentication module can wrap the
> HttpServletRequest and/or HttpServletResponse passed to it and that the
> wrapped instances are passed to the application. This creates a problem
> since the authenticators are implemented as Valves which pass Tomcat's
> o.a.c.connector.[Request|Response] objects. Tomcat's objects implement
> the Servlet spec interfaces so, as long as no wrapping occurs, all is fine.
>

Tomcat could simply use the already wrapped request/response later when
invoking the filter chain. It would be set on the MessageInfo, right ? In
that case, JASPIC should get Request.getRequest() and
Response.getResponse() as its request/response objects and the wrapped
request/response can go into two new fields in Request/Response. Would that
work ?

Honestly, whatever JASPIC would do with its wrapping is likely irrelevant
for the rest of the Cataline pipeline, so as long as the application sees
it it should be fine.

There is a long standing enhancement request [1] for being able to use
> wrapping with Valves. Rémy is -1 on implementing it.
>
> I'd still like to support JASPIC but I do understand where Rémy is
> coming from w.r.t. the risks of wrapping. With all that in mind I have
> been thinking through different implementation options and I have
> reached the point where I think it makes sense to set out the options as
> I see them for discussion.
>
> 1. Give up on JASPIC.
>Pros: All the design / integration issues go away
>  No performance risk
>Cons: We lose the opportunity for a simple OAuth / SAML solution
>
> 2. Don't support wrapping with JASPIC.
>Pros: All the design / integration issues go away
>Cons: Not specification compliant
>  We don't know how much implementations rely on this
>
> 3. Switch the Valve interface to use HttpServletRequest and
>HttpServletResponse.
>Pros: Enables the use of the associated Wrapper classes
>Cons: Valves need access to the internals. ValveBase could
>  provide utility methods for accessing the wrapped
>  o.a.c.connector.[Request|Response] implementations
>  Breaks existing custom Valves
>
> 4. Make o.a.c.connector.[Request|Response] interfaces and provide
>separate concrete implementations.
>Pros: No change to Valve interface
>Cons: JASPIC would need additional code to bridge between these
>  interfaces and the HttpServlet.*Wrapper classes
>
> 5. Move JASPIC processing to the start of the FilterChain since it
>uses HttpServlet[Request|Response]
>Pros: No / minimal API changes for Tomcat
>Cons: Would result in authentication happening in multiple places.
>  Would need to be very careful to ensure requests couldn't
>  bypass authentication, particularly during JASPIC provider
>  (de)registratiob
>  JASPIC would end up duplciating a lot of the authorization
>  code in AuthenticatorBase
>
>
> Of all of these, I think 4 holds the most promise. The first step for
> that option would be reducing the current public interface of
> o.a.c.connector.[Request|Response] to the minimum that is required. From
> a design point of view that is a good thing to do anyway so I plan to
> work on that while these options are discussed. Even if we don't go for
> option 4, the work would be entirely pointless.
>
> If we do follow option 4 then the Javadoc will need updating to make it
> very clear what is supported and what isn't. Generally, I think the
> message needs to be "If you wrap these you are messing with Tomcat's
> internals. Tread very, very carefully. Correct operation of Tomcat
> depends on the correct behaviour of these methods. Before providing an
> alternative implementation of any method you should review the standard
> implementation and how the method is used."
>
> Mark
>
>
> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014
>
> On the other proposed solutions:
1) Well, we did so much with this junk, it would be a problem to give up.
2) It should be properly supported.
3) Meh. OTOH it's probably just some unwrapping.
4) I think 3 is better.
5) Servlet 3 did a lot to move some auth to the application layer, so if
there's an auth step at this point, I don't think it is so horrible
conceptually.

So maybe I like 3 better if my comment at the beginning cannot work.

Rémy


Re: svn commit: r1729390 - in /tomcat/trunk: java/org/apache/catalina/authenticator/ webapps/docs/

2016-02-11 Thread Rémy Maucherat
2016-02-11 18:16 GMT+01:00 Christopher Schultz :

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Mark,
>
> On 2/10/16 6:10 PM, Mark Thomas wrote:
> > On 09/02/2016 14:22, ma...@apache.org wrote:
> >> Author: markt Date: Tue Feb  9 14:22:29 2016 New Revision:
> >> 1729390
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1729390&view=rev Log:
> >> Refactor AuthenticatorBase to support future plans for the JASPIC
> >> implementation.
> >
> > Just a heads up that I'm currently thinking I'm going to need to
> > revert this. As I learn more about JASPIC the authenticate()
> > doesn't look like the best integration point. invoke() looks
> > better.
> >
> > However, I currently have bigger problems. JASPIC wants to be able
> > to wrap requests and responses. Something that isn't possible in a
> > Valve. I am currently looking at ways to make that work.
>
> A valve can wrap a request or a response. We just don't have the
> classes in Tomcat right now to make that super easy.
>
> Classes like these: https://bz.apache.org/bugzilla/show_bug.cgi?id=45014
>
> After the proposed update, a valve would have the ability to wrap [for the
application, not for other valves]. I am still against straight wrapping.

Rémy


Re: svn commit: r1729896 - in /tomcat/trunk/java/org/apache/catalina/connector: LocalStrings.properties Request.java Response.java

2016-02-11 Thread Rémy Maucherat
2016-02-11 22:39 GMT+01:00 :

> Author: markt
> Date: Thu Feb 11 21:39:35 2016
> New Revision: 1729896
>
> URL: http://svn.apache.org/viewvc?rev=1729896&view=rev
> Log:
> Add the ability for Tomcat internal components, mainly Valves, to wrap the
> request and response that will be passed to the application.
>
> Perfect, exactly what I was thinking ! This is so trivial I feel bad I
failed to come up with that design a long time ago.

Rémy


Re: svn commit: r1729980 - /tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java

2016-02-12 Thread Rémy Maucherat
2016-02-12 12:07 GMT+01:00 Mark Thomas :

> On 12/02/2016 10:53, ma...@apache.org wrote:
> > Author: markt
> > Date: Fri Feb 12 10:53:00 2016
> > New Revision: 1729980
> >
> > URL: http://svn.apache.org/viewvc?rev=1729980&view=rev
> > Log:
> > Performance optimisation. For a very simple servlet this reduces
> processing time by ~2%.
>
> For the curious, I am measuring performance as follows:
> - latest Java 8
> - build from clean checkout
> - comment out access log valve
> - set maxKeepAliveRequests to -1
> - deploy a simple test WAR that includes a Servlet that simply returns
>   "Hello, World!" in plain text
> - run ab using:
>   ab -k -c 4 -n 200 http://localhost:8080/perfTest/SimpleServlet


Yes, that's what I used to optimize from 4.1 to 6.0 :)

>
> Tests running on a fully patched 8-core Windows Server 2008R2 machine
> with nothing else running at the time.
>

But I didn't use that then !

>
> After warm-up, the tests is run 11 times and I am tracking mean and
> standard deviation for each tested configuration.
>

Hum, well, less is fine as well IMO. This sort of ab test is also very easy
to profile (-k is critical for that).

Rémy


Re: NIO + JSSE + NIO + OpenSSL

2016-02-14 Thread Rémy Maucherat
2016-02-14 22:45 GMT+01:00 Mark Thomas :

> All,
>
> In preparation for the connector selection webinar next week, I just did
> a quick test of NIO + JSSE and NIO + OpenSSL.
>
> I was working with 9.0.x trunk including my JASPIC patch
>
> NIO + JSSE ~8200 requests/second
>
> Add the native lib to $CATALINA_BASE/bin and restart.
> No other changes at all.
>
> NIO + OpenSSL ~12300 requests/second
>
>
> Simply dropping in the native library improves TLS performance by
> roughly 50%.
>
> Kudos to remm and jfclere.
>

Thanks !

SSL tests are difficult however, what do you use ? Direct buffers help
OpenSSL a lot for example (socket.directBuffer and socket.directSslBuffer
to true). Also one important item is to make sure the tests all use the
same cipher, especially with ab (JSSE might not use the same cipher as
OpenSSL), something like: ab -k -Z "AES128-GCM-SHA256" forces testing of
this common AES-GCM cipher. Newer and more secure ciphers are often way
slower, no surprise there.

Last, APR is still significantly faster for me, which is rather normal.
It's not that critical at this performance level, probably, but it's here
to stay.

Rémy


Re: NIO + JSSE + NIO + OpenSSL

2016-02-15 Thread Rémy Maucherat
2016-02-15 9:11 GMT+01:00 Mark Thomas :

> > Direct buffers help
> > OpenSSL a lot for example (socket.directBuffer and socket.directSslBuffer
> > to true). Also one important item is to make sure the tests all use the
> > same cipher, especially with ab (JSSE might not use the same cipher as
> > OpenSSL), something like: ab -k -Z "AES128-GCM-SHA256" forces testing of
> > this common AES-GCM cipher. Newer and more secure ciphers are often way
> > slower, no surprise there.
>
> Good point. I'll double check that. What I was really after was a some
> numbers to back up a (probably over simplified) "drop in the native
> library and turo-charge your TLS performance" claim.
>

Ok ! I also started with "good enough" benchmarks initially which indicated
OpenSSL was worthwhile, then realized the results could be more accurate.

>
> > Last, APR is still significantly faster for me, which is rather normal.
> > It's not that critical at this performance level, probably, but it's here
> > to stay.
>
> An in depth comparison between the three options would be useful at some
> point.
>
> Have you done much performance tuning of NIO + OpenSSL?
>

The code looks ok to me, as long as direct buffers are used. It needs to
copy data to the OpenSSL BIO and back, that's the main cost besides
encryption and decryption, and I don't think this can be optimized. My
thinking is APR is faster since it skips these two copy operations.

However, the SSL behaviors simply destroys fancier IO APIs (the async
scatter/gather I wanted to introduce):
- JSSE only wants big buffers ...
- OpenSSL prefers direct buffers
So much flexibility ! :(

Rémy


Re: NIO + JSSE + NIO + OpenSSL

2016-02-15 Thread Rémy Maucherat
2016-02-15 10:45 GMT+01:00 Mark Thomas :

> Looks like such a claim is indeed over simplified.
>
> Having tweaking the test so the same cipher is used, NIO+JSSE is about
> 10% faster than NIO+OpenSSL :(
>
> Enabling direct buffers didn't seem to help.
>

Well, you're probably using an unoptimized cipher then. We'd need to
determine the cipher(s) which should be tested.

For example, with OpenSSL, ECDHE-RSA-AES256-GCM-SHA384 is fast (that's what
ab connects with), while DHE-RSA-AES128-SHA256 is slow (ab connects to JSSE
with that, so I forced it with -Z to compare, and OpenSSL is slightly
slower than JSSE with it). With my OpenSSL, ECDHE-RSA-AES256-GCM-SHA384 is
an order of magnitude faster than DHE-RSA-AES128-SHA256 (and it does sound
more secure as well). JSSE doesn't have ECDHE-RSA-AES256-GCM-SHA384
however. Also my Firefox 44 refuses to handshake with JSSE with its default
configuration (there: ssl_error_no_cypher_overlap) :(

So as I was saying, this SSL testing is hard ;)

I believe OpenSSL is much faster in the real world though (and it
implements the latest security/features/etc).


>
> >> Last, APR is still significantly faster for me, which is rather normal.
> >> It's not that critical at this performance level, probably, but it's
> here
> >> to stay.
> >
> > An in depth comparison between the three options would be useful at some
> > point.
>
> This is definitely looking to be the case. I do wonder what role the
> size of the response plays. I was using very small response bodies in my
> test. If OpenSSL is faster at the encryption but comes with an overhead
> then at some point, as response size increases, it should become the
> better option.
>

I'm not convinced, I'm testing tomcat.gif so that should be small as well.

>
> > Have you done much performance tuning of NIO + OpenSSL?
>
> I'll probably take a quick look at this to see of there is anything
> obvious that can be done to improve things.
>
> No problem.

Rémy


Re: NIO + JSSE + NIO + OpenSSL

2016-02-15 Thread Rémy Maucherat
2016-02-15 14:57 GMT+01:00 jean-frederic clere :

> Using a cipher that allow HTTP/2 to work with the standard browsers
> (like firefox and chrome) make sense otherwise we would be benching an
> old "unsafe" cipher.
>
> I can't rerun my apache con tests right now but that time
> AES128-GCM-SHA256 was the cipher I used.
>
> I had done some extensive (?) benchmarking 6 months ago (more or less),
and things are quite different now, cool :)

Looking at the cipher list from my OpenSSL (Fedora 23 OpenSSL), there are
only 8 ciphers left for the cipher suite that Tomcat uses [and TLS 1.2 and
a RSA certificate]. Half with DHE, half with ECDHE. ab refuses to connect
to JSSE with ECDHE and AES 256. With AES 128, a recent JDK 8 worked, but
not OpenJDK 8 from Fedora [which is unusable at the moment since browsers
refuse to connect as well].

So here's the result array (in k reqs/s):
___OpenSSL
JSSEAPR
ECDHE-RSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH63NA
67
ECDHE-RSA-AES256-SHA384 TLSv1.2 Kx=ECDH 37NA
37
DHE-RSA-AES256-GCM-SHA384 TLSv1.2 Kx=DH2230
22
DHE-RSA-AES256-SHA256   TLSv1.2 Kx=DH__   2028
20
ECDHE-RSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH 6530
70
ECDHE-RSA-AES128-SHA256 TLSv1.2 Kx=ECDH4529
45
DHE-RSA-AES128-GCM-SHA256 TLSv1.2 Kx=DH  2229
23
DHE-RSA-AES128-SHA256   TLSv1.2 Kx=DH__ 2028
20

So OpenSSL is much faster for me for ECDHE, but not with DHE. Browsers use
ECDHE.

Rémy


<    1   2   3   4   5   6   7   8   9   10   >