Gump failure of TestSSLHostConfigCompat

2020-02-18 Thread Mark Thomas 
All,

I've spent the last couple of days investigating this test failure.

The failure was triggered by a change in OpenSSL master that appears to
be a regression. Details at:
https://github.com/openssl/openssl/issues/11108#issuecomment-587947477

That change hasn't been back-ported to 1.1.1 so there are no immediate
concerns.

Should this not be considered a regression then we have options for a
work-around.

Mark

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

[Bug 57661] Delay sending of 100 continue response until application tries to read request body

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=57661

Michael Osipov  changed:

   What|Removed |Added

 CC||micha...@apache.org

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

[Bug 64158] Tomcat 7 performance: remove enforcement that disable keep-alive when busy threads go above disable-keep-alive-percentage

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64158

--- Comment #1 from Torres Yang  ---
Created attachment 37025
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37025&action=edit
Remove enforcement that disable keep-alive when busy threads go above
disable-keep-alive-percentage

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

[Bug 64158] New: Tomcat 7 performance: remove enforcement that disable keep-alive when busy threads go above disable-keep-alive-percentage

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64158

Bug ID: 64158
   Summary: Tomcat 7 performance: remove enforcement that disable
keep-alive when busy threads go above
disable-keep-alive-percentage
   Product: Tomcat 7
   Version: trunk
  Hardware: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: torres.y...@broadcom.com
  Target Milestone: ---

Hi Tomcat,

During our performance testing, we found out another Tomcat 7 Acceptor
bottleneck, which is the Tomcat's fix to enforce busy-thread-ratio below
threshold (default appears to 75%) else disable-keep-alive. E.g. 
org.apache.coyote.http11.Http11Processor#disableKeepAlive

This change was added in 2009 to lock-read worker size (in
JIoEndpoint#getCurrentThreadsBusy and later refactored to
AbstractEndpoint#getCurrentThreadsBusy).
https://github.com/apache/tomcat/commit/ecc2fb6f2ecb7f8cb51559adcfca82fa70b2bebf#diff-3253c70c8b90ec7cd422faf801dfa4f8


In Tomcat 6, there's no enforcement to disable keep alive when busy threads go
above disable-keep-alive-percentage.


In Tomcat 7, we're seeing added contention for the lock in
java.util.concurrent.ThreadPoolExecutor#mainLock. This cause more pressure on
get-pool-size which is also waiting for the same lock.




example stacktrace:


"tomcat-exec-executor-2@16852" daemon prio=5 tid=0x66 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
      at
java.util.concurrent.ThreadPoolExecutor.getActiveCount(ThreadPoolExecutor.java:1830)
      at
org.apache.catalina.core.StandardThreadExecutor.getActiveCount(StandardThreadExecutor.java:281)
      at
org.apache.tomcat.util.net.AbstractEndpoint.getCurrentThreadsBusy(AbstractEndpoint.java:547)
      at
org.apache.coyote.http11.Http11Processor.disableKeepAlive(Http11Processor.java:130)
      at
org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1096)
      at
org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:654)
      at
org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:317)
      - locked (a org.apache.tomcat.util.net.SocketWrapper)
      at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
      at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
      at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
      at java.lang.Thread.run(Thread.java:748)

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

[GitHub] [tomcat] bohmber commented on a change in pull request #244: The '$' in the class name of Digester$EnvironmentPropertySource is no…

2020-02-18 Thread GitBox 
bohmber commented on a change in pull request #244: The '$' in the class name 
of Digester$EnvironmentPropertySource is no…
URL: https://github.com/apache/tomcat/pull/244#discussion_r380866275
 
 

 ##
 File path: java/org/apache/tomcat/util/digester/EnvironmentPropertySource.java
 ##
 @@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.digester;
+
+import java.security.Permission;
+
+import org.apache.tomcat.util.IntrospectionUtils;
+import org.apache.tomcat.util.security.PermissionCheck;
+
+public class EnvironmentPropertySource implements 
IntrospectionUtils.SecurePropertySource {
 
 Review comment:
   Super idea will do it. Give me time til Friday.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[Bug 64157] Tomcat 7 performance: enable tomcat to pre-start pool of min spare threads optionally

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64157

--- Comment #1 from Torres Yang  ---
Created attachment 37024
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37024&action=edit
Improved Tomcat 7 performance by allowing start min spare threads optionally

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

[Bug 64157] Tomcat 7 performance: enable tomcat to pre-start pool of min spare threads optionally

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64157

Torres Yang  changed:

   What|Removed |Added

 OS||All

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

[Bug 64157] New: Tomcat 7 performance: enable tomcat to pre-start pool of min spare threads optionally

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64157

Bug ID: 64157
   Summary: Tomcat 7 performance: enable tomcat to pre-start pool
of min spare threads optionally
   Product: Tomcat 7
   Version: trunk
  Hardware: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: torres.y...@broadcom.com
  Target Milestone: ---

Hi Tomcat,

During our performance testing, we found out that tomcat 7 made a mistake on
preStartMinSpareThread.

In Tomcat 6, tomcat doesn't pre-start and maintain a pool of min spare threads,
instead thread pool increased as requests come in.

In Tomcat 7. tomcat made a mistake and pre-start and maintain a pool of min
spare threads, which impacts the performance.

In details:

- In 2010, Tomcat added this commit to OPTIONALLY prestart the core numbers of
workers
https://github.com/apache/tomcat/commit/26fe8127fd51b9b09a8abefa43d63f1f02960e50#diff-aee166200d5b05354b92c2ba73a758f9
And defect track in Bugzila:
https://bz.apache.org/bugzilla/show_bug.cgi?id=43642

- In 2017, Tomcat added this commit to ALWAYS prestart, ignoring the previous
commit, probably by mistake
https://github.com/apache/tomcat/commit/b0e35f94f5dbb24079d29afe0958a4ee02319927#diff-baa9c50854087e10ddd02bd58f546f2a
 


This change contribute to tomcat performance degradation, and it would improved
tomcat 7 if we are able to optionally start the min spare threads.


Here we attached a patch to solve this problem.

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

[Bug 64156] Tomcat 7 Performance: acceptor thread bottleneck at getPoolSize() located at TaskQueue offer function

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64156

--- Comment #1 from Torres Yang  ---
Created attachment 37023
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37023&action=edit
Improved Tomcat 7 performance by reduced number of getPoolSize() call that
located at TaskQueue offer function

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

[Bug 64156] Tomcat 7 Performance: acceptor thread bottleneck at getPoolSize() located at TaskQueue offer function

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64156

Torres Yang  changed:

   What|Removed |Added

 OS||All

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

[Bug 64156] New: Tomcat 7 Performance: acceptor thread bottleneck at getPoolSize() located at TaskQueue offer function

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64156

Bug ID: 64156
   Summary: Tomcat 7 Performance: acceptor thread bottleneck at
getPoolSize() located at TaskQueue offer function
   Product: Tomcat 7
   Version: trunk
  Hardware: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: torres.y...@broadcom.com
  Target Milestone: ---

Tomcat 7 Performance:

During our performance testing, we found out that the acceptor thread
bottleneck at the getPoolSize() located at TaskQueue offer function, which is
an expensive call to AbstractQueuedSynchronizer acquire lock. 


Proposed fix is to store and use poolSize as local variable. This reduces 2-3
expensive calls down to 1 for each request.

@Override
public boolean offer(Runnable o) {
  //we can't do any checks
  if (parent == null) return super.offer(o);
  // getPoolSize() is expensive call to AbstractQueuedSynchronizer acquire
lock
  final int poolSize = parent.getPoolSize();
  //we are maxed out on threads, simply queue the object
  if (poolSize == parent.getMaximumPoolSize()) return super.offer(o);
  //we have idle threads, just add it to the queue
  if (parent.getSubmittedCount() <= poolSize) return super.offer(o);
  //if we have less threads than maximum force creation of a new thread
  if (poolSize < parent.getMaximumPoolSize()) return false;
  //if we reached here, we need to add it to the queue
  return super.offer(o);
}

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

[Bug 64155] New: Tomcat 7 Performance: acceptor thread bottleneck at getPoolSize() located at TaskQueue offer function

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64155

Bug ID: 64155
   Summary: Tomcat 7 Performance: acceptor thread bottleneck at
getPoolSize() located at TaskQueue offer function
   Product: Tomcat 7
   Version: trunk
  Hardware: All
Status: NEW
  Severity: normal
  Priority: P2
 Component: Catalina
  Assignee: dev@tomcat.apache.org
  Reporter: torres.y...@broadcom.com
  Target Milestone: ---

Created attachment 37022
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37022&action=edit
Reduced getPoolSize

Tomcat 7 Performance:

During our performance testing, we found out that the acceptor thread
bottleneck at the getPoolSize() located at TaskQueue offer function, which is
an expensive call to AbstractQueuedSynchronizer acquire lock. 


Proposed fix is to store and use poolSize as local variable. This reduces 2-3
expensive calls down to 1 for each request.

@Override
public boolean offer(Runnable o) {
  //we can't do any checks
  if (parent == null) return super.offer(o);
  // getPoolSize() is expensive call to AbstractQueuedSynchronizer acquire
lock
  final int poolSize = parent.getPoolSize();
  //we are maxed out on threads, simply queue the object
  if (poolSize == parent.getMaximumPoolSize()) return super.offer(o);
  //we have idle threads, just add it to the queue
  if (parent.getSubmittedCount() <= poolSize) return super.offer(o);
  //if we have less threads than maximum force creation of a new thread
  if (poolSize < parent.getMaximumPoolSize()) return false;
  //if we reached here, we need to add it to the queue
  return super.offer(o);
}

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

[tomcat] branch master updated: Disable session persistence by default on StandardManager

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

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


The following commit(s) were added to refs/heads/master by this push:
 new 81cfd2d  Disable session persistence by default on StandardManager
81cfd2d is described below

commit 81cfd2dc665db684b1fba0de5af4d08102dc50fb
Author: remm 
AuthorDate: Tue Feb 18 17:27:48 2020 +0100

Disable session persistence by default on StandardManager

It can have significant side effects, with numerous large sessions
across many webapps causing start/stop performance drops. There are more
robust alternatives in Tomcat, including the persistent manager and
clustering (delta manager).
It can be enabled back using context.xml.
---
 TOMCAT-NEXT.txt   |  2 --
 conf/context.xml  |  4 ++--
 java/org/apache/catalina/session/StandardManager.java |  2 +-
 webapps/docs/changelog.xml|  4 
 webapps/docs/config/manager.xml   | 15 ---
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
index a510693..5751a5b 100644
--- a/TOMCAT-NEXT.txt
+++ b/TOMCAT-NEXT.txt
@@ -47,8 +47,6 @@ New items for 10.0.0.x onwards:
 
  7. Refactor DefaultServlet to use Ranges in parseRanges().
 
- 8. Disable session persistence: StandardManager.pathname defaults to null.
-
 Deferred until 10.0.x:
 
  1.  Remove the ExtensionValidator and associated classes (assuming that the
diff --git a/conf/context.xml b/conf/context.xml
index 4f6fc0c..0a7cfac 100644
--- a/conf/context.xml
+++ b/conf/context.xml
@@ -24,8 +24,8 @@
 WEB-INF/tomcat-web.xml
 ${catalina.base}/conf/web.xml
 
-
+
 
 
diff --git a/java/org/apache/catalina/session/StandardManager.java 
b/java/org/apache/catalina/session/StandardManager.java
index 48ed3b2..d6c7e70 100644
--- a/java/org/apache/catalina/session/StandardManager.java
+++ b/java/org/apache/catalina/session/StandardManager.java
@@ -108,7 +108,7 @@ public class StandardManager extends ManagerBase {
  * temporary working directory provided by our context, available via
  * the jakarta.servlet.context.tempdir context attribute.
  */
-protected String pathname = "SESSIONS.ser";
+protected String pathname = null;
 
 
 // - Properties
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d22d40f..ad7b5fd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -51,6 +51,10 @@
 Refactor HttpServlet.doOptions() to improve performance.
 (markt)
   
+  
+Disable StandardManager session persistence by default. It can be
+enabled back in context.xml. (remm)
+  
 
   
   
diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index e067f45..821085e 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -116,10 +116,10 @@
 Absolute or relative (to the work directory for this Context)
 pathname of the file in which session state will be preserved
 across application restarts, if possible.  The default is
-"SESSIONS.ser".See
+null.See
 Persistence Across Restarts
 for more information. This persistence may be
-disabled by setting this attribute to an empty string.
+enabled by setting this attribute to a non empty string.
   
 
   
@@ -555,15 +555,16 @@
 
   
 
-  
+  
 
 As documented above, every web application by default has
-standard manager implementation configured, and it performs session
-persistence across restarts. To disable this persistence feature, create
+standard manager implementation configured, which can perform session
+persistence across restarts. To enable this persistence feature, create
 a Context configuration file for your web
-application and add the following element there:
+application and add the following element there (in this example,
+it will save sessions to files named SESSIONS.ser):
 
-
+
   
 
 


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

Re: [tomcat] branch master updated: Disable session persistence by default

2020-02-18 Thread Christopher Schultz 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Rémy,

On 2/18/20 04:42, Rémy Maucherat wrote:
> On Tue, Feb 18, 2020 at 10:32 AM Mark Thomas  > wrote:
>
> On 18/02/2020 09:06, r...@apache.org 
> wrote:
>> This is an automated email from the ASF dual-hosted git
>> repository.
>>
>> remm pushed a commit to branch master in repository
>> https://gitbox.apache.org/repos/asf/tomcat.git
>>
>>
>> The following commit(s) were added to refs/heads/master by this
>> push: new d59db7a  Disable session persistence by default d59db7a
>> is described below
>>
>> commit d59db7ae7529fd9f2b067622ae661fd9338b2478 Author: remm
>> mailto:r...@apache.org>> AuthorDate: Tue Feb 18
>> 10:05:49 2020 +0100
>>
>> Disable session persistence by default
>>
>> Persistence should be configured explicitly, either with
> pathname or
>> when using a persistent manager.
>
> Hmm. I'm not sure about this.
>
> I have no figures to back this up but I imagine quite a few people
> use Tomcat the way I have used it in the past. I have often used
> Tomcat to host some simple services that aren't used very often or
> where brief downtime is OK (so a quick restart to pick up a change
> isn't an issue) but session persistence is useful.
>
> For sure, persistence across restarts is not the solution for
> production systems with high availability requirements.
>
> I guess I'm wondering what are the benefits for doing this. I can
> see some downsides but I'm struggling to see the benefit.
>
>
> It increases shutdown and startup when there are lots of sessions,
> and also uses disk space, and production people have been
> complaining about that in the past (then they disable the feature
> since it's obviously not for them).
>
> [...]
>
> As a result, I think it is better to start phasing out this feature
> in Tomcat 10.

+1

It's easy to re-enable.

> You also just said it is mostly useless except it might be part of
> the "worst practices" playbook of many people.

I wouldn't call it a "worst practice". It's just not scalable, and not
appropriate for many environments. If we think that all non-scalable
solutions should be removed, then we should remove the DeltaManager, too
.

> On the user list, there's also a person who wants to add saving the
>  principal to it, adding more risk (especially in 7.0, 8.5 and 9.0
> where the principal still includes the clear text password).
There is a better solution to that: stop storing the principal
password. It's pretty much never needed... is it?

- -chris
-BEGIN PGP SIGNATURE-
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl5MEBwACgkQHPApP6U8
pFhQSQ/9FYi6zJdEvGvVtAGoO/6/HJJhY17KyVaUEapXHZld0T7op8Ro0UgoM0Mi
mOD2VSolPJW1dHCMcB8ESOySh92yn85k6Iz3eK2dGgxHPyFu5gbQg/ko+0dXagS7
2vQ5JPcNW/iRjXUgkZeX48qLWvlGkemZZl/Gd2AUiRJeuCJFn+jALU3YVIHtwYPT
oh0k5SA3ef2DPWYTfaw7Xk72XIHGR5vgbJBZmcahbavEY2/5XB1/PPK9+Z2KFDZP
fFuVcyP/aeLmbDhTAKbXLCrdYR1FeIL2G8nl6j/+kIX0VStMLsi7YXEDA8YRk4tE
pZgRFQEt89ZLBt38jWlezi240QWXcH31NtTTqLIhHlRlN+VKASlUpqG9yzfEkeis
MingrbOCGbWD42y5vsX+4vInFP535pJztKeKa3FgK8cXX5x999Sg3mrncvl4P8ge
EiSo8zkLmpTlkU+NGz2A0sBlSR5JZQgQClcL+TV5jxStQmkldVSlFKV+Av7QYCK0
UlmKcEHlS4qJMj3gBJu28owbcHI0+HMfdsybsvI8eKChucb8wF0nyYEo7nieGG6A
bTpEb8tO4kBXtjqSy60a7ck1asprPA9STnJS4xm3N4krafbfXi89I7G6QZI5jD4f
z9e9viOpfarI9OSwBDxe+OSYknKtR6EnfOEE+hAn7cvcXsvvc0I=
=3RZd
-END PGP SIGNATURE-

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

[GitHub] [tomcat] ChristopherSchultz commented on issue #242: get a hands on ...

2020-02-18 Thread GitBox 
ChristopherSchultz commented on issue #242: get a hands on ...
URL: https://github.com/apache/tomcat/pull/242#issuecomment-587542753
 
 
   > The issue with refactoring for clean code is that the benefits are 
subjective and people are going to have different views.
   
   There is a term for this: 
[bikeshedding](https://en.wikipedia.org/wiki/Law_of_triviality)
   
   Specific refactorings are often a good idea. Re-factoring code to make it 
"feel nicer" for a single programmer often ends up in a commit war.
   
   Let's see where we can build consensus, and move-forward with those 
refactorings. Others will simply have to wait.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] ChristopherSchultz commented on a change in pull request #244: The '$' in the class name of Digester$EnvironmentPropertySource is no…

2020-02-18 Thread GitBox 
ChristopherSchultz commented on a change in pull request #244: The '$' in the 
class name of Digester$EnvironmentPropertySource is no…
URL: https://github.com/apache/tomcat/pull/244#discussion_r380774990
 
 

 ##
 File path: java/org/apache/tomcat/util/digester/EnvironmentPropertySource.java
 ##
 @@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.digester;
+
+import java.security.Permission;
+
+import org.apache.tomcat.util.IntrospectionUtils;
+import org.apache.tomcat.util.security.PermissionCheck;
+
+public class EnvironmentPropertySource implements 
IntrospectionUtils.SecurePropertySource {
 
 Review comment:
   Since you are doing a bit of re-factoring, could you add some javadoc? Your 
use of the class suggests that you understand its use and the need for its 
existence. No need for anything fancy. Just a one or two sentence description 
and maybe a reference to the users manual if appropriate.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] mads1980 commented on issue #246: OpenSSLEngine improvements to guard against multiple shutdown() calls triggered by construction exception and finalize() later

2020-02-18 Thread GitBox 
mads1980 commented on issue #246: OpenSSLEngine improvements to guard against 
multiple shutdown() calls triggered by construction exception and finalize() 
later
URL: https://github.com/apache/tomcat/pull/246#issuecomment-587507009
 
 
   Hi Remy, thank you for your feedback. Object.finalize() has been 
   deprecated since Java 9 because of its weird subtleties. Still, even if 
   the JVM behaviour is broken, shouldn't we still fix this? The solution 
   seems simple enough, especially if this would avoid JVM crashes.
   *
   Manuel Dominguez Sarmiento*
   
   On 18/02/2020 11:31, Rémy Maucherat wrote:
   >
   > It's quite normal as the problem was evident and legitimate with 
   > OpenSSLContext, while with OpenSSLEngine I consider the JVM is broken. 
   > I'd like to remind that the engine is referenced directly from the 
   > channel, and then the JVM seems to happily discard everything in a 
   > random order. So: whatever ...
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub 
   > 
,
 
   > or unsubscribe 
   > 
.
   >
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] rmaucher commented on issue #246: OpenSSLEngine improvements to guard against multiple shutdown() calls triggered by construction exception and finalize() later

2020-02-18 Thread GitBox 
rmaucher commented on issue #246: OpenSSLEngine improvements to guard against 
multiple shutdown() calls triggered by construction exception and finalize() 
later
URL: https://github.com/apache/tomcat/pull/246#issuecomment-587488112
 
 
   It's quite normal as the problem was evident and legitimate with 
OpenSSLContext, while with OpenSSLEngine I consider the JVM is broken. I'd like 
to remind that the engine is referenced directly from the channel, and then the 
JVM seems to happily discard everything in a random order. So: whatever ...


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] mads1980 opened a new pull request #246: OpenSSLEngine improvements to guard against multiple shutdown() calls triggered by construction exception and finalize() later

2020-02-18 Thread GitBox 
mads1980 opened a new pull request #246: OpenSSLEngine improvements to guard 
against multiple shutdown() calls triggered by construction exception and 
finalize() later
URL: https://github.com/apache/tomcat/pull/246
 
 
   We started getting random SIGSEGV JVM crashes on our servers after upgrading 
to OpenJDK 13.0.2
   
   The crashes are always in tcnative code, on the JVM Finalizer daemon thread, 
at OpenSSLEngine.shutdown(). We're on Tomcat 9.0.31, tcnative 1.2.23, OpenSSL 
1.1.1d and APR 1.7.0 using NIO connectors. This combination works fine with 
Oracle JDK 8u202 as well as OpenJDK 8u242, but crashes began immediately after 
upgrading to OpenJDK 13.0.2
   
   Since the failure is related to tcnative code and finalization, we guess the 
new JVM is probably invoking the finalizers sooner, so this is exposing a bug 
in OpenSSLEngine which was already present, but was not being triggered on 
previous JVMs because of different timing.
   
   The crash occurs when SSL.freeSSL() is invoked, so this probably means 
either the pointer is zero, or a double SSL.freeSSL() is invoked. This can 
happen if there are uncaught exceptions within the OpenSSLEngine constructor, 
as finalization can execute concurrently with object construction.
   
   We adapted the solution found in OpenSSLContext.destroy() which evidently 
was causing similar issues. The current volatile "destroyed" checks found in 
OpenSSLEngine seem to be insufficient, so we replaced it with an AtomicBoolean. 
Also, we check whether "networkBIO" and
   "ssl" are zero in shutdown() to avoid attempting to free a null pointer.
   
   We found a discussing regarding these issues in the following thread:
   
http://mail-archives.apache.org/mod_mbox/tomcat-dev/201901.mbox/%3c44ce40f1-36a1-df9b-b648-8635d9d84...@kippdata.de%3E
   
   It seems the improvements made it to OpenSSLContext but not OpenSSLEngine. 
This pull request should fix remaining issues.
   
   See attached sample HotSpot error dumps.
   
   
[hs_err_pid28766.log](https://github.com/apache/tomcat/files/4219652/hs_err_pid28766.log)
   
[hs_err_pid32426.log](https://github.com/apache/tomcat/files/4219653/hs_err_pid32426.log)
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] mads1980 closed pull request #245: OpenSSLEngine improvements to guard against multiple shutdown() calls triggered by construction exception and finalize() later

2020-02-18 Thread GitBox 
mads1980 closed pull request #245: OpenSSLEngine improvements to guard against 
multiple shutdown() calls triggered by construction exception and finalize() 
later
URL: https://github.com/apache/tomcat/pull/245
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] mads1980 opened a new pull request #245: OpenSSLEngine improvements to guard against multiple shutdown() calls triggered by construction exception and finalize() later

2020-02-18 Thread GitBox 
mads1980 opened a new pull request #245: OpenSSLEngine improvements to guard 
against multiple shutdown() calls triggered by construction exception and 
finalize() later
URL: https://github.com/apache/tomcat/pull/245
 
 
   We started getting random SIGSEGV JVM crashes on our servers after upgrading 
to OpenJDK 13.0.2
   
   The crashes are always in tcnative code, on the JVM Finalizer daemon thread, 
at OpenSSLEngine.shutdown(). We're on Tomcat 9.0.31, tcnative 1.2.23, OpenSSL 
1.1.1d and APR 1.7.0 using NIO connectors. This combination works fine with 
Oracle JDK 8u202 as well as OpenJDK 8u242, but crashes began immediately after 
upgrading to OpenJDK 13.0.2
   
   Since the failure is related to tcnative code and finalization, we guess the 
new JVM is probably invoking the finalizers sooner, so this is exposing a bug 
in OpenSSLEngine which was already present, but was not being triggered on 
previous JVMs because of different timing.
   
   The crash occurs when SSL.freeSSL() is invoked, so this probably means 
either the pointer is zero, or a double SSL.freeSSL() is invoked. This can 
happen if there are uncaught exceptions within the OpenSSLEngine constructor, 
as finalization can execute concurrently with object construction.
   
   We adapted the solution found in OpenSSLContext.destroy() which evidently 
was causing similar issues. The current volatile "destroyed" checks found in 
OpenSSLEngine seem to be insufficient, so we replaced it with an AtomicBoolean. 
Also, we check whether "networkBIO" and 
   "ssl" are zero in shutdown() to avoid attempting to free a null pointer.
   
   We found a discussing regarding these issues in the following thread:
   
http://mail-archives.apache.org/mod_mbox/tomcat-dev/201901.mbox/%3c44ce40f1-36a1-df9b-b648-8635d9d84...@kippdata.de%3E
   
   It seems the improvements made it to OpenSSLContext but not OpenSSLEngine. 
This pull request should fix remaining issues.
   
   See sample crash dump extract below:
   
   #
   # A fatal error has been detected by the Java Runtime Environment:
   #
   #  SIGSEGV (0xb) at pc=0x7fc98d024f58, pid=32426, tid=32435
   #
   # JRE version: OpenJDK Runtime Environment (13.0.2+8) (build 13.0.2+8)
   # Java VM: OpenJDK 64-Bit Server VM (13.0.2+8, mixed mode, tiered, 
compressed oops, g1 gc, linux-amd64)
   # Problematic frame:
   # C  [libcrypto.so.1.1+0x16ff58]  CRYPTO_get_ex_data+0x8
   #
   # Core dump will be written. Default location: Core dumps may be processed 
with "/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e" (or dumping to 
//core.32426)
   #
   # If you would like to submit a bug report, please visit:
   #   http://bugreport.java.com/bugreport/crash.jsp
   # The crash happened outside the Java Virtual Machine in native code.
   # See problematic frame for where to report the bug.
   #
   
   ---  S U M M A R Y 
   
   Command Line: -Dcatalina.home=/home/tomcat -Dcatalina.base=/home/tomcat 
-Djava.library.path=/home/tomcat/lib 
-Djava.security.properties=/home/tomcat/conf/java.security.properties 
-Djava.io.tmpdir=/var/tmp 
-Djava.util.logging.manager=org.apache.juli.ClassLoaderLogManager 
-Djava.util.logging.config.file=/home/tomcat/conf/logging.properties 
-XX:ErrorFile=/home/tomcat/logs/hs_err_pid%p.log 
-XX:HeapDumpPath=/home/tomcat/logs/ --illegal-access=permit -Xshare:off 
-XX:+PrintVMOptions -XX:+UnlockExperimentalVMOptions 
-XX:+UnlockDiagnosticVMOptions -XX:+UsePerfData -XX:+DoEscapeAnalysis 
-Djava.awt.headless=true -Djava.net.preferIPv4Stack=true 
-XX:-OmitStackTraceInFastThrow -XX:+PrintStringTableStatistics 
-XX:StringTableSize=103 -XX:SymbolTableSize=103 -XX:+PrintCompilation 
-XX:+PrintInlining -XX:+UseBiasedLocking -XX:BiasedLockingStartupDelay=0 
-XX:MaxInlineLevel=15 -Xms5G -Xmx5G -Xss512K -XX:MetaspaceSize=128M 
-XX:MaxMetaspaceSize=256M -XX:+UseLargePages -XX:InitialCodeCacheSize=128M 
-XX:ReservedCodeCacheSize=128M -XX:+UseCodeCacheFlushing -XX:+UseCompressedOops 
-verbose:gc -XX:+DisableExplicitGC -XX:+ParallelRefProcEnabled -XX:+UseG1GC 
-XX:MaxGCPauseMillis=200 -XX:GCTimeRatio=4 -XX:G1NewSizePercent=30 
-XX:G1MaxNewSizePercent=60 -XX:InitiatingHeapOccupancyPercent=0 
-XX:G1ReservePercent=10 -XX:G1HeapRegionSize=32M -XX:+UseStringDeduplication 
-Xlog:gc*,gc+ergo*=trace,gc+heap=debug,classhisto*=trace,safepoint:file=/home/tomcat/logs/gclog.2020-02-17:level,time,uptime,tags
 
-Dcom.renxo.cms.util.io.network.NetworkUtils.preferredLocalHostSubnet=192.168.0.0/16
 
-Dcom.renxo.cms.util.io.network.NetworkUtils.developmentEnvironmentSubnet=192.168.3.0/24
 -javaagent:/home/tomcat/apminsight/apminsight-javaagent.jar 
-Dapminsight.application.name=homer.renxo.net applications 
-Dfile.encoding=UTF-8 -Dsun.jnu.encoding=UTF-8 
-Djava.security.egd=file:/dev/urandom 
-Djava.rmi.server.hostname=homer.renxo.net -Djava.io.tmpdir=/home/tomcat/temp/ 
-Dcommons.daemon.process.id=32426 -Dcommons.daemon.process.parent=28765 
-Dcommons.daemon.version=1.2.2 abort org.apache.catalina.st

[Bug 64153] New: ServerContainer is not available in ServletContext

2020-02-18 Thread bugzilla 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64153

Bug ID: 64153
   Summary: ServerContainer is not available in ServletContext
   Product: Tomcat 9
   Version: 9.0.31
  Hardware: PC
OS: Linux
Status: NEW
  Severity: normal
  Priority: P2
 Component: WebSocket
  Assignee: dev@tomcat.apache.org
  Reporter: boris_pet...@live.com
  Target Milestone: -

After updating Tomcat from 9.0.30 to 9.0.31 our integration tests don't even
start. Running Tomcat 9.0.31 in production works fine. In the integration tests
we use "org.apache.tomcat.embed:tomcat-embed-websocket" so I guess the issue is
somewhere there.

We use CometD and upon boot, the following exception is raised:

```
java.lang.IllegalArgumentException: Missing WebSocket ServerContainer
at
org.cometd.websocket.server.WebSocketTransport.init(WebSocketTransport.java:72)
at
org.cometd.server.BayeuxServerImpl.initializeServerTransports(BayeuxServerImpl.java:255)
at
org.cometd.server.BayeuxServerImpl.doStart(BayeuxServerImpl.java:135)
at
org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
at org.cometd.server.CometDServlet.init(CometDServlet.java:64)
at
org.cometd.annotation.AnnotationCometDServlet.init(AnnotationCometDServlet.java:52)
at javax.servlet.GenericServlet.init(GenericServlet.java:244)
at
org.apache.catalina.core.StandardWrapper.initServlet(StandardWrapper.java:1134)
at
org.apache.catalina.core.StandardWrapper.loadServlet(StandardWrapper.java:1089)
at
org.apache.catalina.core.StandardWrapper.load(StandardWrapper.java:983)
at
org.apache.catalina.core.StandardContext.loadOnStartup(StandardContext.java:4871)
at
org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5180)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
at
org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1384)
at
org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1374)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at
org.apache.tomcat.util.threads.InlineExecutorService.execute(InlineExecutorService.java:75)
at
java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:140)
at
org.apache.catalina.core.ContainerBase.startInternal(ContainerBase.java:909)
at
org.apache.catalina.core.StandardHost.startInternal(StandardHost.java:841)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
at
org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1384)
at
org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1374)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at
org.apache.tomcat.util.threads.InlineExecutorService.execute(InlineExecutorService.java:75)
at
java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:140)
at
org.apache.catalina.core.ContainerBase.startInternal(ContainerBase.java:909)
at
org.apache.catalina.core.StandardEngine.startInternal(StandardEngine.java:262)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
at
org.apache.catalina.core.StandardService.startInternal(StandardService.java:421)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
at
org.apache.catalina.core.StandardServer.startInternal(StandardServer.java:930)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:183)
at org.apache.catalina.startup.Tomcat.start(Tomcat.java:467)
...
```

The error is coming from this line:

https://github.com/cometd/cometd/blob/3.1.12/cometd-java/cometd-java-websocket/cometd-java-websocket-javax-server/src/main/java/org/cometd/websocket/server/WebSocketTransport.java#L70

This is our initialization code:

```
tomcat = Tomcat.new
tomcat.port = @port
tomcat.connector.add_upgrade_protocol(Http2Protocol.new) # this will create the
default connector

base_path = java.nio.file.Path.of(Dir.tmpdir, 'project/webapp').to_string
root_ctx = tomcat.add_context('/project', base_path)
root_ctx.add_mime_mapping('css', 'text/css')
root_ctx.add_mime_mapping('js', 'application/javascript')
root_ctx.loader = WebappLoader.new(JRuby.runtime.jruby_class_loader)

ctx_cfg = ProjectContextConfig.new
ctx_cfg.default_web_xml = tomcat.no_default_web_xml_path
root_ctx.add_lifecycle_listener(ctx_cfg)

tomcat.start
```

I have no clue what could have changed and what is broken. Any suggestions as
to how do I debug this would be appreciated.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-

[tomcat-connectors] branch master updated: Update documentation reference to secret attribute of Tomcat AJP Connector, following up to its rename in recent versions of Tomcat.

2020-02-18 Thread kkolinko 
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new 83dc3e4  Update documentation reference to secret attribute of Tomcat 
AJP Connector, following up to its rename in recent versions of Tomcat.
83dc3e4 is described below

commit 83dc3e486509cf63b0c478b46d75b4b886088652
Author: Konstantin Kolinko 
AuthorDate: Tue Feb 18 15:16:09 2020 +0300

Update documentation reference to secret attribute of Tomcat AJP Connector,
following up to its rename in recent versions of Tomcat.
---
 xdocs/reference/workers.xml | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xdocs/reference/workers.xml b/xdocs/reference/workers.xml
index d17c7e4..039469b 100644
--- a/xdocs/reference/workers.xml
+++ b/xdocs/reference/workers.xml
@@ -974,9 +974,10 @@ This feature has been added in jk 1.2.38.
 You can set a secret keyword on the Tomcat AJP Connector. Then only requests
 from workers with the same secret keyword will be accepted.
 
-Use request.secret="secret key word" in your Tomcat AJP Connector
-configuration in Tomcat 5.5 or 6.0 and requiredSecret="secret key word"
-in Tomcat 7.0 onwards.
+Use attribute secret="secret key word" in your Tomcat AJP Connector 
configuration.
+(Historical note: the attribute name was requiredSecret in Tomcat
+9.0, 8.x, 7.0 versions released earlier than February 2020,
+request.secret in Tomcat 6.0 and earlier.)
 
 
 If you set a secret on a load balancer, all its members will inherit this 
secret.


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

Re: [tomcat] branch master updated: Disable session persistence by default

2020-02-18 Thread Mark Thomas 
On 18/02/2020 09:42, Rémy Maucherat wrote:
> On Tue, Feb 18, 2020 at 10:32 AM Mark Thomas  > wrote:
> 
> On 18/02/2020 09:06, r...@apache.org  wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> >      new d59db7a  Disable session persistence by default
> > d59db7a is described below
> >
> > commit d59db7ae7529fd9f2b067622ae661fd9338b2478
> > Author: remm mailto:r...@apache.org>>
> > AuthorDate: Tue Feb 18 10:05:49 2020 +0100
> >
> >     Disable session persistence by default
> >     
> >     Persistence should be configured explicitly, either with
> pathname or
> >     when using a persistent manager.
> 
> Hmm. I'm not sure about this.
> 
> I have no figures to back this up but I imagine quite a few people use
> Tomcat the way I have used it in the past. I have often used Tomcat to
> host some simple services that aren't used very often or where brief
> downtime is OK (so a quick restart to pick up a change isn't an issue)
> but session persistence is useful.
> 
> For sure, persistence across restarts is not the solution for production
> systems with high availability requirements.
> 
> I guess I'm wondering what are the benefits for doing this. I can see
> some downsides but I'm struggling to see the benefit.
> 
> 
> It increases shutdown and startup when there are lots of sessions, and
> also uses disk space, and production people have been complaining about
> that in the past (then they disable the feature since it's obviously not
> for them). You also just said it is mostly useless except it might be
> part of the "worst practices" playbook of many people.

Fair enough. I'm convinced.

> On the user list, there's also a person who wants to add saving the
> principal to it, adding more risk (especially in 7.0, 8.5 and 9.0 where
> the principal still includes the clear text password).

I'm just reviewing that proposal now.

> As a result, I think it is better to start phasing out this feature in
> Tomcat 10. The user can then either:
> - configure it back using pathname (it's very easy to do it in
> context.xml - the default context.xml actually includes a commented out
> Manager element to disable persistence, it would switch to enabling it)

That would work for me.

Thanks for the explanation.

Mark


> - use a persistent manager
> - use the delta manager
> 
> Rémy
>  
> 
> 
> Mark
> 
> 
> > ---
> >  TOMCAT-NEXT.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
> > index 3be3e12..a510693 100644
> > --- a/TOMCAT-NEXT.txt
> > +++ b/TOMCAT-NEXT.txt
> > @@ -47,6 +47,7 @@ New items for 10.0.0.x onwards:
> > 
> >   7. Refactor DefaultServlet to use Ranges in parseRanges().
> > 
> > + 8. Disable session persistence: StandardManager.pathname
> defaults to null.
> > 
> >  Deferred until 10.0.x:
> > 
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> 
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> 
> >
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> 
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 
> 


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

Re: [tomcat] branch master updated: Disable session persistence by default

2020-02-18 Thread Rémy Maucherat 
On Tue, Feb 18, 2020 at 10:32 AM Mark Thomas  wrote:

> On 18/02/2020 09:06, r...@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> >  new d59db7a  Disable session persistence by default
> > d59db7a is described below
> >
> > commit d59db7ae7529fd9f2b067622ae661fd9338b2478
> > Author: remm 
> > AuthorDate: Tue Feb 18 10:05:49 2020 +0100
> >
> > Disable session persistence by default
> >
> > Persistence should be configured explicitly, either with pathname or
> > when using a persistent manager.
>
> Hmm. I'm not sure about this.
>
> I have no figures to back this up but I imagine quite a few people use
> Tomcat the way I have used it in the past. I have often used Tomcat to
> host some simple services that aren't used very often or where brief
> downtime is OK (so a quick restart to pick up a change isn't an issue)
> but session persistence is useful.
>
> For sure, persistence across restarts is not the solution for production
> systems with high availability requirements.
>
> I guess I'm wondering what are the benefits for doing this. I can see
> some downsides but I'm struggling to see the benefit.
>

It increases shutdown and startup when there are lots of sessions, and also
uses disk space, and production people have been complaining about that in
the past (then they disable the feature since it's obviously not for them).
You also just said it is mostly useless except it might be part of the
"worst practices" playbook of many people.
On the user list, there's also a person who wants to add saving the
principal to it, adding more risk (especially in 7.0, 8.5 and 9.0 where the
principal still includes the clear text password).

As a result, I think it is better to start phasing out this feature in
Tomcat 10. The user can then either:
- configure it back using pathname (it's very easy to do it in context.xml
- the default context.xml actually includes a commented out Manager element
to disable persistence, it would switch to enabling it)
- use a persistent manager
- use the delta manager

Rémy


>
> Mark
>
>
> > ---
> >  TOMCAT-NEXT.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
> > index 3be3e12..a510693 100644
> > --- a/TOMCAT-NEXT.txt
> > +++ b/TOMCAT-NEXT.txt
> > @@ -47,6 +47,7 @@ New items for 10.0.0.x onwards:
> >
> >   7. Refactor DefaultServlet to use Ranges in parseRanges().
> >
> > + 8. Disable session persistence: StandardManager.pathname defaults to
> null.
> >
> >  Deferred until 10.0.x:
> >
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: [tomcat] branch master updated: Disable session persistence by default

2020-02-18 Thread Mark Thomas 
On 18/02/2020 09:06, r...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
> 
> 
> The following commit(s) were added to refs/heads/master by this push:
>  new d59db7a  Disable session persistence by default
> d59db7a is described below
> 
> commit d59db7ae7529fd9f2b067622ae661fd9338b2478
> Author: remm 
> AuthorDate: Tue Feb 18 10:05:49 2020 +0100
> 
> Disable session persistence by default
> 
> Persistence should be configured explicitly, either with pathname or
> when using a persistent manager.

Hmm. I'm not sure about this.

I have no figures to back this up but I imagine quite a few people use
Tomcat the way I have used it in the past. I have often used Tomcat to
host some simple services that aren't used very often or where brief
downtime is OK (so a quick restart to pick up a change isn't an issue)
but session persistence is useful.

For sure, persistence across restarts is not the solution for production
systems with high availability requirements.

I guess I'm wondering what are the benefits for doing this. I can see
some downsides but I'm struggling to see the benefit.

Mark


> ---
>  TOMCAT-NEXT.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
> index 3be3e12..a510693 100644
> --- a/TOMCAT-NEXT.txt
> +++ b/TOMCAT-NEXT.txt
> @@ -47,6 +47,7 @@ New items for 10.0.0.x onwards:
>  
>   7. Refactor DefaultServlet to use Ranges in parseRanges().
>  
> + 8. Disable session persistence: StandardManager.pathname defaults to null.
>  
>  Deferred until 10.0.x:
>  
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


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

[tomcat] branch master updated: Disable session persistence by default

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

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


The following commit(s) were added to refs/heads/master by this push:
 new d59db7a  Disable session persistence by default
d59db7a is described below

commit d59db7ae7529fd9f2b067622ae661fd9338b2478
Author: remm 
AuthorDate: Tue Feb 18 10:05:49 2020 +0100

Disable session persistence by default

Persistence should be configured explicitly, either with pathname or
when using a persistent manager.
---
 TOMCAT-NEXT.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
index 3be3e12..a510693 100644
--- a/TOMCAT-NEXT.txt
+++ b/TOMCAT-NEXT.txt
@@ -47,6 +47,7 @@ New items for 10.0.0.x onwards:
 
  7. Refactor DefaultServlet to use Ranges in parseRanges().
 
+ 8. Disable session persistence: StandardManager.pathname defaults to null.
 
 Deferred until 10.0.x:
 


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