[Bug 63875] Tomcat 8.5.46, APR/libtcnative crashes
https://bz.apache.org/bugzilla/show_bug.cgi?id=63875 --- Comment #4 from Michael Osipov --- (In reply to Mark Thomas from comment #3) > Both those crashes are during shutdown of the NIO2 connector when it is > using OpenSSL for TLS. > > It would be worth trying an upgrade to 8.5.47 and Tomcat Native 1.2.23 > although I don't see anything in the changelog that obviously matches this > issue. I wonder whether the APR connector would crash here too. -- 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: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new c41fe3b Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
c41fe3b is described below
commit c41fe3b64d0caed36cc3a76c872e272290473a4f
Author: Mark Thomas
AuthorDate: Wed Oct 23 08:50:11 2019 +0200
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
Improve the check of the Content-Encoding header when looking to see if
Tomcat is serving pre-compressed content. Ensure that only a full token
is matched and that the match is case insensitive.
---
java/org/apache/coyote/CompressionConfig.java | 28 ++
java/org/apache/coyote/LocalStrings.properties | 2 ++
java/org/apache/coyote/http11/Http11Processor.java | 18 ++
.../apache/tomcat/util/http/parser/TokenList.java | 24 ++-
webapps/docs/changelog.xml | 6 +
5 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/java/org/apache/coyote/CompressionConfig.java
b/java/org/apache/coyote/CompressionConfig.java
index fd0652e..520ed2e 100644
--- a/java/org/apache/coyote/CompressionConfig.java
+++ b/java/org/apache/coyote/CompressionConfig.java
@@ -20,17 +20,26 @@ import java.io.IOException;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.Enumeration;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.StringTokenizer;
import java.util.regex.Pattern;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.MimeHeaders;
import org.apache.tomcat.util.http.ResponseUtil;
import org.apache.tomcat.util.http.parser.AcceptEncoding;
+import org.apache.tomcat.util.http.parser.TokenList;
+import org.apache.tomcat.util.res.StringManager;
public class CompressionConfig {
+private static final Log log = LogFactory.getLog(CompressionConfig.class);
+private static final StringManager sm =
StringManager.getManager(CompressionConfig.class);
+
private int compressionLevel = 0;
private Pattern noCompressionUserAgents = null;
private String compressibleMimeType =
"text/html,text/xml,text/plain,text/css," +
@@ -193,10 +202,21 @@ public class CompressionConfig {
// Check if content is not already compressed
MessageBytes contentEncodingMB =
responseHeaders.getValue("Content-Encoding");
-if (contentEncodingMB != null &&
-(contentEncodingMB.indexOf("gzip") != -1 ||
-contentEncodingMB.indexOf("br") != -1)) {
-return false;
+if (contentEncodingMB != null) {
+// Content-Encoding values are ordered but order is not important
+// for this check so use a Set rather than a List
+Set tokens = new HashSet<>();
+try {
+
TokenList.parseTokenList(responseHeaders.values("Content-Encoding"), tokens);
+} catch (IOException e) {
+// Because we are using StringReader, any exception here is a
+// Tomcat bug.
+
log.warn(sm.getString("compressionConfig.ContentEncodingParseFail"), e);
+return false;
+}
+if (tokens.contains("gzip") || tokens.contains("br")) {
+return false;
+}
}
// If force mode, the length and MIME type checks are skipped
diff --git a/java/org/apache/coyote/LocalStrings.properties
b/java/org/apache/coyote/LocalStrings.properties
index b14fc2c..90159d3 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -46,6 +46,8 @@ abstractProtocolHandler.stop=Stopping ProtocolHandler [{0}]
asyncStateMachine.invalidAsyncState=Calling [{0}] is not valid for a request
with Async state [{1}]
+compressionConfig.ContentEncodingParseFail=Failed to parse Content-Encoding
header when chekcing to see if compression was already in use
+
request.notAsync=It is only valid to switch to non-blocking IO within async
processing or HTTP upgrade processing
request.nullReadListener=The listener passed to setReadListener() may not be
null
request.readListenerSet=The non-blocking read listener has already been set
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index 6df04cc..7be52d0 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -18,10 +18,7 @@ package org.apache.coyote.http11;
import java.io.IOException;
import java.io.InterruptedIOException;
-import java.io.StringReader;
import java.nio.ByteBuffer;
-import java.util.Collectio
[tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 9054e10 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
9054e10 is described below
commit 9054e10d53170afcd7dd85bd22335238625958dc
Author: Mark Thomas
AuthorDate: Wed Oct 23 08:50:11 2019 +0200
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
Improve the check of the Content-Encoding header when looking to see if
Tomcat is serving pre-compressed content. Ensure that only a full token
is matched and that the match is case insensitive.
---
java/org/apache/coyote/CompressionConfig.java | 28 ++
java/org/apache/coyote/LocalStrings.properties | 2 ++
java/org/apache/coyote/http11/Http11Processor.java | 18 ++
.../apache/tomcat/util/http/parser/TokenList.java | 24 ++-
webapps/docs/changelog.xml | 6 +
5 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/java/org/apache/coyote/CompressionConfig.java
b/java/org/apache/coyote/CompressionConfig.java
index fd0652e..520ed2e 100644
--- a/java/org/apache/coyote/CompressionConfig.java
+++ b/java/org/apache/coyote/CompressionConfig.java
@@ -20,17 +20,26 @@ import java.io.IOException;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.Enumeration;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.StringTokenizer;
import java.util.regex.Pattern;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.MimeHeaders;
import org.apache.tomcat.util.http.ResponseUtil;
import org.apache.tomcat.util.http.parser.AcceptEncoding;
+import org.apache.tomcat.util.http.parser.TokenList;
+import org.apache.tomcat.util.res.StringManager;
public class CompressionConfig {
+private static final Log log = LogFactory.getLog(CompressionConfig.class);
+private static final StringManager sm =
StringManager.getManager(CompressionConfig.class);
+
private int compressionLevel = 0;
private Pattern noCompressionUserAgents = null;
private String compressibleMimeType =
"text/html,text/xml,text/plain,text/css," +
@@ -193,10 +202,21 @@ public class CompressionConfig {
// Check if content is not already compressed
MessageBytes contentEncodingMB =
responseHeaders.getValue("Content-Encoding");
-if (contentEncodingMB != null &&
-(contentEncodingMB.indexOf("gzip") != -1 ||
-contentEncodingMB.indexOf("br") != -1)) {
-return false;
+if (contentEncodingMB != null) {
+// Content-Encoding values are ordered but order is not important
+// for this check so use a Set rather than a List
+Set tokens = new HashSet<>();
+try {
+
TokenList.parseTokenList(responseHeaders.values("Content-Encoding"), tokens);
+} catch (IOException e) {
+// Because we are using StringReader, any exception here is a
+// Tomcat bug.
+
log.warn(sm.getString("compressionConfig.ContentEncodingParseFail"), e);
+return false;
+}
+if (tokens.contains("gzip") || tokens.contains("br")) {
+return false;
+}
}
// If force mode, the length and MIME type checks are skipped
diff --git a/java/org/apache/coyote/LocalStrings.properties
b/java/org/apache/coyote/LocalStrings.properties
index 03dec37..3f0fbdf 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -48,6 +48,8 @@ abstractProtocolHandler.stopError=Failed to stop end point
associated with Proto
asyncStateMachine.invalidAsyncState=Calling [{0}] is not valid for a request
with Async state [{1}]
+compressionConfig.ContentEncodingParseFail=Failed to parse Content-Encoding
header when chekcing to see if compression was already in use
+
request.notAsync=It is only valid to switch to non-blocking IO within async
processing or HTTP upgrade processing
request.nullReadListener=The listener passed to setReadListener() may not be
null
request.readListenerSet=The non-blocking read listener has already been set
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index 05e595a..f6de5f1 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -18,10 +18,7 @@ package org.apache.coyote.http11;
import java.io.IOException;
import java.io.InterruptedIOException;
-import java.io.StringReader;
import java.nio.ByteBuffer;
-import
[tomcat] branch 7.0.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/7.0.x by this push:
new 0c756e4 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
0c756e4 is described below
commit 0c756e4252cd1d3669fdb693990bc8da9820a615
Author: Mark Thomas
AuthorDate: Wed Oct 23 11:30:54 2019 +0200
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
Improve the check of the Content-Encoding header when looking to see if
Tomcat is serving pre-compressed content. Ensure that only a full token
is matched and that the match is case insensitive.
---
.../coyote/http11/AbstractHttp11Processor.java | 35 +++---
.../apache/coyote/http11/LocalStrings.properties | 1 +
.../apache/tomcat/util/http/parser/TokenList.java | 24 ++-
webapps/docs/changelog.xml | 6
4 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Processor.java
b/java/org/apache/coyote/http11/AbstractHttp11Processor.java
index 37ecc9a..2064f93 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Processor.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Processor.java
@@ -19,7 +19,6 @@ package org.apache.coyote.http11;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.io.StringReader;
-import java.util.Collection;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.List;
@@ -611,10 +610,21 @@ public abstract class AbstractHttp11Processor extends
AbstractProcessor {
// Check if content is not already compressed
MessageBytes contentEncodingMB =
response.getMimeHeaders().getValue("Content-Encoding");
-if ((contentEncodingMB != null) &&
-(contentEncodingMB.indexOf("gzip") != -1 ||
-contentEncodingMB.indexOf("br") != -1)) {
-return false;
+if (contentEncodingMB != null) {
+// Content-Encoding values are ordered but order is not important
+// for this check so use a Set rather than a List
+Set tokens = new HashSet();
+try {
+
TokenList.parseTokenList(response.getMimeHeaders().values("Content-Encoding"),
tokens);
+} catch (IOException e) {
+// Because we are using StringReader, any exception here is a
+// Tomcat bug.
+
getLog().warn(sm.getString("http11Processor.contentEncodingParseFail"), e);
+return false;
+}
+if (tokens.contains("gzip") || tokens.contains("br")) {
+return false;
+}
}
// If force mode, always compress (test purposes only)
@@ -1342,7 +1352,7 @@ public abstract class AbstractHttp11Processor extends
AbstractProcessor {
MessageBytes connectionValueMB =
headers.getValue(Constants.CONNECTION);
if (connectionValueMB != null && !connectionValueMB.isNull()) {
Set tokens = new HashSet();
-parseConnectionTokens(headers, tokens);
+TokenList.parseTokenList(headers.values(Constants.CONNECTION),
tokens);
if (tokens.contains(Constants.CLOSE)) {
keepAlive = false;
} else if (tokens.contains(Constants.KEEPALIVE)) {
@@ -1758,22 +1768,11 @@ public abstract class AbstractHttp11Processor
extends AbstractProcessor {
}
Set tokens = new HashSet();
-parseConnectionTokens(headers, tokens);
+TokenList.parseTokenList(headers.values(Constants.CONNECTION), tokens);
return tokens.contains(token);
}
-private static void parseConnectionTokens(MimeHeaders headers,
Collection tokens) throws IOException {
-Enumeration values = headers.values(Constants.CONNECTION);
-while (values.hasMoreElements()) {
-String nextHeaderValue = values.nextElement();
-if (nextHeaderValue != null) {
-TokenList.parseTokenList(new StringReader(nextHeaderValue),
tokens);
-}
-}
-}
-
-
protected abstract boolean prepareSendfile(OutputFilter[] outputFilters);
diff --git a/java/org/apache/coyote/http11/LocalStrings.properties
b/java/org/apache/coyote/http11/LocalStrings.properties
index 7f3eb7e..ad41849 100644
--- a/java/org/apache/coyote/http11/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -16,6 +16,7 @@
http11Processor.upgrade=An internal error has occurred as upgraded connections
should only be processed by the dedicated upgrade processor implementations
http11processor.comet.notsupported=The Comet protocol is not supported by this
connector
+http11Processor.contentEncodingParseFail=Failed to parse Content-Encoding
header when chekcing to see if c
[Bug 63829] CompressionConfig does compare request header values for complete tokens case-insensitively
https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Mark Thomas --- Fixed in: - master for 9.0.28 onwards - 8.5.x for 8.5.48 onwards - 7.0.x for 7.0.98 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 63864] Use new TokenList parser for Http11Processor Transfer-Encoding handling
https://bz.apache.org/bugzilla/show_bug.cgi?id=63864 --- Comment #1 from Michael Osipov --- The spots in question are: * https://github.com/apache/tomcat/blob/master/java/org/apache/coyote/http11/Http11Processor.java#L722-L739 * https://github.com/apache/tomcat/blob/8.5.x/java/org/apache/coyote/http11/Http11Processor.java#L1112-L1129 -- 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: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new a9ac9c0 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830
a9ac9c0 is described below
commit a9ac9c0cd4c63106667a9af8a332dc4d2dc1389b
Author: Mark Thomas
AuthorDate: Wed Oct 23 12:15:05 2019 +0200
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830
Ensure this unit test passes when running on system using a locale that
uses a language other than English.
---
test/org/apache/coyote/http2/TestHttp2Limits.java | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/test/org/apache/coyote/http2/TestHttp2Limits.java
b/test/org/apache/coyote/http2/TestHttp2Limits.java
index 2a9ba74..d5f109f 100644
--- a/test/org/apache/coyote/http2/TestHttp2Limits.java
+++ b/test/org/apache/coyote/http2/TestHttp2Limits.java
@@ -31,9 +31,12 @@ import org.junit.Test;
import org.apache.catalina.connector.Connector;
import org.apache.coyote.http2.HpackEncoder.State;
import org.apache.tomcat.util.http.MimeHeaders;
+import org.apache.tomcat.util.res.StringManager;
public class TestHttp2Limits extends Http2TestBase {
+private static final StringManager sm =
StringManager.getManager(TestHttp2Limits.class);
+
@Test
public void testHeaderLimits1x128() throws Exception {
// Well within limits
@@ -247,6 +250,12 @@ public class TestHttp2Limits extends Http2TestBase {
break;
}
case CONNECTION_RESET: {
+// This message uses i18n and needs to be used in a regular
+// expression (since we don't know the connection ID). Generate the
+// string as a regular expression and then replace '[' and ']' with
+// the escaped values.
+String limitMessage = sm.getString("http2Parser.headerLimitSize",
"\\d++", "3");
+limitMessage = limitMessage.replace("[", "\\[").replace("]",
"\\]");
// Connection reset. Connection ID will vary so use a pattern
// On some platform / Connector combinations (e.g. Windows / APR),
// the TCP connection close will be processed before the client
gets
@@ -257,7 +266,7 @@ public class TestHttp2Limits extends Http2TestBase {
try {
parser.readFrame(true);
Assert.assertThat(output.getTrace(), RegexMatcher.matchesRegex(
-"0-Goaway-\\[1\\]-\\[11\\]-\\[Connection \\[\\d++\\],
Stream \\[3\\], .*"));
+"0-Goaway-\\[1\\]-\\[11\\]-\\[" + limitMessage +
"\\]"));
} catch (IOException se) {
// Expected on some platforms
}
@@ -490,9 +499,14 @@ public class TestHttp2Limits extends Http2TestBase {
// NIO2 can sometimes send window updates depending timing
skipWindowSizeFrames();
-// Connection ID will vary so use a pattern
+// This message uses i18n and needs to be used in a regular
+// expression (since we don't know the connection ID). Generate the
+// string as a regular expression and then replace '[' and ']' with
+// the escaped values.
+String limitMessage = sm.getString("http2Parser.headerLimitSize",
"\\d++", "3");
+limitMessage = limitMessage.replace("[", "\\[").replace("]",
"\\]");
Assert.assertThat(output.getTrace(), RegexMatcher.matchesRegex(
-"0-Goaway-\\[3\\]-\\[11\\]-\\[Connection \\[\\d++\\],
Stream \\[3\\], .*"));
+"0-Goaway-\\[3\\]-\\[11\\]-\\[" + limitMessage + "\\]"));
break;
}
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 94682c0 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830
94682c0 is described below
commit 94682c0917a0cc795d3f485817a938d67ea8ed49
Author: Mark Thomas
AuthorDate: Wed Oct 23 12:15:05 2019 +0200
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830
Ensure this unit test passes when running on system using a locale that
uses a language other than English.
---
test/org/apache/coyote/http2/TestHttp2Limits.java | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/test/org/apache/coyote/http2/TestHttp2Limits.java
b/test/org/apache/coyote/http2/TestHttp2Limits.java
index 2a9ba74..d5f109f 100644
--- a/test/org/apache/coyote/http2/TestHttp2Limits.java
+++ b/test/org/apache/coyote/http2/TestHttp2Limits.java
@@ -31,9 +31,12 @@ import org.junit.Test;
import org.apache.catalina.connector.Connector;
import org.apache.coyote.http2.HpackEncoder.State;
import org.apache.tomcat.util.http.MimeHeaders;
+import org.apache.tomcat.util.res.StringManager;
public class TestHttp2Limits extends Http2TestBase {
+private static final StringManager sm =
StringManager.getManager(TestHttp2Limits.class);
+
@Test
public void testHeaderLimits1x128() throws Exception {
// Well within limits
@@ -247,6 +250,12 @@ public class TestHttp2Limits extends Http2TestBase {
break;
}
case CONNECTION_RESET: {
+// This message uses i18n and needs to be used in a regular
+// expression (since we don't know the connection ID). Generate the
+// string as a regular expression and then replace '[' and ']' with
+// the escaped values.
+String limitMessage = sm.getString("http2Parser.headerLimitSize",
"\\d++", "3");
+limitMessage = limitMessage.replace("[", "\\[").replace("]",
"\\]");
// Connection reset. Connection ID will vary so use a pattern
// On some platform / Connector combinations (e.g. Windows / APR),
// the TCP connection close will be processed before the client
gets
@@ -257,7 +266,7 @@ public class TestHttp2Limits extends Http2TestBase {
try {
parser.readFrame(true);
Assert.assertThat(output.getTrace(), RegexMatcher.matchesRegex(
-"0-Goaway-\\[1\\]-\\[11\\]-\\[Connection \\[\\d++\\],
Stream \\[3\\], .*"));
+"0-Goaway-\\[1\\]-\\[11\\]-\\[" + limitMessage +
"\\]"));
} catch (IOException se) {
// Expected on some platforms
}
@@ -490,9 +499,14 @@ public class TestHttp2Limits extends Http2TestBase {
// NIO2 can sometimes send window updates depending timing
skipWindowSizeFrames();
-// Connection ID will vary so use a pattern
+// This message uses i18n and needs to be used in a regular
+// expression (since we don't know the connection ID). Generate the
+// string as a regular expression and then replace '[' and ']' with
+// the escaped values.
+String limitMessage = sm.getString("http2Parser.headerLimitSize",
"\\d++", "3");
+limitMessage = limitMessage.replace("[", "\\[").replace("]",
"\\]");
Assert.assertThat(output.getTrace(), RegexMatcher.matchesRegex(
-"0-Goaway-\\[3\\]-\\[11\\]-\\[Connection \\[\\d++\\],
Stream \\[3\\], .*"));
+"0-Goaway-\\[3\\]-\\[11\\]-\\[" + limitMessage + "\\]"));
break;
}
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 63830] Tests in org.apache.coyote.http2.TestHttp2Limits fails due to locale unwareness
https://bz.apache.org/bugzilla/show_bug.cgi?id=63830 Mark Thomas changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #1 from Mark Thomas --- Fixed in: - master for 9.0.28 onwards - 8.5.x for 8.5.48 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
Re: [tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
On 23/10/2019 11:09, ma...@apache.org wrote: > This is an automated email from the ASF dual-hosted git repository. > > markt pushed a commit to branch 8.5.x > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > The following commit(s) were added to refs/heads/8.5.x by this push: > new 9054e10 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 > 9054e10 is described below > > commit 9054e10d53170afcd7dd85bd22335238625958dc > Author: Mark Thomas > AuthorDate: Wed Oct 23 08:50:11 2019 +0200 > > Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 > > Improve the check of the Content-Encoding header when looking to see if > Tomcat is serving pre-compressed content. Ensure that only a full token > is matched and that the match is case insensitive. This isn't complete for 8.5.x because only HTTP/2 uses CompressionConfig to make the compress / don't compress decision. I could apply a similar change to the relevant parts of Http11Processor but I was wondering about the possibility of a more extensive back-port that aligned the 8.5.x implementation with 9.0.x. This would involve API changes including: - retain a reference to the Protocol - change to constructor signature - remove unnecessary getters and setters While this is tempting from both a simplification PoV and from an aligning 8.5.x and 9.0.x PoV I do wonder what the risk of breakage is if users are extending Http11Processor. It is an internal API but I suspect it is still used by some. I think I am going to look at see if there is some sort of middle ground to be found. Meanwhile, what do people think about API changes along the lines of the above. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
On 23/10/2019 13:28, Mark Thomas wrote: > On 23/10/2019 11:09, ma...@apache.org wrote: >> This is an automated email from the ASF dual-hosted git repository. >> >> markt pushed a commit to branch 8.5.x >> in repository https://gitbox.apache.org/repos/asf/tomcat.git >> >> >> The following commit(s) were added to refs/heads/8.5.x by this push: >> new 9054e10 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 >> 9054e10 is described below >> >> commit 9054e10d53170afcd7dd85bd22335238625958dc >> Author: Mark Thomas >> AuthorDate: Wed Oct 23 08:50:11 2019 +0200 >> >> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 >> >> Improve the check of the Content-Encoding header when looking to see if >> Tomcat is serving pre-compressed content. Ensure that only a full token >> is matched and that the match is case insensitive. > > This isn't complete for 8.5.x because only HTTP/2 uses CompressionConfig > to make the compress / don't compress decision. > > I could apply a similar change to the relevant parts of Http11Processor > but I was wondering about the possibility of a more extensive back-port > that aligned the 8.5.x implementation with 9.0.x. This would involve API > changes including: > - retain a reference to the Protocol > - change to constructor signature > - remove unnecessary getters and setters > > While this is tempting from both a simplification PoV and from an > aligning 8.5.x and 9.0.x PoV I do wonder what the risk of breakage is if > users are extending Http11Processor. It is an internal API but I suspect > it is still used by some. > > I think I am going to look at see if there is some sort of middle ground > to be found. Meanwhile, what do people think about API changes along the > lines of the above. Looking at the history, we have changed the API for the constructor in the past so I think it would be safe to do that again. My plan is to replace most of the parameters with the Protocol. I think the getters and setters will need to stay. They can/should probably be deprecated as they are removed in 9.0.x. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 63875] Tomcat 8.5.46, APR/libtcnative crashes
https://bz.apache.org/bugzilla/show_bug.cgi?id=63875 Remy Maucherat changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #5 from Remy Maucherat --- It would be useful to test Tomcat 9 with NIO. The stack indicates a crash on stop only, are the context reloads related to it ? -- 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 63867] Add option for reason phrase
https://bz.apache.org/bugzilla/show_bug.cgi?id=63867 --- Comment #12 from Remy Maucherat --- This is a straight repost from https://bz.apache.org/bugzilla/show_bug.cgi?id=60362#c12 Note that the firmware demands "200 OK", not just "200 foo". I was pretty harsh in that other BZ. I still don't understand how the device can securely connect to any webserver, thus causing a major security risk for its unfortunate users. His only option is to dump the product he paid money for. So the reporter claims mistreatment but seems pretty good at doing worse. That being said, the reporter will likely be back again two years from now wasting our time again about his firmware, so can't we simply add a system property with an embarrassing name ? Like ADD_OK_REASON_PHRASE_FOR_BROKEN_CLIENTS. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Refactor Processor creation
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 117f44c Refactor Processor creation
117f44c is described below
commit 117f44cc6907d96884d6d215a7bacdccefc2067d
Author: Mark Thomas
AuthorDate: Wed Oct 23 15:31:43 2019 +0200
Refactor Processor creation
Better align processor creation with 9.0.x and to prepare for
back-porting of compression changes.
---
.../coyote/http11/AbstractHttp11Protocol.java | 10 ++---
java/org/apache/coyote/http11/Http11Processor.java | 44 +-
2 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index 86ecd1a..0310a0e 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -348,6 +348,9 @@ public abstract class AbstractHttp11Protocol extends
AbstractProtocol {
allowedTrailerHeaders.removeAll(toRemove);
}
}
+protected Set getAllowedTrailerHeadersInternal() {
+return allowedTrailerHeaders;
+}
public String getAllowedTrailerHeaders() {
// Chances of a size change between these lines are small enough that a
// sync is unnecessary.
@@ -870,14 +873,9 @@ public abstract class AbstractHttp11Protocol extends
AbstractProtocol {
// - Common
code
-@SuppressWarnings("deprecation")
@Override
protected Processor createProcessor() {
-Http11Processor processor = new Http11Processor(getMaxHttpHeaderSize(),
-getAllowHostHeaderMismatch(), getRejectIllegalHeaderName(),
getEndpoint(),
-getMaxTrailerSize(), allowedTrailerHeaders,
getMaxExtensionSize(),
-getMaxSwallowSize(), httpUpgradeProtocols,
getSendReasonPhrase(),
-relaxedPathChars, relaxedQueryChars);
+Http11Processor processor = new Http11Processor(this, getEndpoint());
processor.setAdapter(getAdapter());
processor.setMaxKeepAliveRequests(getMaxKeepAliveRequests());
processor.setConnectionUploadTimeout(getConnectionUploadTimeout());
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index f6de5f1..b922d61 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -21,7 +21,6 @@ import java.io.InterruptedIOException;
import java.nio.ByteBuffer;
import java.util.HashSet;
import java.util.Locale;
-import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
@@ -73,6 +72,9 @@ public class Http11Processor extends AbstractProcessor {
private static final StringManager sm =
StringManager.getManager(Http11Processor.class);
+private final AbstractHttp11Protocol protocol;
+
+
/**
* Input.
*/
@@ -214,37 +216,30 @@ public class Http11Processor extends AbstractProcessor {
protected SendfileDataBase sendfileData = null;
-/**
- * UpgradeProtocol information
- */
-private final Map httpUpgradeProtocols;
-
-private final boolean allowHostHeaderMismatch;
-
-
-public Http11Processor(int maxHttpHeaderSize, boolean
allowHostHeaderMismatch,
-boolean rejectIllegalHeaderName, AbstractEndpoint endpoint, int
maxTrailerSize,
-Set allowedTrailerHeaders, int maxExtensionSize, int
maxSwallowSize,
-Map httpUpgradeProtocols, boolean
sendReasonPhrase,
-String relaxedPathChars, String relaxedQueryChars) {
-
+@SuppressWarnings("deprecation")
+public Http11Processor(AbstractHttp11Protocol protocol,
AbstractEndpoint endpoint) {
super(endpoint);
+this.protocol = protocol;
-httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
+httpParser = new HttpParser(protocol.getRelaxedPathChars(),
+protocol.getRelaxedQueryChars());
-inputBuffer = new Http11InputBuffer(request, maxHttpHeaderSize,
rejectIllegalHeaderName, httpParser);
+inputBuffer = new Http11InputBuffer(request,
protocol.getMaxHttpHeaderSize(),
+protocol.getRejectIllegalHeaderName(), httpParser);
request.setInputBuffer(inputBuffer);
-outputBuffer = new Http11OutputBuffer(response, maxHttpHeaderSize,
sendReasonPhrase);
+outputBuffer = new Http11OutputBuffer(response,
protocol.getMaxHttpHeaderSize(),
+protocol.getSendReasonPhrase());
response.setOutputBuffer(outputBuffer);
// Create and add the identity filters.
-inputBuffer.addFilter(new IdentityInputFilter(maxSwallowSize));
+
[Bug 63867] Add option for reason phrase
https://bz.apache.org/bugzilla/show_bug.cgi?id=63867 --- Comment #13 from Michael Osipov --- (In reply to Remy Maucherat from comment #12) > This is a straight repost from > https://bz.apache.org/bugzilla/show_bug.cgi?id=60362#c12 > > Note that the firmware demands "200 OK", not just "200 foo". I was pretty > harsh in that other BZ. I still don't understand how the device can securely > connect to any webserver, thus causing a major security risk for its > unfortunate users. His only option is to dump the product he paid money for. > So the reporter claims mistreatment but seems pretty good at doing worse. > > That being said, the reporter will likely be back again two years from now > wasting our time again about his firmware, so can't we simply add a system > property with an embarrassing name ? Like > ADD_OK_REASON_PHRASE_FOR_BROKEN_CLIENTS. He will be back as soon as Tomcat 10 is out and Spring Boot dumps Tomcat 9. I surely that he won't have this clients updated by then. -- 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 BZ-63835/9.0.x updated (a5e3e1d -> 38fdae1)
This is an automated email from the ASF dual-hosted git repository. michaelo pushed a change to branch BZ-63835/9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. discard a5e3e1d Properly determine requests left on a "keepAlive" connection discard f903e1f First draft add db7e2d2 Further align complete()/dispatch() if called during async I/O add 71389c0 asyncStarted should be false once complete/dispatch in onTimeout add 02b865c asyncStarted should be false once complete/dispatch in onError add 8e94406 63765: Properly mark container as FAILED when a JVM error occurs on stop add 4892eda Update to Tomcat 9.0.27 add 44a50ce Add test case for bug 63816 add 85289c3 Add debug log messages for the triggering of async listener events add 327c553 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors add 302b06b Fix debug message formatting add a80693f Expand test for https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 add b0376a2 Refactor add 0a7deb5 Refactor add a730395 Remove an illegal state transition add 8d4d663 Hack to fix fialing test add e7988cc Refactor the unit test to avoid race conditions add 339f910 Fix BZ number add 101ac39 Refactor Vary parser to the more generic TokenList parser add 7c913da Add a case sensitive / insensitive option to the token list parser add 11dee21 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63824 add c76d9f3 Simplify on the grounds all tokens of interest are case-insensitive add 4dd08ae Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63825 add c298138 Remove unused String add 3d73dfc Fix possible NPE with excessive header size add b641633 Update state definitions and associated diagram (now a lot simpler) add b8cc215 Minor optimisation - add new line to access log message outside the sync add 46ebe8b Additional fix for https://bz.apache.org/bugzilla/show_bug.cgi?id=63825 add e3f18d5 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63826 add fe11123 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63815 add d252ad8 Update Korean translations add fc5ddda Made some variable names more consistent with the other parts. add 386da63 Merge pull request #217 from euske/master add c41fe3b Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 add a9ac9c0 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63830 new 38fdae1 BZ 63835: Add support for Keep-Alive header This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (a5e3e1d) \ N -- N -- N refs/heads/BZ-63835/9.0.x (38fdae1) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: bin/catalina.sh| 16 +- bin/daemon.sh | 4 +- bin/tool-wrapper.sh| 2 +- build.xml | 4 + java/javax/servlet/GenericServlet.java | 6 +- .../org/apache/catalina/core/AsyncContextImpl.java | 12 + .../apache/catalina/core/LocalStrings.properties | 4 + .../catalina/filters/AddDefaultCharsetFilter.java | 12 +- .../membership/cloud/CloudMembershipService.java | 2 +- java/org/apache/catalina/util/LifecycleBase.java | 2 +- .../org/apache/catalina/valves/AccessLogValve.java | 2 +- java/org/apache/coyote/AbstractProcessor.java | 17 +- java/org/apache/coyote/AsyncStateMachine.java | 274 +++ java/org/apache/coyote/CompressionConfig.java | 28 +- java/org/apache/coyote/LocalStrings.properties | 3 +- java/org/apache/coyote/LocalStrings_fr.properties | 1 - java/org/apache/coyote/LocalStrings_ja.properties | 1 - java/org/apache/coyote/LocalStrings_ko.properties | 1 - java/org/apache/coyote/http11/Constants.java | 8 + java/org/apache/coyote/http11/Http11Processor.java | 90 ++--- java/org/apache/jasper/JspCompilationContext.java | 7 +- java/org/apache/jasper/compiler/Generator.java | 4 +- java/org/apache/tomcat/util/buf/CharChunk.java | 4 +- .../tomcat/util/compat/LocalStrings_ko
[tomcat] 01/01: BZ 63835: Add support for Keep-Alive header
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch BZ-63835/9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 38fdae10298cfd239e25428319522b0c8ae70087
Author: Michael Osipov
AuthorDate: Wed Oct 23 15:37:42 2019 +0200
BZ 63835: Add support for Keep-Alive header
---
java/org/apache/coyote/http11/Http11Processor.java | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index 7be52d0..262cc8c 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -915,8 +915,26 @@ public class Http11Processor extends AbstractProcessor {
headers.addValue(Constants.CONNECTION).setString(
Constants.CLOSE);
}
-} else if (!http11 && !getErrorState().isError()) {
-
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
+} else if (!getErrorState().isError()) {
+if (!http11) {
+
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
+}
+
+boolean connectionKeepAlivePresent =
+isConnectionToken(request.getMimeHeaders(),
Constants.KEEPALIVE);
+
+if (connectionKeepAlivePresent) {
+int keepAliveTimeout = protocol.getKeepAliveTimeout();
+
+if (keepAliveTimeout > 0) {
+String value = "timeout=" + keepAliveTimeout / 1000L;
+headers.setValue("Keep-Alive").setString(value);
+}
+
+if (http11) {
+
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
+}
+}
}
// Add server header
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o opened a new pull request #218: WIP: BZ 63835: Add support for Keep-Alive header
michael-o opened a new pull request #218: WIP: BZ 63835: Add support for Keep-Alive header URL: https://github.com/apache/tomcat/pull/218 This is work in progress, no tests yet. It happily runs with curl. I'd like to evaluate whether this is something worth merging for the community. @markt-asf You were talking about performance indications, can you provide your approach how you measure that?! I'd simply add `nanoTime()` before and after. WDYT? If this passes, I'd be happy to add tests with a re-review. 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 #218: WIP: BZ 63835: Add support for Keep-Alive header
ChristopherSchultz commented on a change in pull request #218: WIP: BZ 63835:
Add support for Keep-Alive header
URL: https://github.com/apache/tomcat/pull/218#discussion_r338125259
##
File path: java/org/apache/coyote/http11/Http11Processor.java
##
@@ -915,8 +915,26 @@ protected final void prepareResponse() throws IOException
{
headers.addValue(Constants.CONNECTION).setString(
Constants.CLOSE);
}
-} else if (!http11 && !getErrorState().isError()) {
-
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
+} else if (!getErrorState().isError()) {
+if (!http11) {
+
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
+}
+
+boolean connectionKeepAlivePresent =
+isConnectionToken(request.getMimeHeaders(),
Constants.KEEPALIVE);
+
+if (connectionKeepAlivePresent) {
+int keepAliveTimeout = protocol.getKeepAliveTimeout();
+
+if (keepAliveTimeout > 0) {
Review comment:
Did we decide that supporting a non-standard response header was a good idea?
Should we use `Constants.KEEP_ALIVE` (or even `Constants.KEEPALIVE`) for the
string instead of a string value, here?
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 #218: WIP: BZ 63835: Add support for Keep-Alive header
ChristopherSchultz commented on a change in pull request #218: WIP: BZ 63835:
Add support for Keep-Alive header
URL: https://github.com/apache/tomcat/pull/218#discussion_r338121286
##
File path: java/org/apache/coyote/http11/Http11Processor.java
##
@@ -915,8 +915,26 @@ protected final void prepareResponse() throws IOException
{
headers.addValue(Constants.CONNECTION).setString(
Constants.CLOSE);
}
-} else if (!http11 && !getErrorState().isError()) {
-
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
+} else if (!getErrorState().isError()) {
+if (!http11) {
+
headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
Review comment:
Is this okay for, say, HTTP 0.9? I realize that this code was previously
there.
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 63205] Unable to load certificate store on openjdk
https://bz.apache.org/bugzilla/show_bug.cgi?id=63205 --- Comment #8 from Axel --- Sorry, haven't noticed your comment Mark because i waited for a notification from Bugzilla. Next time I'll do it better. Tested now with 8.5.47 and it works again using RACF Keyrings. Thank you very much. That's great, 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
Re: [tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Mark, On 10/23/19 07:38, Mark Thomas wrote: > On 23/10/2019 13:28, Mark Thomas wrote: >> On 23/10/2019 11:09, ma...@apache.org wrote: >>> This is an automated email from the ASF dual-hosted git >>> repository. >>> >>> markt pushed a commit to branch 8.5.x in repository >>> https://gitbox.apache.org/repos/asf/tomcat.git >>> >>> >>> The following commit(s) were added to refs/heads/8.5.x by this >>> push: new 9054e10 Fix >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 9054e10 is >>> described below >>> >>> commit 9054e10d53170afcd7dd85bd22335238625958dc Author: Mark >>> Thomas AuthorDate: Wed Oct 23 08:50:11 2019 >>> +0200 >>> >>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 >>> >>> Improve the check of the Content-Encoding header when looking >>> to see if Tomcat is serving pre-compressed content. Ensure that >>> only a full token is matched and that the match is case >>> insensitive. >> >> This isn't complete for 8.5.x because only HTTP/2 uses >> CompressionConfig to make the compress / don't compress >> decision. >> >> I could apply a similar change to the relevant parts of >> Http11Processor but I was wondering about the possibility of a >> more extensive back-port that aligned the 8.5.x implementation >> with 9.0.x. This would involve API changes including: - retain a >> reference to the Protocol - change to constructor signature - >> remove unnecessary getters and setters >> >> While this is tempting from both a simplification PoV and from >> an aligning 8.5.x and 9.0.x PoV I do wonder what the risk of >> breakage is if users are extending Http11Processor. It is an >> internal API but I suspect it is still used by some. >> >> I think I am going to look at see if there is some sort of middle >> ground to be found. Meanwhile, what do people think about API >> changes along the lines of the above. > > Looking at the history, we have changed the API for the constructor > in the past so I think it would be safe to do that again. My plan > is to replace most of the parameters with the Protocol. I think the > getters and setters will need to stay. They can/should probably be > deprecated as they are removed in 9.0.x. Maybe just add another constructor instead of replacing? I guess that would make the code uglier in other places where you'd prefer to use Protocol instead of Parts/Of/Protocol and have to check to see what you've got. - -chris -BEGIN PGP SIGNATURE- Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl2weCQACgkQHPApP6U8 pFiRbBAAiK81Tr/i+gBQox9/9FoBBh98609HqkCn2F3KtqXKH5XqVa1zRw6LFG9P vo4PUP5NQni3g/QIOyUGGp7TZ5yOXSs/h4awKzMrezRGRHOwvSGw7LsLU6KI1cIZ rOWdEk/j17luLuw6KcxKKcgXHFgG7e0UH2OINF8NuNG4hLP9ZDYEbQBYltY4wKVq Tr0ppqQcGZogsrY/qzCwq4CAwoAgfw2hpoUygJYXT0/5Xwa3EMnYT6afbzhkuAcM 8DOlN71G+qPsJ0rablsbHZ0KE4JQ6hWioU7o+w+JyOAcJEiYPz2UBzezPDBsr6Id E7+txlQ5tWXsPOvYpE7aDvdjF7z+vQ+C3RpYk7mCFB4UOcYJasK4uQZhieLl6aKt FElqFIl/UkZOiEY0z3H6MU9yJ2fOFyFqjhcvzTpptUTDmpKj4Ks1D+nwxkr25om/ spnKX/cRF1mYjgFww/LgTP7qOn/BSVUtj44H8OJiGDVzK1giTrpF++uUYMdaMF33 HVk80lIhkyxlbQzTH+3C2HQhfiHn1t4TbZSvSRaTMFJvKHV6LB2NGfbqEigIMVvk RtOFeK4jGvhnVS91eRNpQW/hu+Vc14ISOQmt/s6gpKel+Sq5XRQJLPO7svOOc2Cz rm/NxWCGdMr7EEi4g2p0s/a1FK2BXnr/nalKYMBUOHWXdU/6Yes= =9+Lb -END PGP SIGNATURE- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
On 23/10/2019 17:56, Christopher Schultz wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > Mark, > > On 10/23/19 07:38, Mark Thomas wrote: >> On 23/10/2019 13:28, Mark Thomas wrote: >>> On 23/10/2019 11:09, ma...@apache.org wrote: This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/8.5.x by this push: new 9054e10 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 9054e10 is described below commit 9054e10d53170afcd7dd85bd22335238625958dc Author: Mark Thomas AuthorDate: Wed Oct 23 08:50:11 2019 +0200 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 Improve the check of the Content-Encoding header when looking to see if Tomcat is serving pre-compressed content. Ensure that only a full token is matched and that the match is case insensitive. >>> >>> This isn't complete for 8.5.x because only HTTP/2 uses >>> CompressionConfig to make the compress / don't compress >>> decision. >>> >>> I could apply a similar change to the relevant parts of >>> Http11Processor but I was wondering about the possibility of a >>> more extensive back-port that aligned the 8.5.x implementation >>> with 9.0.x. This would involve API changes including: - retain a >>> reference to the Protocol - change to constructor signature - >>> remove unnecessary getters and setters >>> >>> While this is tempting from both a simplification PoV and from >>> an aligning 8.5.x and 9.0.x PoV I do wonder what the risk of >>> breakage is if users are extending Http11Processor. It is an >>> internal API but I suspect it is still used by some. >>> >>> I think I am going to look at see if there is some sort of middle >>> ground to be found. Meanwhile, what do people think about API >>> changes along the lines of the above. >> >> Looking at the history, we have changed the API for the constructor >> in the past so I think it would be safe to do that again. My plan >> is to replace most of the parameters with the Protocol. I think the >> getters and setters will need to stay. They can/should probably be >> deprecated as they are removed in 9.0.x. > > Maybe just add another constructor instead of replacing? I guess that > would make the code uglier in other places where you'd prefer to use > Protocol instead of Parts/Of/Protocol and have to check to see what > you've got. Exactly. On the grounds we have changed this constructor a few times during the life of 8.5.x, I think this change is safe. It should also reduce the need for future changes. The next step (that wires in the CompressionConfig) might be a little more controversial. I need to finish that work and see what it looks like. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 63867] Add option for reason phrase
https://bz.apache.org/bugzilla/show_bug.cgi?id=63867 --- Comment #14 from Christopher Schultz --- So shall we mark this as DUPLICATE of bug #60362, or are we entertaining the idea of resurrecting the reason-phrase for Tomcat 9? I think most of the Tomcat devs are -0 at best for bringing back the reason-phrase. A formal vote seems heavy-handed. -- 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 62964] Add RFC7807 conformant Problem Details for HTTP status codes for Webdav Servlets
https://bz.apache.org/bugzilla/show_bug.cgi?id=62964 Christopher Schultz changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #4 from Christopher Schultz --- I've looked at the RFC and it's not great. It basically says "you must repeat the HTTP response code in the 'status' field", everything else is optional, and any endpoint can make-up its own URIs for the "error type". So the "status" doesn't add anything, and every other field has no meaning unless the client and server agree on the URIs for the "type". If WebDAV has defined specific "type" URIs then I think this is a plausible feature for implementation. Until then, it's just individual developers making stuff up in the hopes that clients choose that particular implementation. Building against non-standards gets everyone into trouble. -- 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 62140] catalina.sh should document the verbs it accepts as command-line arguments
https://bz.apache.org/bugzilla/show_bug.cgi?id=62140 --- Comment #6 from Steve Sanders --- (In reply to Mark Thomas from comment #5) > It is yours. Go for it. I wanted to take care of this as an easy into to contributing, but looks like this was already fixed with this commit: https://github.com/apache/tomcat/commit/328264a92cdec4302dbc68ef7377eebc3ecf0c4e Should this bug be marked as closed? -- 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 62140] catalina.sh should document the verbs it accepts as command-line arguments
https://bz.apache.org/bugzilla/show_bug.cgi?id=62140 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #7 from Mark Thomas --- It should. Sorry about that. I can't remember why I fixed this rather than leaving it open. Of the currently open bugs, 63865 looks like it should be a fairly simple one. The only caveat is it needs to be fixed before the next release which is due at the start of November so you only have ~1 week. If that doesn't work for you, ask on the dev@ list and we'll try and point you towards something else. -- 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
