Re: [QUESTION] Purchase UML tool using Google security funding
Am 29.08.24 um 18:29 schrieb Mark Thomas: On 29/08/2024 15:34, Felix Schumacher wrote: While I don't object to buying a license, I would love to know, which diagram you looked at and what exactly did not work out. (the activation stuff in mermaid is brittle, but I think I managed to get them all right) I couldn't find a way to get the gap in the activation of Catalina between the call to setParentClassLoader() and start(). I see how you fixed that. Nice. There are a couple of places where the message arrows don't quite meet up with the activation bar correctly and the await note isn't quite in the right place. I only see one gap at the message await() from Catalina to itself. The note is a bit better placed, when we use "note right" (or even "note left") instead of "note right of Catalina". To get even more of the original look, we can use UML style, by adding "skinparam style strictuml" and to mimic the original drawing even more, we can switch off the actors at the bottom, by specifying "hide footbox". (full code at the end) and for mermaidjs I got I found that very hard to read but I suspect that is a fairly easy fix. My biggest complaint with mermaidjs was that the text on messages to self is centered rather than to the right. That is probably fixable too. We can configure some stuff in a YAML like begin block by adding (at the top of the file) --- config: messageAlign: left mirrorActors: false --- The three dashes are important and part of the header block. mirrorActors switch off the actors at the bottom, to mimic the original diagram (not that I like it that much). There is a missing activation bar for the digester but that might be due to the issues you mentioned. Yes, there was a missing "deactivate Digester" (again, full code at the end) The label is in the right place for await() which is good. To summarize my findings. plantuml seemed to be more predictable and feature-rich (for sequence diagrams) than mermaidjs. But I didn't see any showstoppers with both of them. Another alternative to use would be umlet (https://www.umlet.com/), which I used way back, but haven't looked at lately. I'll take a look. I hope you didn't mind the inline code and thus this long message. Not at all. This is all really useful. I do really like the idea of the source being human readable but I think I am still leaning towards Visual Paradigm because it doesn't have any of the niggles in the output and generally, we have a lot more control over the final layout. Another factor is time. While Visual Paradigm also has its quirks, I've found I have spent far less time cajoling the tool into providing the output I want. Yes, I can understand that. It seems, that we have different bars on what is acceptable visually :) As promised above in the text, the code for plantuml with the corrections is: @startuml hide footbox skinparam style strictuml activate Bootstrap Bootstrap -> Bootstrap: initClassLoaders() Bootstrap -->> Catalina ** : newInstance() Bootstrap -> Catalina: setParentClassLoader() activate Catalina deactivate Catalina Bootstrap -> Catalina: start() activate Catalina Catalina -> Catalina: load() activate Catalina Catalina -> Catalina: initNaming() Catalina -> Catalina: parseServerXml() activate Catalina Catalina -->> Digester ** : note right of Digester The digester creates all of the objects defined //server.xml// but only the Server is shown here for simplicity end note Catalina -> Digester: parse() activate Digester Digester -->> Server ** : deactivate Digester deactivate Catalina deactivate Catalina Catalina -> Catalina: initStream() Catalina -> Server: init() activate Server deactivate Server Catalina -> Server: start() activate Server Catalina -> Catalina: await() activate Catalina note right This is where Tomcat spends time serving requests end note deactivate Catalina Catalina -> Catalina: stop() activate Catalina Catalina -> Server: stop() Catalina -> Server: destroy() Catalina <<-- Server: deactivate Server deactivate Catalina Bootstrap <<-- Catalina: deactivate Catalina deactivate Bootstrap @enduml and for mermaidjs it is --- title: Some sequenceDiagram config: messageAlign: left mirrorActors: false --- sequenceDiagram activate Bootstrap Bootstrap->>Bootstrap: initClassLoader() create participant Catalina Bootstrap--)Catalina: newInstance() Bootstrap->>+Catalina: setParentClassLoader() deactivate Catalina Bootstrap->>+Catalina: start() Catalina->>+Catalina: load() Catalina->>Catalina: initNaming() Catalina->>+Catalina: parseServerXml() create participant Digester Catalina--)Digester: note right of Digester: The digester creates allof the objects definedin server.xml but only theserver is shown here forsimplicity Catalina->>+Digester: parse() create participant Server Digester--)Server: deactivate Digester
Re: [QUESTION] Purchase UML tool using Google security funding
On 30/08/2024 08:01, Felix Schumacher wrote: Am 29.08.24 um 18:29 schrieb Mark Thomas: On 29/08/2024 15:34, Felix Schumacher wrote: While I don't object to buying a license, I would love to know, which diagram you looked at and what exactly did not work out. (the activation stuff in mermaid is brittle, but I think I managed to get them all right) I couldn't find a way to get the gap in the activation of Catalina between the call to setParentClassLoader() and start(). I see how you fixed that. Nice. There are a couple of places where the message arrows don't quite meet up with the activation bar correctly and the await note isn't quite in the right place. I only see one gap at the message await() from Catalina to itself. The note is a bit better placed, when we use "note right" (or even "note left") instead of "note right of Catalina". To get even more of the original look, we can use UML style, by adding "skinparam style strictuml" and to mimic the original drawing even more, we can switch off the actors at the bottom, by specifying "hide footbox". (full code at the end) You have convinced me. I did a little more playing around and if I add explicit return messages it fixes the incomplete message line and the activation bars look better. I can even put the await note exactly where I want it :) I think we switch to PlantUML and keep Visual Paradigm as a fallback option. Thanks for all your input on this. It has been really helpful. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch main updated: Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new f1789a8827 Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap f1789a8827 is described below commit f1789a8827a77a7b223cd1781805a76cdb23a120 Author: Mark Thomas AuthorDate: Fri Aug 30 11:27:29 2024 +0100 Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap Based on sample code and test cases provided by John Engebretson --- .../catalina/core/ApplicationHttpRequest.java | 3 +- java/org/apache/catalina/util/ParameterMap.java| 13 +++ .../catalina/util/TestParameterMapPerformance.java | 102 + 3 files changed, 116 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java b/java/org/apache/catalina/core/ApplicationHttpRequest.java index 3438a4bc0e..3b512c30db 100644 --- a/java/org/apache/catalina/core/ApplicationHttpRequest.java +++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java @@ -713,8 +713,7 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { return; } -parameters = new ParameterMap<>(); -parameters.putAll(getRequest().getParameterMap()); +parameters = new ParameterMap<>(getRequest().getParameterMap()); mergeParameters(); ((ParameterMap) parameters).setLocked(true); parsedParams = true; diff --git a/java/org/apache/catalina/util/ParameterMap.java b/java/org/apache/catalina/util/ParameterMap.java index 208893ab2d..38515e3460 100644 --- a/java/org/apache/catalina/util/ParameterMap.java +++ b/java/org/apache/catalina/util/ParameterMap.java @@ -87,6 +87,19 @@ public final class ParameterMap implements Map, Serializable { } +/** + * Optimised constructor for ParameterMap. + * + * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=69285"; + * + * @param map Map whose contents are duplicated in the new map + */ +public ParameterMap(ParameterMap map) { +delegatedMap = new LinkedHashMap<>(map.delegatedMap); +unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap); +} + + /** * The current lock state of this parameter map. */ diff --git a/test/org/apache/catalina/util/TestParameterMapPerformance.java b/test/org/apache/catalina/util/TestParameterMapPerformance.java new file mode 100644 index 00..a9bf7e9f34 --- /dev/null +++ b/test/org/apache/catalina/util/TestParameterMapPerformance.java @@ -0,0 +1,102 @@ +/* + * 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.catalina.util; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; + +public class TestParameterMapPerformance { + +private final int numTests = 5; +private final int numTestIterations = 100; + +private ParameterMap baseParams; +private ParameterMap testParams; + +@Test +public void testCompareStandardAndOptimizedMapConstructor() { +Map values = new HashMap<>(); +for (int i = 0; i < 50; i++) { +Integer integer = Integer.valueOf(i); +values.put(integer, integer); +} + +baseParams = new ParameterMap<>(values); +baseParams.setLocked(true); + +// warmup +for (int i = 0; i < numTestIterations; i++) { +useStandardConstructor(); +useOptimizedMapConstructor(); +} + +List standardDurations = new ArrayList<>(); +for (int i = 0; i < numTests; i++) { +System.gc(); +long startStandard = System.currentTimeMillis(); +for (int j = 0; j < numTestIterations; j++) { +useStandardConstructor(); +} +long duration = System.currentTimeMillis() - startStandard; +standardDurations.add(Long.valueOf(duration)); +System.out.println("Done with standard in " + duration + "ms"); +} +/* +
(tomcat) branch 11.0.x updated: Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/11.0.x by this push: new 92ec31c806 Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap 92ec31c806 is described below commit 92ec31c806d15cd5509c62152e9b830396d5cbfa Author: Mark Thomas AuthorDate: Fri Aug 30 11:27:29 2024 +0100 Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap Based on sample code and test cases provided by John Engebretson --- .../catalina/core/ApplicationHttpRequest.java | 3 +- java/org/apache/catalina/util/ParameterMap.java| 13 +++ .../catalina/util/TestParameterMapPerformance.java | 102 + webapps/docs/changelog.xml | 5 + 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java b/java/org/apache/catalina/core/ApplicationHttpRequest.java index 3438a4bc0e..3b512c30db 100644 --- a/java/org/apache/catalina/core/ApplicationHttpRequest.java +++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java @@ -713,8 +713,7 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { return; } -parameters = new ParameterMap<>(); -parameters.putAll(getRequest().getParameterMap()); +parameters = new ParameterMap<>(getRequest().getParameterMap()); mergeParameters(); ((ParameterMap) parameters).setLocked(true); parsedParams = true; diff --git a/java/org/apache/catalina/util/ParameterMap.java b/java/org/apache/catalina/util/ParameterMap.java index 208893ab2d..38515e3460 100644 --- a/java/org/apache/catalina/util/ParameterMap.java +++ b/java/org/apache/catalina/util/ParameterMap.java @@ -87,6 +87,19 @@ public final class ParameterMap implements Map, Serializable { } +/** + * Optimised constructor for ParameterMap. + * + * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=69285"; + * + * @param map Map whose contents are duplicated in the new map + */ +public ParameterMap(ParameterMap map) { +delegatedMap = new LinkedHashMap<>(map.delegatedMap); +unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap); +} + + /** * The current lock state of this parameter map. */ diff --git a/test/org/apache/catalina/util/TestParameterMapPerformance.java b/test/org/apache/catalina/util/TestParameterMapPerformance.java new file mode 100644 index 00..a9bf7e9f34 --- /dev/null +++ b/test/org/apache/catalina/util/TestParameterMapPerformance.java @@ -0,0 +1,102 @@ +/* + * 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.catalina.util; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; + +public class TestParameterMapPerformance { + +private final int numTests = 5; +private final int numTestIterations = 100; + +private ParameterMap baseParams; +private ParameterMap testParams; + +@Test +public void testCompareStandardAndOptimizedMapConstructor() { +Map values = new HashMap<>(); +for (int i = 0; i < 50; i++) { +Integer integer = Integer.valueOf(i); +values.put(integer, integer); +} + +baseParams = new ParameterMap<>(values); +baseParams.setLocked(true); + +// warmup +for (int i = 0; i < numTestIterations; i++) { +useStandardConstructor(); +useOptimizedMapConstructor(); +} + +List standardDurations = new ArrayList<>(); +for (int i = 0; i < numTests; i++) { +System.gc(); +long startStandard = System.currentTimeMillis(); +for (int j = 0; j < numTestIterations; j++) { +useStandardConstructor(); +} +long duration = System.currentTimeMillis() - startStandard; +standardDurations.add(Long.valueOf(duration)); +System.out.println("Done
(tomcat) branch 10.1.x updated: Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/10.1.x by this push: new 7483c63b8e Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap 7483c63b8e is described below commit 7483c63b8ebaa865c3e1462e696e7e8363d77609 Author: Mark Thomas AuthorDate: Fri Aug 30 11:27:29 2024 +0100 Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap Based on sample code and test cases provided by John Engebretson --- .../catalina/core/ApplicationHttpRequest.java | 3 +- java/org/apache/catalina/util/ParameterMap.java| 13 +++ .../catalina/util/TestParameterMapPerformance.java | 102 + webapps/docs/changelog.xml | 9 ++ 4 files changed, 125 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java b/java/org/apache/catalina/core/ApplicationHttpRequest.java index 12534bada5..a438ad7239 100644 --- a/java/org/apache/catalina/core/ApplicationHttpRequest.java +++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java @@ -710,8 +710,7 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { return; } -parameters = new ParameterMap<>(); -parameters.putAll(getRequest().getParameterMap()); +parameters = new ParameterMap<>(getRequest().getParameterMap()); mergeParameters(); ((ParameterMap) parameters).setLocked(true); parsedParams = true; diff --git a/java/org/apache/catalina/util/ParameterMap.java b/java/org/apache/catalina/util/ParameterMap.java index 208893ab2d..38515e3460 100644 --- a/java/org/apache/catalina/util/ParameterMap.java +++ b/java/org/apache/catalina/util/ParameterMap.java @@ -87,6 +87,19 @@ public final class ParameterMap implements Map, Serializable { } +/** + * Optimised constructor for ParameterMap. + * + * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=69285"; + * + * @param map Map whose contents are duplicated in the new map + */ +public ParameterMap(ParameterMap map) { +delegatedMap = new LinkedHashMap<>(map.delegatedMap); +unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap); +} + + /** * The current lock state of this parameter map. */ diff --git a/test/org/apache/catalina/util/TestParameterMapPerformance.java b/test/org/apache/catalina/util/TestParameterMapPerformance.java new file mode 100644 index 00..a9bf7e9f34 --- /dev/null +++ b/test/org/apache/catalina/util/TestParameterMapPerformance.java @@ -0,0 +1,102 @@ +/* + * 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.catalina.util; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; + +public class TestParameterMapPerformance { + +private final int numTests = 5; +private final int numTestIterations = 100; + +private ParameterMap baseParams; +private ParameterMap testParams; + +@Test +public void testCompareStandardAndOptimizedMapConstructor() { +Map values = new HashMap<>(); +for (int i = 0; i < 50; i++) { +Integer integer = Integer.valueOf(i); +values.put(integer, integer); +} + +baseParams = new ParameterMap<>(values); +baseParams.setLocked(true); + +// warmup +for (int i = 0; i < numTestIterations; i++) { +useStandardConstructor(); +useOptimizedMapConstructor(); +} + +List standardDurations = new ArrayList<>(); +for (int i = 0; i < numTests; i++) { +System.gc(); +long startStandard = System.currentTimeMillis(); +for (int j = 0; j < numTestIterations; j++) { +useStandardConstructor(); +} +long duration = System.currentTimeMillis() - startStandard; +standardDurations.add(Long.valueOf(duration)); +System.out.println("Done
(tomcat) branch 9.0.x updated: Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 1b46c9908f Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap 1b46c9908f is described below commit 1b46c9908fa472d7fac327f48ac52e0fc82cc965 Author: Mark Thomas AuthorDate: Fri Aug 30 11:27:29 2024 +0100 Fix BZ 69285. Optimize creation of ParameterMap from ParameterMap Based on sample code and test cases provided by John Engebretson --- .../catalina/core/ApplicationHttpRequest.java | 3 +- java/org/apache/catalina/util/ParameterMap.java| 13 +++ .../catalina/util/TestParameterMapPerformance.java | 102 + webapps/docs/changelog.xml | 9 ++ 4 files changed, 125 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java b/java/org/apache/catalina/core/ApplicationHttpRequest.java index af2bd80551..df29788973 100644 --- a/java/org/apache/catalina/core/ApplicationHttpRequest.java +++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java @@ -710,8 +710,7 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { return; } -parameters = new ParameterMap<>(); -parameters.putAll(getRequest().getParameterMap()); +parameters = new ParameterMap<>(getRequest().getParameterMap()); mergeParameters(); ((ParameterMap) parameters).setLocked(true); parsedParams = true; diff --git a/java/org/apache/catalina/util/ParameterMap.java b/java/org/apache/catalina/util/ParameterMap.java index 208893ab2d..38515e3460 100644 --- a/java/org/apache/catalina/util/ParameterMap.java +++ b/java/org/apache/catalina/util/ParameterMap.java @@ -87,6 +87,19 @@ public final class ParameterMap implements Map, Serializable { } +/** + * Optimised constructor for ParameterMap. + * + * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=69285"; + * + * @param map Map whose contents are duplicated in the new map + */ +public ParameterMap(ParameterMap map) { +delegatedMap = new LinkedHashMap<>(map.delegatedMap); +unmodifiableDelegatedMap = Collections.unmodifiableMap(delegatedMap); +} + + /** * The current lock state of this parameter map. */ diff --git a/test/org/apache/catalina/util/TestParameterMapPerformance.java b/test/org/apache/catalina/util/TestParameterMapPerformance.java new file mode 100644 index 00..a9bf7e9f34 --- /dev/null +++ b/test/org/apache/catalina/util/TestParameterMapPerformance.java @@ -0,0 +1,102 @@ +/* + * 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.catalina.util; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; + +public class TestParameterMapPerformance { + +private final int numTests = 5; +private final int numTestIterations = 100; + +private ParameterMap baseParams; +private ParameterMap testParams; + +@Test +public void testCompareStandardAndOptimizedMapConstructor() { +Map values = new HashMap<>(); +for (int i = 0; i < 50; i++) { +Integer integer = Integer.valueOf(i); +values.put(integer, integer); +} + +baseParams = new ParameterMap<>(values); +baseParams.setLocked(true); + +// warmup +for (int i = 0; i < numTestIterations; i++) { +useStandardConstructor(); +useOptimizedMapConstructor(); +} + +List standardDurations = new ArrayList<>(); +for (int i = 0; i < numTests; i++) { +System.gc(); +long startStandard = System.currentTimeMillis(); +for (int j = 0; j < numTestIterations; j++) { +useStandardConstructor(); +} +long duration = System.currentTimeMillis() - startStandard; +standardDurations.add(Long.valueOf(duration)); +System.out.println("Done w
[Bug 69285] Performance improvement to ApplicationHttpRequest.parseParameters()
https://bz.apache.org/bugzilla/show_bug.cgi?id=69285 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Mark Thomas --- Thanks for the report, analysis and the test case. As always, much appreciated. Fixed in: - 11.0.x for 11.0.0-M25 onwards - 10.1.x for 10.1.29 onwards - 9.0.x for 9.0.94 onwards -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 69285] Performance improvement to ApplicationHttpRequest.parseParameters()
https://bz.apache.org/bugzilla/show_bug.cgi?id=69285 --- Comment #4 from John Engebretson --- Thank you! :) -- 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 69068] ReadListener#onError not called on read exception during async processing
https://bz.apache.org/bugzilla/show_bug.cgi?id=69068 --- Comment #4 from hypnoce --- Created attachment 39857 --> https://bz.apache.org/bugzilla/attachment.cgi?id=39857&action=edit Alternative fix The proposed solution is dependent on the stream timeout. For long running/streaming use cases, no error is reported. I'm not sure about the proposed patch, but it seems to cover use cases where stream timeout is big. Let me know what you think. -- 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 main updated: Fix Tomcat not sending close_notify with OpenSSLImplementation
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 83e9dfd84d Fix Tomcat not sending close_notify with OpenSSLImplementation 83e9dfd84d is described below commit 83e9dfd84dfba839ec1d728b2b74e657f180c301 Author: Mark Thomas AuthorDate: Fri Aug 30 15:21:08 2024 +0100 Fix Tomcat not sending close_notify with OpenSSLImplementation --- .../apache/tomcat/util/net/openssl/OpenSSLEngine.java | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 0eb7beb55d..914eafb901 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -465,8 +465,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // If isOutboundDone is set, then the data from the network BIO // was the close_notify message -- we are not required to wait -// for the receipt the peer's close_notify message -- shutdown. -if (isOutboundDone) { +// for the receipt of the peer's close_notify message -- shutdown. +if (isOutboundDone()) { shutdown(); } @@ -637,7 +637,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check to see if we received a close_notify message from the peer if (!receivedShutdown && (SSL.getShutdown(state.ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) { receivedShutdown = true; -closeOutbound(); closeInbound(); } if (bytesProduced == 0 && (written == 0 || (written > 0 && !src.hasRemaining() && handshakeFinished))) { @@ -692,7 +691,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn isInboundDone = true; engineClosed = true; -shutdown(); +if (isOutboundDone()) { +// Only call shutdown if there is no outbound data pending. +shutdown(); +} if (accepted != Accepted.NOT && !receivedShutdown) { throw new SSLException(sm.getString("engine.inboundClose")); @@ -1073,13 +1075,15 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check if we are in the shutdown phase if (engineClosed) { -// Waiting to send the close_notify message if (SSL.pendingWrittenBytesInBIO(state.networkBIO) != 0) { +// Waiting to send the close_notify message return SSLEngineResult.HandshakeStatus.NEED_WRAP; } -// Must be waiting to receive the close_notify message -return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +if (!isInboundDone()) { +// Must be waiting to receive the close_notify message +return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +} } return SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING; - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 69301] New: wrong headers in access log http2 with trailers
https://bz.apache.org/bugzilla/show_bug.cgi?id=69301 Bug ID: 69301 Summary: wrong headers in access log http2 with trailers Product: Tomcat 9 Version: 9.0.x Hardware: Macintosh OS: Mac OS X 10.1 Status: NEW Severity: normal Priority: P2 Component: Catalina Assignee: dev@tomcat.apache.org Reporter: hypn...@gmail.com Target Milestone: - Created attachment 39858 --> https://bz.apache.org/bugzilla/attachment.cgi?id=39858&action=edit Unit test and fix Hi, when logging an http2 request with trailers, headers get cleaned and replaced by the trailers, resulting in wrong access logs. Attached the patch with test case and potential fix. Let me know what you think. Thanks -- 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 main updated: Remove unnecessary code (shutdown() sets these values)
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 2e5cb6951c Remove unnecessary code (shutdown() sets these values) 2e5cb6951c is described below commit 2e5cb6951cea14ec0773e83be36669a24915074c Author: Mark Thomas AuthorDate: Fri Aug 30 15:22:16 2024 +0100 Remove unnecessary code (shutdown() sets these values) --- java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 914eafb901..99bd981944 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -559,9 +559,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // protect against protocol overflow attack vector if (len > MAX_ENCRYPTED_PACKET_LENGTH) { -isInboundDone = true; -isOutboundDone = true; -engineClosed = true; shutdown(); throw new SSLException(sm.getString("engine.oversizedPacket")); } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 11.0.x updated: Fix Tomcat not sending close_notify with OpenSSLImplementation
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/11.0.x by this push: new f97e3a24ec Fix Tomcat not sending close_notify with OpenSSLImplementation f97e3a24ec is described below commit f97e3a24ecccddcb2b3018fb6626854ac13e70f5 Author: Mark Thomas AuthorDate: Fri Aug 30 15:21:08 2024 +0100 Fix Tomcat not sending close_notify with OpenSSLImplementation --- .../apache/tomcat/util/net/openssl/OpenSSLEngine.java | 18 +++--- webapps/docs/changelog.xml | 5 + 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 0eb7beb55d..914eafb901 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -465,8 +465,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // If isOutboundDone is set, then the data from the network BIO // was the close_notify message -- we are not required to wait -// for the receipt the peer's close_notify message -- shutdown. -if (isOutboundDone) { +// for the receipt of the peer's close_notify message -- shutdown. +if (isOutboundDone()) { shutdown(); } @@ -637,7 +637,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check to see if we received a close_notify message from the peer if (!receivedShutdown && (SSL.getShutdown(state.ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) { receivedShutdown = true; -closeOutbound(); closeInbound(); } if (bytesProduced == 0 && (written == 0 || (written > 0 && !src.hasRemaining() && handshakeFinished))) { @@ -692,7 +691,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn isInboundDone = true; engineClosed = true; -shutdown(); +if (isOutboundDone()) { +// Only call shutdown if there is no outbound data pending. +shutdown(); +} if (accepted != Accepted.NOT && !receivedShutdown) { throw new SSLException(sm.getString("engine.inboundClose")); @@ -1073,13 +1075,15 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check if we are in the shutdown phase if (engineClosed) { -// Waiting to send the close_notify message if (SSL.pendingWrittenBytesInBIO(state.networkBIO) != 0) { +// Waiting to send the close_notify message return SSLEngineResult.HandshakeStatus.NEED_WRAP; } -// Must be waiting to receive the close_notify message -return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +if (!isInboundDone()) { +// Must be waiting to receive the close_notify message +return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +} } return SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index d2366f1bed..798065a138 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -134,6 +134,11 @@ Rfc6265CookieProcessor. The default behaviour is unchanged. (markt) + +Ensure that Tomcat sends a TLS close_notify message after receiving one +from the client when using the OpenSSLImplementation. +(markt) + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 10.1.x updated: Fix Tomcat not sending close_notify with OpenSSLImplementation
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/10.1.x by this push: new 34b170f4db Fix Tomcat not sending close_notify with OpenSSLImplementation 34b170f4db is described below commit 34b170f4dbc390f87336d129bdc499034919a672 Author: Mark Thomas AuthorDate: Fri Aug 30 15:21:08 2024 +0100 Fix Tomcat not sending close_notify with OpenSSLImplementation --- .../apache/tomcat/util/net/openssl/OpenSSLEngine.java | 18 +++--- webapps/docs/changelog.xml | 5 + 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 17b5e7a3ac..4be8027aa4 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -467,8 +467,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // If isOutboundDone is set, then the data from the network BIO // was the close_notify message -- we are not required to wait -// for the receipt the peer's close_notify message -- shutdown. -if (isOutboundDone) { +// for the receipt of the peer's close_notify message -- shutdown. +if (isOutboundDone()) { shutdown(); } @@ -639,7 +639,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check to see if we received a close_notify message from the peer if (!receivedShutdown && (SSL.getShutdown(state.ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) { receivedShutdown = true; -closeOutbound(); closeInbound(); } if (bytesProduced == 0 && (written == 0 || (written > 0 && !src.hasRemaining() && handshakeFinished))) { @@ -694,7 +693,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn isInboundDone = true; engineClosed = true; -shutdown(); +if (isOutboundDone()) { +// Only call shutdown if there is no outbound data pending. +shutdown(); +} if (accepted != Accepted.NOT && !receivedShutdown) { throw new SSLException(sm.getString("engine.inboundClose")); @@ -1076,13 +1078,15 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check if we are in the shutdown phase if (engineClosed) { -// Waiting to send the close_notify message if (SSL.pendingWrittenBytesInBIO(state.networkBIO) != 0) { +// Waiting to send the close_notify message return SSLEngineResult.HandshakeStatus.NEED_WRAP; } -// Must be waiting to receive the close_notify message -return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +if (!isInboundDone()) { +// Must be waiting to receive the close_notify message +return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +} } return SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index b3de77ad9c..b1b809fcbe 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -128,6 +128,11 @@ Rfc6265CookieProcessor. The default behaviour is unchanged. (markt) + +Ensure that Tomcat sends a TLS close_notify message after receiving one +from the client when using the OpenSSLImplementation. +(markt) + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 9.0.x updated: Fix Tomcat not sending close_notify with OpenSSLImplementation
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 8937b3d68d Fix Tomcat not sending close_notify with OpenSSLImplementation 8937b3d68d is described below commit 8937b3d68d2cc2aa5c5d0745002d838ee87856f8 Author: Mark Thomas AuthorDate: Fri Aug 30 15:21:08 2024 +0100 Fix Tomcat not sending close_notify with OpenSSLImplementation --- .../apache/tomcat/util/net/openssl/OpenSSLEngine.java | 18 +++--- webapps/docs/changelog.xml | 5 + 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index e3eb30c13b..1cfbc90d9e 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -467,8 +467,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // If isOutboundDone is set, then the data from the network BIO // was the close_notify message -- we are not required to wait -// for the receipt the peer's close_notify message -- shutdown. -if (isOutboundDone) { +// for the receipt of the peer's close_notify message -- shutdown. +if (isOutboundDone()) { shutdown(); } @@ -639,7 +639,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check to see if we received a close_notify message from the peer if (!receivedShutdown && (SSL.getShutdown(ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) { receivedShutdown = true; -closeOutbound(); closeInbound(); } if (bytesProduced == 0 && (written == 0 || (written > 0 && !src.hasRemaining() && handshakeFinished))) { @@ -694,7 +693,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn isInboundDone = true; engineClosed = true; -shutdown(); +if (isOutboundDone()) { +// Only call shutdown if there is no outbound data pending. +shutdown(); +} if (accepted != Accepted.NOT && !receivedShutdown) { throw new SSLException(sm.getString("engine.inboundClose")); @@ -1076,13 +1078,15 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // Check if we are in the shutdown phase if (engineClosed) { -// Waiting to send the close_notify message if (SSL.pendingWrittenBytesInBIO(networkBIO) != 0) { +// Waiting to send the close_notify message return SSLEngineResult.HandshakeStatus.NEED_WRAP; } -// Must be waiting to receive the close_notify message -return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +if (!isInboundDone()) { +// Must be waiting to receive the close_notify message +return SSLEngineResult.HandshakeStatus.NEED_UNWRAP; +} } return SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 5214c7d8db..544c972460 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -128,6 +128,11 @@ Rfc6265CookieProcessor. The default behaviour is unchanged. (markt) + +Ensure that Tomcat sends a TLS close_notify message after receiving one +from the client when using the OpenSSLImplementation. +(markt) + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 10.1.x updated: Remove unnecessary code (shutdown() sets these values)
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/10.1.x by this push: new 8a38b16258 Remove unnecessary code (shutdown() sets these values) 8a38b16258 is described below commit 8a38b1625889b2f4f8c76341943d9f404c5ee80f Author: Mark Thomas AuthorDate: Fri Aug 30 15:22:16 2024 +0100 Remove unnecessary code (shutdown() sets these values) --- java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 4be8027aa4..6b5e5f5288 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -561,9 +561,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // protect against protocol overflow attack vector if (len > MAX_ENCRYPTED_PACKET_LENGTH) { -isInboundDone = true; -isOutboundDone = true; -engineClosed = true; shutdown(); throw new SSLException(sm.getString("engine.oversizedPacket")); } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 11.0.x updated: Remove unnecessary code (shutdown() sets these values)
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/11.0.x by this push: new c8bf84f155 Remove unnecessary code (shutdown() sets these values) c8bf84f155 is described below commit c8bf84f155eb2946cde7a14de04f196e68927294 Author: Mark Thomas AuthorDate: Fri Aug 30 15:22:16 2024 +0100 Remove unnecessary code (shutdown() sets these values) --- java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 914eafb901..99bd981944 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -559,9 +559,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // protect against protocol overflow attack vector if (len > MAX_ENCRYPTED_PACKET_LENGTH) { -isInboundDone = true; -isOutboundDone = true; -engineClosed = true; shutdown(); throw new SSLException(sm.getString("engine.oversizedPacket")); } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 9.0.x updated: Remove unnecessary code (shutdown() sets these values)
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 3bf4ab6f74 Remove unnecessary code (shutdown() sets these values) 3bf4ab6f74 is described below commit 3bf4ab6f74b935bb17efce864d744b464bcab9a9 Author: Mark Thomas AuthorDate: Fri Aug 30 15:22:16 2024 +0100 Remove unnecessary code (shutdown() sets these values) --- java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java index 1cfbc90d9e..3a2a16d31c 100644 --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java @@ -561,9 +561,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn // protect against protocol overflow attack vector if (len > MAX_ENCRYPTED_PACKET_LENGTH) { -isInboundDone = true; -isOutboundDone = true; -engineClosed = true; shutdown(); throw new SSLException(sm.getString("engine.oversizedPacket")); } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 69068] ReadListener#onError not called on read exception during async processing
https://bz.apache.org/bugzilla/show_bug.cgi?id=69068 --- Comment #5 from Mark Thomas --- Please open a new issue for this. If you were able to modify/extend the existing test case to demonstrate the issue that would be very helpful. I'm not sure about the proposed patch. I'm wondering if this should also be handled by Stream.receiveReset() -- 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 69302] New: ReadListener#onError not called on read exception during async processing with read timeout
https://bz.apache.org/bugzilla/show_bug.cgi?id=69302 Bug ID: 69302 Summary: ReadListener#onError not called on read exception during async processing with read timeout Product: Tomcat 9 Version: 9.0.x Hardware: Macintosh OS: Mac OS X 10.1 Status: NEW Severity: normal Priority: P2 Component: Catalina Assignee: dev@tomcat.apache.org Reporter: hypn...@gmail.com Target Milestone: - Created attachment 39859 --> https://bz.apache.org/bugzilla/attachment.cgi?id=39859&action=edit Use case and potential fix Hi, https://bz.apache.org/bugzilla/show_bug.cgi?id=69068 throws a SocketTimeoutException after stream timeout is reached. When this timeout is big (streaming gRPC requests for example), when a client closes the connection, no error/completion is reported at the level of the servlet listeners. Let me know what you think. Thanks François -- 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 69301] wrong headers in access log http2 with trailers
https://bz.apache.org/bugzilla/show_bug.cgi?id=69301 --- Comment #1 from Mark Thomas --- Thanks for the report and the test case. It makes it so much easier for us to work with and is very much appreciated. It is taking a little longer than expected to look at this as there is a timing issue. I think I have tracked it down to the test case (I thought initially it was in Tomcat's header handling). I need to write and test a fix. Hopefully later today. -- 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 main updated: Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new af58939689 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers af58939689 is described below commit af589396891d658a2b9b69ce1fc496880b0dcfe3 Author: Mark Thomas AuthorDate: Fri Aug 30 19:26:31 2024 +0100 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers The non-trailer headers were not visible to the access log. They were replaced by the trailer headers. --- java/org/apache/coyote/http2/Stream.java | 9 +- .../apache/coyote/http2/TestHttp2AccessLogs.java | 172 + 2 files changed, 177 insertions(+), 4 deletions(-) diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index 65a130892b..3db615827b 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -570,10 +570,11 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { return; } -// We can re-use the MimeHeaders from the response since they have -// already been processed by the encoder at this point -MimeHeaders mimeHeaders = coyoteResponse.getMimeHeaders(); -mimeHeaders.recycle(); +/* + * Need a dedicated MimeHeaders for trailers as the MimeHeaders from the response needs to be retained in case + * the access log needs to log header values. + */ +MimeHeaders mimeHeaders = new MimeHeaders(); Map headerMap = supplier.get(); if (headerMap == null) { diff --git a/test/org/apache/coyote/http2/TestHttp2AccessLogs.java b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java new file mode 100644 index 00..ae9a9b5ace --- /dev/null +++ b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java @@ -0,0 +1,172 @@ +/* + * 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.coyote.http2; + +import java.io.CharArrayWriter; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.Wrapper; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.valves.AbstractAccessLogValve; + +public class TestHttp2AccessLogs extends Http2TestBase { + +private static final int COUNT_LIMIT = 20; +private static final long SLEEP = 50; + +private void configureAndStartWebApplication(CharArrayWriter writer) throws LifecycleException { +Tomcat tomcat = getTomcatInstance(); + +Context ctxt = getProgrammaticRootContext(); +Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); +ctxt.addServletMappingDecoded("/simple", "simple"); +Wrapper w = Tomcat.addServlet(ctxt, "trailers", new TrailersServlet()); +w.setAsyncSupported(true); +ctxt.addServletMappingDecoded("/trailers", "trailers"); +TesterAccessLogValve valve = new TesterAccessLogValve(writer); +valve.setPattern("%{header-key}o %{trailer-key}o"); +tomcat.getHost().getPipeline().addValve(valve); +tomcat.start(); +} + + +@Test +public void testHeadersAndTrailersInAccessLog() throws Exception { +CharArrayWriter writer = new CharArrayWriter(); + +enableHttp2(false); +configureAndStartWebApplication(writer); +openClientConnection(false); +doHttpUpgrade(); +sendClientPreface(); +validateHttp2InitialResponse(); + +// Validate the access log is as expected after the initial requests +int count = 0; +String result = writer.toString(); +String expectedInitial = "- -- -"; +while (count < COUNT_LIMIT && !expectedInit
(tomcat) branch 11.0.x updated: Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/11.0.x by this push: new c0df448d0d Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers c0df448d0d is described below commit c0df448d0de13aa267c5dc7dd279c8e7ef4115ed Author: Mark Thomas AuthorDate: Fri Aug 30 19:26:31 2024 +0100 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers The non-trailer headers were not visible to the access log. They were replaced by the trailer headers. --- java/org/apache/coyote/http2/Stream.java | 9 +- .../apache/coyote/http2/TestHttp2AccessLogs.java | 172 + webapps/docs/changelog.xml | 5 + 3 files changed, 182 insertions(+), 4 deletions(-) diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index 65a130892b..3db615827b 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -570,10 +570,11 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { return; } -// We can re-use the MimeHeaders from the response since they have -// already been processed by the encoder at this point -MimeHeaders mimeHeaders = coyoteResponse.getMimeHeaders(); -mimeHeaders.recycle(); +/* + * Need a dedicated MimeHeaders for trailers as the MimeHeaders from the response needs to be retained in case + * the access log needs to log header values. + */ +MimeHeaders mimeHeaders = new MimeHeaders(); Map headerMap = supplier.get(); if (headerMap == null) { diff --git a/test/org/apache/coyote/http2/TestHttp2AccessLogs.java b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java new file mode 100644 index 00..ae9a9b5ace --- /dev/null +++ b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java @@ -0,0 +1,172 @@ +/* + * 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.coyote.http2; + +import java.io.CharArrayWriter; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.Wrapper; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.valves.AbstractAccessLogValve; + +public class TestHttp2AccessLogs extends Http2TestBase { + +private static final int COUNT_LIMIT = 20; +private static final long SLEEP = 50; + +private void configureAndStartWebApplication(CharArrayWriter writer) throws LifecycleException { +Tomcat tomcat = getTomcatInstance(); + +Context ctxt = getProgrammaticRootContext(); +Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); +ctxt.addServletMappingDecoded("/simple", "simple"); +Wrapper w = Tomcat.addServlet(ctxt, "trailers", new TrailersServlet()); +w.setAsyncSupported(true); +ctxt.addServletMappingDecoded("/trailers", "trailers"); +TesterAccessLogValve valve = new TesterAccessLogValve(writer); +valve.setPattern("%{header-key}o %{trailer-key}o"); +tomcat.getHost().getPipeline().addValve(valve); +tomcat.start(); +} + + +@Test +public void testHeadersAndTrailersInAccessLog() throws Exception { +CharArrayWriter writer = new CharArrayWriter(); + +enableHttp2(false); +configureAndStartWebApplication(writer); +openClientConnection(false); +doHttpUpgrade(); +sendClientPreface(); +validateHttp2InitialResponse(); + +// Validate the access log is as expected after the initial requests +int count = 0; +String result = writer.toString(); +String expectedInitial
(tomcat) branch 10.1.x updated: Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/10.1.x by this push: new 4d08ae46c7 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers 4d08ae46c7 is described below commit 4d08ae46c74658c54d8231183f0d4b3ddf8b5212 Author: Mark Thomas AuthorDate: Fri Aug 30 19:26:31 2024 +0100 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers The non-trailer headers were not visible to the access log. They were replaced by the trailer headers. --- java/org/apache/coyote/http2/Stream.java | 9 +- .../apache/coyote/http2/TestHttp2AccessLogs.java | 172 + webapps/docs/changelog.xml | 5 + 3 files changed, 182 insertions(+), 4 deletions(-) diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index 9837dfd072..876b85158a 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -577,10 +577,11 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { return; } -// We can re-use the MimeHeaders from the response since they have -// already been processed by the encoder at this point -MimeHeaders mimeHeaders = coyoteResponse.getMimeHeaders(); -mimeHeaders.recycle(); +/* + * Need a dedicated MimeHeaders for trailers as the MimeHeaders from the response needs to be retained in case + * the access log needs to log header values. + */ +MimeHeaders mimeHeaders = new MimeHeaders(); Map headerMap = supplier.get(); if (headerMap == null) { diff --git a/test/org/apache/coyote/http2/TestHttp2AccessLogs.java b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java new file mode 100644 index 00..ae9a9b5ace --- /dev/null +++ b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java @@ -0,0 +1,172 @@ +/* + * 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.coyote.http2; + +import java.io.CharArrayWriter; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.Wrapper; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.valves.AbstractAccessLogValve; + +public class TestHttp2AccessLogs extends Http2TestBase { + +private static final int COUNT_LIMIT = 20; +private static final long SLEEP = 50; + +private void configureAndStartWebApplication(CharArrayWriter writer) throws LifecycleException { +Tomcat tomcat = getTomcatInstance(); + +Context ctxt = getProgrammaticRootContext(); +Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); +ctxt.addServletMappingDecoded("/simple", "simple"); +Wrapper w = Tomcat.addServlet(ctxt, "trailers", new TrailersServlet()); +w.setAsyncSupported(true); +ctxt.addServletMappingDecoded("/trailers", "trailers"); +TesterAccessLogValve valve = new TesterAccessLogValve(writer); +valve.setPattern("%{header-key}o %{trailer-key}o"); +tomcat.getHost().getPipeline().addValve(valve); +tomcat.start(); +} + + +@Test +public void testHeadersAndTrailersInAccessLog() throws Exception { +CharArrayWriter writer = new CharArrayWriter(); + +enableHttp2(false); +configureAndStartWebApplication(writer); +openClientConnection(false); +doHttpUpgrade(); +sendClientPreface(); +validateHttp2InitialResponse(); + +// Validate the access log is as expected after the initial requests +int count = 0; +String result = writer.toString(); +String expectedInitial
(tomcat) branch 9.0.x updated: Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers
This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 577d3e14e1 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers 577d3e14e1 is described below commit 577d3e14e14ab7d41b3b41b54c3bb211b327b731 Author: Mark Thomas AuthorDate: Fri Aug 30 19:26:31 2024 +0100 Fix BZ 69301 - Wrong headers in access log with HTTP/2 + trailers The non-trailer headers were not visible to the access log. They were replaced by the trailer headers. --- java/org/apache/coyote/http2/Stream.java | 9 +- .../apache/coyote/http2/TestHttp2AccessLogs.java | 172 + webapps/docs/changelog.xml | 5 + 3 files changed, 182 insertions(+), 4 deletions(-) diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index 8ebc41997a..1b2f244f2d 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -575,10 +575,11 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { return; } -// We can re-use the MimeHeaders from the response since they have -// already been processed by the encoder at this point -MimeHeaders mimeHeaders = coyoteResponse.getMimeHeaders(); -mimeHeaders.recycle(); +/* + * Need a dedicated MimeHeaders for trailers as the MimeHeaders from the response needs to be retained in case + * the access log needs to log header values. + */ +MimeHeaders mimeHeaders = new MimeHeaders(); Map headerMap = supplier.get(); if (headerMap == null) { diff --git a/test/org/apache/coyote/http2/TestHttp2AccessLogs.java b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java new file mode 100644 index 00..ae9a9b5ace --- /dev/null +++ b/test/org/apache/coyote/http2/TestHttp2AccessLogs.java @@ -0,0 +1,172 @@ +/* + * 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.coyote.http2; + +import java.io.CharArrayWriter; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Context; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.Wrapper; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.valves.AbstractAccessLogValve; + +public class TestHttp2AccessLogs extends Http2TestBase { + +private static final int COUNT_LIMIT = 20; +private static final long SLEEP = 50; + +private void configureAndStartWebApplication(CharArrayWriter writer) throws LifecycleException { +Tomcat tomcat = getTomcatInstance(); + +Context ctxt = getProgrammaticRootContext(); +Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); +ctxt.addServletMappingDecoded("/simple", "simple"); +Wrapper w = Tomcat.addServlet(ctxt, "trailers", new TrailersServlet()); +w.setAsyncSupported(true); +ctxt.addServletMappingDecoded("/trailers", "trailers"); +TesterAccessLogValve valve = new TesterAccessLogValve(writer); +valve.setPattern("%{header-key}o %{trailer-key}o"); +tomcat.getHost().getPipeline().addValve(valve); +tomcat.start(); +} + + +@Test +public void testHeadersAndTrailersInAccessLog() throws Exception { +CharArrayWriter writer = new CharArrayWriter(); + +enableHttp2(false); +configureAndStartWebApplication(writer); +openClientConnection(false); +doHttpUpgrade(); +sendClientPreface(); +validateHttp2InitialResponse(); + +// Validate the access log is as expected after the initial requests +int count = 0; +String result = writer.toString(); +String expectedInitial =
[Bug 69301] wrong headers in access log http2 with trailers
https://bz.apache.org/bugzilla/show_bug.cgi?id=69301 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #2 from Mark Thomas --- Fixed in: - 11.0.x for 11.0.0-M25 onwards - 10.1.x for 10.1.29 onwards - 9.0.x for 9.0.94 onwards -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Buildbot failure in on tomcat-9.0.x
Build status: BUILD FAILED: compile (failure) Logs copied. (failure) Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/37/builds/1056 Blamelist: Mark Thomas Build Text: compile (failure) Logs copied. (failure) Status Detected: new failure Build Source Stamp: [branch 9.0.x] 577d3e14e14ab7d41b3b41b54c3bb211b327b731 Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 1 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 2 shell_11: 2 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org