[GitHub] [tomcat] rmaucher commented on issue #142: Defer creation of encodingToCharsetCache
rmaucher commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470434736 Besides the 31ms startup time, it's still worse in every way compared to the current code. 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] y987425112 opened a new pull request #145: Adding volatile keywords to member variables
y987425112 opened a new pull request #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145
The semantics of the Java programming language allow compilers and
microprocessors to
perform optimizations that can interact with incorrectly synchronized code
in ways that can
produce behaviors that seem
paradoxical.[https://edelivery.oracle.com/otn-pub/jcp/memory_model-1.0-pfd-spec-oth-JSpec/memory_model-1_0-pfd-spec.pdf](url)
page6
For example
```
org.apache.catalina.core.StandardContext.java
private String applicationListeners[] = new String[0];
.
@Override
public void addApplicationListener(String listener) {
```
synchronized (applicationListenersLock) {
String results[] = new String[applicationListeners.length + 1];
for (int i = 0; i < applicationListeners.length; i++) {
if (listener.equals(applicationListeners[i])) {
log.info(sm.getString("standardContext.duplicateListener",listener));
return;
}
results[i] = applicationListeners[i];
}
results[applicationListeners.length] = listener;
applicationListeners = results;
}
fireContainerEvent("addApplicationListener", listener);
}
We expect this line of code(`applicationListeners = results;`) to be
executed eventually
But because of instruction reordering,The following operating procedures are
also allowed to exis
```
@Override
public void addApplicationListener(String listener) {
```
synchronized (applicationListenersLock) {
String results[] = new String[applicationListeners.length + 1];
applicationListeners = results;
for (int i = 0; i < applicationListeners.length; i++) {
if (listener.equals(applicationListeners[i])) {
log.info(sm.getString("standardContext.duplicateListener",listener));
return;
}
results[i] = applicationListeners[i];
}
results[applicationListeners.length] = listener;
}
fireContainerEvent("addApplicationListener", listener);
}
If this happens, it will be different from our expectations.
If volatile modifiers are added to variables, instruction reordering is
prohibited
`private String applicationListeners[] = new String[0]; `-->
` private volatile String applicationListeners[] = new String[0];`
Satisfy the purpose of the last execution of this line of
code(`applicationListeners = results;`).
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
[tomcat] branch master updated: Fix broken HTTP/2 push
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 d8aae21 Fix broken HTTP/2 push
d8aae21 is described below
commit d8aae2192af76b2aab88be502b92310b065f4022
Author: Mark Thomas
AuthorDate: Thu Mar 7 10:34:00 2019 +
Fix broken HTTP/2 push
---
java/org/apache/coyote/http2/Stream.java | 26 +++---
webapps/docs/changelog.xml | 4
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 95c2101..3b14bb6 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -58,6 +58,8 @@ class Stream extends AbstractStream implements HeaderEmitter {
private static final MimeHeaders ACK_HEADERS;
+private static final Integer HTTP_UPGRADE_STREAM = Integer.valueOf(1);
+
static {
Response response = new Response();
response.setStatus(100);
@@ -100,21 +102,23 @@ class Stream extends AbstractStream implements
HeaderEmitter {
this.inputBuffer = new StreamInputBuffer();
this.coyoteRequest.setInputBuffer(inputBuffer);
} else {
-// HTTP/1.1 upgrade
+// HTTP/2 Push or HTTP/1.1 upgrade
this.coyoteRequest = coyoteRequest;
this.inputBuffer = null;
// Headers have been read by this point
state.receivedStartOfHeaders();
-// Populate coyoteRequest from headers
-try {
-prepareRequest();
-} catch (IllegalArgumentException iae) {
-// Something in the headers is invalid
-// Set correct return status
-coyoteResponse.setStatus(400);
-// Set error flag. This triggers error processing rather than
-// the normal mapping
-coyoteResponse.setError();
+if (HTTP_UPGRADE_STREAM.equals(identifier)) {
+// Populate coyoteRequest from headers (HTTP/1.1 only)
+try {
+prepareRequest();
+} catch (IllegalArgumentException iae) {
+// Something in the headers is invalid
+// Set correct return status
+coyoteResponse.setStatus(400);
+// Set error flag. This triggers error processing rather
than
+// the normal mapping
+coyoteResponse.setError();
+}
}
// TODO Assuming the body has been read at this point is not valid
state.receivedEndOfStream();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 86830b0..c831843 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,10 @@
NIO2 should try to use SocketTimeoutException everywhere rather than a
mix of it and InterruptedByTimeout. (remm)
+
+Correct an error in the request validation that meant that HTTP/2 push
+requests always resulted in a 400 response. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Fix broken HTTP/2 push
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 4938ddc Fix broken HTTP/2 push
4938ddc is described below
commit 4938ddc6be047ed7e020f2f51bd6c99da650d896
Author: Mark Thomas
AuthorDate: Thu Mar 7 10:34:00 2019 +
Fix broken HTTP/2 push
---
java/org/apache/coyote/http2/Stream.java | 26 +++---
webapps/docs/changelog.xml | 4
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 35d066a..2c9021e 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -55,6 +55,8 @@ public class Stream extends AbstractStream implements
HeaderEmitter {
private static final MimeHeaders ACK_HEADERS;
+private static final Integer HTTP_UPGRADE_STREAM = Integer.valueOf(1);
+
static {
Response response = new Response();
response.setStatus(100);
@@ -97,21 +99,23 @@ public class Stream extends AbstractStream implements
HeaderEmitter {
this.inputBuffer = new StreamInputBuffer();
this.coyoteRequest.setInputBuffer(inputBuffer);
} else {
-// HTTP/1.1 upgrade
+// HTTP/2 Push or HTTP/1.1 upgrade
this.coyoteRequest = coyoteRequest;
this.inputBuffer = null;
// Headers have been read by this point
state.receivedStartOfHeaders();
-// Populate coyoteRequest from headers
-try {
-prepareRequest();
-} catch (IllegalArgumentException iae) {
-// Something in the headers is invalid
-// Set correct return status
-coyoteResponse.setStatus(400);
-// Set error flag. This triggers error processing rather than
-// the normal mapping
-coyoteResponse.setError();
+if (HTTP_UPGRADE_STREAM.equals(identifier)) {
+// Populate coyoteRequest from headers (HTTP/1.1 only)
+try {
+prepareRequest();
+} catch (IllegalArgumentException iae) {
+// Something in the headers is invalid
+// Set correct return status
+coyoteResponse.setStatus(400);
+// Set error flag. This triggers error processing rather
than
+// the normal mapping
+coyoteResponse.setError();
+}
}
// TODO Assuming the body has been read at this point is not valid
state.receivedEndOfStream();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 90297df..64b2d07 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -160,6 +160,10 @@
NIO2 should try to use SocketTimeoutException everywhere rather than a
mix of it and InterruptedByTimeout. (remm)
+
+Correct an error in the request validation that meant that HTTP/2 push
+requests always resulted in a 400 response. (markt)
+
-
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=63223
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 38ef1f6 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63223
38ef1f6 is described below
commit 38ef1f624aaf045458b6fe055742fa680a96a9e1
Author: Mark Thomas
AuthorDate: Thu Mar 7 10:50:05 2019 +
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63223
Include pushed streams in the count of currently open streams
Reject (ignore) a pushed stream if max streams has been reached
Correctly track the state of a pushed stream
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 8
java/org/apache/coyote/http2/Stream.java | 5 +
java/org/apache/coyote/http2/StreamStateMachine.java | 8 +++-
webapps/docs/changelog.xml| 4
4 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 4189a3f..349a4c8 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -551,6 +551,7 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
synchronized (socketWrapper) {
doWriteHeaders(stream, pushedStreamId, mimeHeaders, endOfStream,
payloadSize);
}
+stream.sentHeaders();
if (endOfStream) {
stream.sentEndOfStream();
}
@@ -1177,6 +1178,13 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
void push(Request request, Stream associatedStream) throws IOException {
+if (localSettings.getMaxConcurrentStreams() <
activeRemoteStreamCount.incrementAndGet()) {
+// If there are too many open streams, simply ignore the push
+// request.
+activeRemoteStreamCount.decrementAndGet();
+return;
+}
+
Stream pushStream;
// Synchronized since PUSH_PROMISE frames have to be sent in order.
Once
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 3b14bb6..32c90b8 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -620,6 +620,11 @@ class Stream extends AbstractStream implements
HeaderEmitter {
}
+final void sentHeaders() {
+state.sentHeaders();
+}
+
+
final void sentEndOfStream() {
streamOutputBuffer.endOfStreamSent = true;
state.sentEndOfStream();
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 7b3c215..0da3a2a 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -53,6 +53,12 @@ class StreamStateMachine {
}
+final synchronized void sentHeaders() {
+// No change if currently OPEN
+stateChange(State.RESERVED_LOCAL, State.HALF_CLOSED_REMOTE);
+}
+
+
final synchronized void receivedStartOfHeaders() {
stateChange(State.IDLE, State.OPEN);
stateChange(State.RESERVED_REMOTE, State.HALF_CLOSED_LOCAL);
@@ -170,7 +176,7 @@ class StreamStateMachine {
Http2Error.PROTOCOL_ERROR, FrameType.PRIORITY,
FrameType.RST,
FrameType.WINDOW_UPDATE),
-RESERVED_REMOTE(false, false, true, true,
+RESERVED_REMOTE(false, true, true, true,
Http2Error.PROTOCOL_ERROR, FrameType.HEADERS,
FrameType.PRIORITY,
FrameType.RST),
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c831843..3331c08 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,10 @@
Correct an error in the request validation that meant that HTTP/2 push
requests always resulted in a 400 response. (markt)
+
+63223: Correctly account for push requests when tracking
+currently active HTTP/2 streams. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rainerjung commented on issue #145: Adding volatile keywords to member variables
rainerjung commented on issue #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145#issuecomment-470482121 My understanding always was, that reordering must obey as-if-serial semantics for any single thread. The complexity only kicks in when checking, whether effects of statements run by one thread are visible by another. The concrete example you discuss IMHO does not have any visibility problems, because the changes are inside a synchronized block. Changes done by thread T1 while holding any lock are visible by any other thread T2 provided that T1 releases the lock after the changes and T2 acquired it before accessing the changed data. 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
buildbot success in on tomcat-trunk
The Buildbot has detected a restored build on builder tomcat-trunk while building tomcat. Full details are available at: https://ci.apache.org/builders/tomcat-trunk/builds/4118 Buildbot URL: https://ci.apache.org/ Buildslave for this Build: silvanus_ubuntu Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-commit' triggered this build Build Source Stamp: [branch master] d8aae2192af76b2aab88be502b92310b065f4022 Blamelist: Mark Thomas Build succeeded! Sincerely, -The Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
buildbot success in on tomcat-85-trunk
The Buildbot has detected a restored build on builder tomcat-85-trunk while building tomcat. Full details are available at: https://ci.apache.org/builders/tomcat-85-trunk/builds/1688 Buildbot URL: https://ci.apache.org/ Buildslave for this Build: silvanus_ubuntu Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-85-commit' triggered this build Build Source Stamp: [branch 8.5.x] 4938ddc6be047ed7e020f2f51bd6c99da650d896 Blamelist: Mark Thomas Build succeeded! Sincerely, -The Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488340
`package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}`
Dead cycle
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] y987425112 removed a comment on issue #145: Adding volatile keywords to member variables
y987425112 removed a comment on issue #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488340
`package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}`
Dead cycle
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] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488862
```
package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
```
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}
Dead cycle
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] y987425112 removed a comment on issue #145: Adding volatile keywords to member variables
y987425112 removed a comment on issue #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470488862
```
package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
```
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}
Dead cycle
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] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470489493
```
package com.ydy.thread.volatile2;
public class VolatileTest {
public static void main(String[] args) throws Exception {
Task task=new Task();
task.start();
Thread.sleep(2000);
task.setFlag(false);
System.out.println("task.setFlag(true)");
}
private static class Task extends Thread{
privateboolean flag=true;
private int num=0;
@Override
public void run() {
// TODO Auto-generated method stub
while(getFlag()) {
num ++;
}
}
private boolean getFlag() {
return flag;
}
public synchronized void setFlag(boolean flag) {
this.flag = flag;
}
}
}
```
Dead cycle
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
[tomcat] branch 8.5.x updated: Align HTTP/2 code with trunk to simplify back-ports
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 e529749 Align HTTP/2 code with trunk to simplify back-ports
e529749 is described below
commit e529749c1eeaceaca77fd83e87c522b271ab6311
Author: Mark Thomas
AuthorDate: Thu Mar 7 11:26:34 2019 +
Align HTTP/2 code with trunk to simplify back-ports
---
java/org/apache/coyote/http2/Http2Parser.java | 54 +++---
.../apache/coyote/http2/Http2UpgradeHandler.java | 29 +++-
2 files changed, 44 insertions(+), 39 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2Parser.java
b/java/org/apache/coyote/http2/Http2Parser.java
index d696cd4..3980094 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -449,33 +449,6 @@ class Http2Parser {
}
-private void onHeadersComplete(int streamId) throws Http2Exception {
-// Any left over data is a compression error
-if (headerReadBuffer.position() > 0) {
-throw new ConnectionException(
-
sm.getString("http2Parser.processFrameHeaders.decodingDataLeft"),
-Http2Error.COMPRESSION_ERROR);
-}
-
-// Delay validation (and triggering any exception) until this point
-// since all the headers still have to be read if a StreamException is
-// going to be thrown.
-hpackDecoder.getHeaderEmitter().validateHeaders();
-
-output.headersEnd(streamId);
-
-if (headersEndStream) {
-output.receivedEndOfStream(streamId);
-headersEndStream = false;
-}
-
-// Reset size for new request if the buffer was previously expanded
-if (headerReadBuffer.capacity() >
Constants.DEFAULT_HEADER_READ_BUFFER_SIZE) {
-headerReadBuffer =
ByteBuffer.allocate(Constants.DEFAULT_HEADER_READ_BUFFER_SIZE);
-}
-}
-
-
private void readUnknownFrame(int streamId, FrameType frameType, int
flags, int payloadSize)
throws IOException {
try {
@@ -518,6 +491,33 @@ class Http2Parser {
}
+private void onHeadersComplete(int streamId) throws Http2Exception {
+// Any left over data is a compression error
+if (headerReadBuffer.position() > 0) {
+throw new ConnectionException(
+
sm.getString("http2Parser.processFrameHeaders.decodingDataLeft"),
+Http2Error.COMPRESSION_ERROR);
+}
+
+// Delay validation (and triggering any exception) until this point
+// since all the headers still have to be read if a StreamException is
+// going to be thrown.
+hpackDecoder.getHeaderEmitter().validateHeaders();
+
+output.headersEnd(streamId);
+
+if (headersEndStream) {
+output.receivedEndOfStream(streamId);
+headersEndStream = false;
+}
+
+// Reset size for new request if the buffer was previously expanded
+if (headerReadBuffer.capacity() >
Constants.DEFAULT_HEADER_READ_BUFFER_SIZE) {
+headerReadBuffer =
ByteBuffer.allocate(Constants.DEFAULT_HEADER_READ_BUFFER_SIZE);
+}
+}
+
+
/*
* Implementation note:
* Validation applicable to all incoming frames should be implemented here.
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 6f3287e..6e266c3 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -242,17 +242,7 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
}
// Send the initial settings frame
-try {
-byte[] settings = localSettings.getSettingsFrameForPending();
-socketWrapper.write(true, settings, 0, settings.length);
-socketWrapper.flush(true);
-} catch (IOException ioe) {
-String msg = sm.getString("upgradeHandler.sendPrefaceFail",
connectionId);
-if (log.isDebugEnabled()) {
-log.debug(msg);
-}
-throw new ProtocolException(msg, ioe);
-}
+writeSettings();
// Make sure the client has sent a valid connection preface before we
// send the response to the original request over HTTP/2.
@@ -527,6 +517,22 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
}
+private void writeSettings() {
+// Send the initial settings frame
+try {
+byte[] settings = localSettings.getSettingsFrameForPending();
+socketWrapper.write(true, settings, 0, settings.length);
+socketWrapper.flush(true);
+} catch (IOExcep
Re: Tomcat Site Redesign
пн, 4 мар. 2019 г. в 09:02, Igal Sapir : > > I have uploaded the Tomcat site redesign to a temporary location for review: > http://people.apache.org/~isapir/mockups/tomcat-site/ 1. Overall: Thank you for your effort, but I do not like it. At all. 2. Impressions: 1) The text is not as readable as the existing site. It is hard to read it because of color. It blurs with background. 2) Advertisement steals important part of the page. Nobody likes advertisements, you know? Especially when it is thrown in one's face. Who are our clients? Why people come to the site and what do they expect to see? Newspapers like to throw ads like that and hide the real text, but they make money from that and people tolerate them (to a certain degree). 3) I do not like menu at the top. Those were popular some time ago, but I think that is a diminishing trend. (What is a concept behind such design element? It is not a material element, not something from real world. How do people operate with it?) It does not make sense that the menu is flying at top when I scroll. This menu occupies precious space on the screen. A page title flying like that makes more sense. 4) The "Apache Tomcat" in menu line is too prominent in font and color. It steals attention from the real page title and hurts eyes. 3. Technically: 1) Printing =?. It is not printer-friendly! Browser -> Print preview. I see that it does not fit a paper page. 2) Accessibility =?. (Tab key navigation with keyboard. Accessing menus. Accessing links). Keyboard. How does one operate that menu with a keyboard? Mouse. Why do I need two clicks to open anything through that menu? 3) Titles and dates. "2019-02-21 Tomcat 7.0.93 Released" The dates should not be spelled like that if they are part of a sentence. How do you read that sentence aloud (in English)? (Actually, the dates should not be in the sentence. They are a separate auxiliary piece of information). 4) Search field (in menu bar): It is too small for any real text. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated: Remove temporary debug code that is no longer being used
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 cc367b6 Remove temporary debug code that is no longer being used
cc367b6 is described below
commit cc367b62c94df4634070cda8ff8a26f974f5ab03
Author: Mark Thomas
AuthorDate: Thu Mar 7 11:29:29 2019 +
Remove temporary debug code that is no longer being used
---
java/org/apache/coyote/http2/StreamProcessor.java | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index 89cad0a..6870926 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -76,8 +76,6 @@ class StreamProcessor extends AbstractProcessor {
ConnectionException ce = new
ConnectionException(sm.getString(
"streamProcessor.error.connection",
stream.getConnectionId(),
stream.getIdentifier()),
Http2Error.INTERNAL_ERROR);
-// TODO - Temporary debug code
-log.info(ce.getMessage(), ce);
stream.close(ce);
} else if (!getErrorState().isIoAllowed()) {
StreamException se = stream.getResetException();
@@ -93,10 +91,9 @@ class StreamProcessor extends AbstractProcessor {
} catch (Exception e) {
String msg =
sm.getString("streamProcessor.error.connection",
stream.getConnectionId(), stream.getIdentifier());
-// TODO - Temporary debug code
-//if (log.isDebugEnabled()) {
-log.info(msg, e);
-//}
+if (log.isDebugEnabled()) {
+log.debug(msg, e);
+}
ConnectionException ce = new ConnectionException(msg,
Http2Error.INTERNAL_ERROR);
ce.initCause(e);
stream.close(ce);
-
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: Align HTTP/2 code with master to simplify back-ports
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 4c782c5 Align HTTP/2 code with master to simplify back-ports
4c782c5 is described below
commit 4c782c59e5f21e16810237f8ff22813821bbe53a
Author: Mark Thomas
AuthorDate: Thu Mar 7 11:45:39 2019 +
Align HTTP/2 code with master to simplify back-ports
---
.../apache/coyote/http2/Http2UpgradeHandler.java | 87 +-
1 file changed, 50 insertions(+), 37 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 6e266c3..8e45d4f 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -558,6 +558,22 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
void writeHeaders(Stream stream, int pushedStreamId, MimeHeaders
mimeHeaders,
boolean endOfStream, int payloadSize) throws IOException {
+// This ensures the Stream processing thread has control of the socket.
+synchronized (socketWrapper) {
+doWriteHeaders(stream, pushedStreamId, mimeHeaders, endOfStream,
payloadSize);
+}
+if (endOfStream) {
+stream.sentEndOfStream();
+}
+}
+
+
+/*
+ * Separate method to allow Http2AsyncUpgradeHandler to call this code
+ * without synchronizing on socketWrapper since it doesn't need to.
+ */
+protected void doWriteHeaders(Stream stream, int pushedStreamId,
+MimeHeaders mimeHeaders, boolean endOfStream, int payloadSize)
throws IOException {
if (log.isDebugEnabled()) {
if (pushedStreamId == 0) {
@@ -586,47 +602,44 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
boolean first = true;
State state = null;
-// This ensures the Stream processing thread has control of the socket.
-synchronized (socketWrapper) {
-while (state != State.COMPLETE) {
-if (first && pushedStreamIdBytes != null) {
-payload.put(pushedStreamIdBytes);
-}
-state = getHpackEncoder().encode(mimeHeaders, payload);
-payload.flip();
-if (state == State.COMPLETE || payload.limit() > 0) {
-ByteUtil.setThreeBytes(header, 0, payload.limit());
-if (first) {
-first = false;
-if (pushedStreamIdBytes == null) {
-header[3] = FrameType.HEADERS.getIdByte();
-} else {
-header[3] = FrameType.PUSH_PROMISE.getIdByte();
-}
-if (endOfStream) {
-header[4] = FLAG_END_OF_STREAM;
-}
+while (state != State.COMPLETE) {
+if (first && pushedStreamIdBytes != null) {
+payload.put(pushedStreamIdBytes);
+}
+state = getHpackEncoder().encode(mimeHeaders, payload);
+payload.flip();
+if (state == State.COMPLETE || payload.limit() > 0) {
+ByteUtil.setThreeBytes(header, 0, payload.limit());
+if (first) {
+first = false;
+if (pushedStreamIdBytes == null) {
+header[3] = FrameType.HEADERS.getIdByte();
} else {
-header[3] = FrameType.CONTINUATION.getIdByte();
-}
-if (state == State.COMPLETE) {
-header[4] += FLAG_END_OF_HEADERS;
-}
-if (log.isDebugEnabled()) {
-log.debug(payload.limit() + " bytes");
+header[3] = FrameType.PUSH_PROMISE.getIdByte();
}
-ByteUtil.set31Bits(header, 5, stream.getIdAsInt());
-try {
-socketWrapper.write(true, header, 0, header.length);
-socketWrapper.write(true, payload);
-socketWrapper.flush(true);
-} catch (IOException ioe) {
-handleAppInitiatedIOException(ioe);
+if (endOfStream) {
+header[4] = FLAG_END_OF_STREAM;
}
-payload.clear();
-} else if (state == State.UNDERFLOW) {
-payload = ByteBuffer.allocate(payload.capacity() * 2);
+} else {
+header[3] = FrameType.CONTINUATION.getIdByte();
+}
+if (state == State
Re: svn commit: r1854025 - in /tomcat/trunk/java/org/apache/tomcat/util/net: AbstractJsseEndpoint.java SSLUtilBase.java jsse/JSSEUtil.java openssl/OpenSSLUtil.java
On Thu, Feb 21, 2019 at 10:29 AM wrote:
> Author: markt
> Date: Thu Feb 21 09:29:29 2019
> New Revision: 1854025
>
> URL: http://svn.apache.org/viewvc?rev=1854025&view=rev
> Log:
> Refactor creation of SSLContext to include configuration
>
There's probably an issue with that strategy. I have one of my test
configurations which uses a plain (old) dumb pkcs1 certificate file - the
private key uses BEGIN RSA PRIVATE KEY - with OpenSSL. Predictably it
doesn't work with this addition +sslContext.init(getKeyManagers(),
getTrustManagers(), null); as it calls getKeyManagers().
The OpenSSLContext should probably override getKeyManagers() to work around
the issue, right ? [like, actually avoid using a real keystore at all in
that case] Beyond that point, I'm pretty sure it would work fine.
The exception is:
org.apache.catalina.LifecycleException: Protocol handler initialization
failed
at
org.apache.catalina.connector.Connector.initInternal(Connector.java:983)
at org.apache.catalina.util.LifecycleBase.init(LifecycleBase.java:136)
at
org.apache.catalina.core.StandardService.initInternal(StandardService.java:535)
at org.apache.catalina.util.LifecycleBase.init(LifecycleBase.java:136)
at
org.apache.catalina.core.StandardServer.initInternal(StandardServer.java:1055)
at org.apache.catalina.util.LifecycleBase.init(LifecycleBase.java:136)
at org.apache.catalina.startup.Catalina.load(Catalina.java:585)
at org.apache.catalina.startup.Catalina.load(Catalina.java:608)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.catalina.startup.Bootstrap.load(Bootstrap.java:306)
at org.apache.catalina.startup.Bootstrap.main(Bootstrap.java:491)
Caused by: java.lang.IllegalArgumentException: Cannot store non-PrivateKeys
at
org.apache.tomcat.util.net.AbstractJsseEndpoint.createSSLContext(AbstractJsseEndpoint.java:99)
at
org.apache.tomcat.util.net.AbstractJsseEndpoint.initialiseSsl(AbstractJsseEndpoint.java:71)
at org.apache.tomcat.util.net.Nio2Endpoint.bind(Nio2Endpoint.java:158)
at
org.apache.tomcat.util.net.AbstractEndpoint.bindWithCleanup(AbstractEndpoint.java:1103)
at
org.apache.tomcat.util.net.AbstractEndpoint.init(AbstractEndpoint.java:1116)
at org.apache.coyote.AbstractProtocol.init(AbstractProtocol.java:557)
at
org.apache.coyote.http11.AbstractHttp11Protocol.init(AbstractHttp11Protocol.java:74)
at
org.apache.catalina.connector.Connector.initInternal(Connector.java:980)
... 13 more
Caused by: java.security.KeyStoreException: Cannot store non-PrivateKeys
at
sun.security.provider.JavaKeyStore.engineSetKeyEntry(JavaKeyStore.java:261)
at
sun.security.provider.JavaKeyStore$JKS.engineSetKeyEntry(JavaKeyStore.java:56)
at
sun.security.provider.KeyStoreDelegator.engineSetKeyEntry(KeyStoreDelegator.java:117)
at
sun.security.provider.JavaKeyStore$DualFormatJKS.engineSetKeyEntry(JavaKeyStore.java:70)
at java.security.KeyStore.setKeyEntry(KeyStore.java:1140)
at
org.apache.tomcat.util.net.SSLUtilBase.getKeyManagers(SSLUtilBase.java:313)
at
org.apache.tomcat.util.net.SSLUtilBase.createSSLContext(SSLUtilBase.java:239)
at
org.apache.tomcat.util.net.AbstractJsseEndpoint.createSSLContext(AbstractJsseEndpoint.java:97)
... 20 more
Rémy
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java
> tomcat/trunk/java/org/apache/tomcat/util/net/SSLUtilBase.java
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSEUtil.java
> tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java?rev=1854025&r1=1854024&r2=1854025&view=diff
>
> ==
> --- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java
> (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java
> Thu Feb 21 09:29:29 2019
> @@ -109,7 +109,6 @@ public abstract class AbstractJsseEndpoi
> SSLContext sslContext;
> try {
> sslContext =
> sslUtil.createSSLContext(negotiableProtocols);
> -sslContext.init(sslUtil.getKeyManagers(),
> sslUtil.getTrustManagers(), null);
> } catch (Exception e) {
> throw new IllegalArgumentException(e.getMessage(), e);
> }
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SSLUtilBase.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SSLUtilBase.java?rev=1854025&r1=1854024&r2=185402
[tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63223
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 882a7b4 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63223
882a7b4 is described below
commit 882a7b48186c9f07108f4e44937f02e408e173cb
Author: Mark Thomas
AuthorDate: Thu Mar 7 10:50:05 2019 +
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63223
Include pushed streams in the count of currently open streams
Reject (ignore) a pushed stream if max streams has been reached
Correctly track the state of a pushed stream
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 8
java/org/apache/coyote/http2/Stream.java | 7 ++-
java/org/apache/coyote/http2/StreamStateMachine.java | 2 +-
webapps/docs/changelog.xml| 4
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 8e45d4f..86f0e93 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -562,6 +562,7 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
synchronized (socketWrapper) {
doWriteHeaders(stream, pushedStreamId, mimeHeaders, endOfStream,
payloadSize);
}
+stream.sentHeaders();
if (endOfStream) {
stream.sentEndOfStream();
}
@@ -1179,6 +1180,13 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
void push(Request request, Stream associatedStream) throws IOException {
+if (localSettings.getMaxConcurrentStreams() <
activeRemoteStreamCount.incrementAndGet()) {
+// If there are too many open streams, simply ignore the push
+// request.
+activeRemoteStreamCount.decrementAndGet();
+return;
+}
+
Stream pushStream;
// Synchronized since PUSH_PROMISE frames have to be sent in order.
Once
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 2c9021e..2facca3 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -586,7 +586,12 @@ public class Stream extends AbstractStream implements
HeaderEmitter {
}
-void sentEndOfStream() {
+final void sentHeaders() {
+state.sentStartOfHeaders();
+}
+
+
+final void sentEndOfStream() {
streamOutputBuffer.endOfStreamSent = true;
state.sentEndOfStream();
}
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 7b2d7be..e076395 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -181,7 +181,7 @@ public class StreamStateMachine {
Http2Error.PROTOCOL_ERROR, FrameType.PRIORITY,
FrameType.RST,
FrameType.WINDOW_UPDATE),
-RESERVED_REMOTE(false, false, true, true,
+RESERVED_REMOTE(false, true, true, true,
Http2Error.PROTOCOL_ERROR, FrameType.HEADERS,
FrameType.PRIORITY,
FrameType.RST),
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 64b2d07..29f5fc6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -164,6 +164,10 @@
Correct an error in the request validation that meant that HTTP/2 push
requests always resulted in a 400 response. (markt)
+
+63223: Correctly account for push requests when tracking
+currently active HTTP/2 streams. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 63223] org.apache.coyote.http2.Http2UpgradeHandler.pruneClosedStreams
https://bz.apache.org/bugzilla/show_bug.cgi?id=63223 Mark Thomas changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #4 from Mark Thomas --- Looking at the code, those messages should never appear if everything is being tracked correctly. I've found a couple of places where pushed streams weren't being properly tracked and I've fixed them. I've also improved the error message so, if it should be logged, it is clearer what is happening. I'm still a little concerned at the rate the messages were being logged. The most they can appear - even if the tracking has gone wrong - is once every 10 new streams created for a connection. The rate you were seeing the messages suggests a very high rate of stream creation. The access log should be able to confirm this. Should you still see this in 9.0.17 the improved log messages will help. The other thing that would be useful is a thread dump. ideally three thread dumps taken one after another. Fixed in: - trunk for 9.0.17 onwards - 8.5.x for 8.5.39 onwards -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 7.0.x updated: Fix back-port for BZ63206 / 6e3551b
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 be024ab Fix back-port for BZ63206 / 6e3551b
be024ab is described below
commit be024ab104271d88f8646139980d396deeab11af
Author: Mark Thomas
AuthorDate: Thu Mar 7 11:59:11 2019 +
Fix back-port for BZ63206 / 6e3551b
---
java/org/apache/catalina/connector/Request.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index 8a18bc9..a7877fb 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -2876,7 +2876,7 @@ implements HttpServletRequest {
if (!location.exists() && context.getCreateUploadTargets()) {
log.warn(sm.getString("coyoteRequest.uploadCreate",
-location.getAbsolutePath(),
getMappingData().wrapper.getName()));
+location.getAbsolutePath(),
((Wrapper)getMappingData().wrapper).getName()));
if (!location.mkdirs()) {
log.warn(sm.getString("coyoteRequest.uploadCreateFail",
location.getAbsolutePath()));
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rainerjung commented on issue #145: Adding volatile keywords to member variables
rainerjung commented on issue #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145#issuecomment-470501897 Yes of course, because the threads do not oby to the rules I have described. From my comment: "Changes done by thread T1 while holding any lock are visible by any other thread T2 provided that T1 releases the lock after the changes and T2 acquired it before accessing the changed data." 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] markt-asf commented on issue #145: Adding volatile keywords to member variables
markt-asf commented on issue #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145#issuecomment-470502673 The claims made above are incorrect. They fail to take account of the requirement that > However, compilers are allowed to reorder the instructions in each thread, when this **does not affect the execution of that thread in isolation**. (my emphasis) The claimed re-ordering is not possible because it changes the behaviour of the code. 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] markt-asf closed pull request #145: Adding volatile keywords to member variables
markt-asf closed pull request #145: Adding volatile keywords to member variables URL: https://github.com/apache/tomcat/pull/145 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] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470503719
package com.ydy.thread.volatile2;
public class VolatileTest2 {
public static void main(String[] args) {
Task task = new Task();
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.add();
}
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.print();
}
}
});
t1.start();
t2.start();
}
}
class Task {
private int i = 0;
private int j = 0;
private Object lock = new Object();
/**
* add
*/
public synchronized void add() {
i++;
j++;
}
public void print() {
synchronized (lock) {
if (j > i) {
// If no instruction reordering occurs, this
line of code will never run
System.out.println("Wrong result");
}
}
}
}
If no instruction reordering occurs, this line of code will never run
`System.out.println("Wrong result");`
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] y987425112 removed a comment on issue #145: Adding volatile keywords to member variables
y987425112 removed a comment on issue #145: Adding volatile keywords to member
variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470503719
package com.ydy.thread.volatile2;
public class VolatileTest2 {
public static void main(String[] args) {
Task task = new Task();
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.add();
}
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.print();
}
}
});
t1.start();
t2.start();
}
}
class Task {
private int i = 0;
private int j = 0;
private Object lock = new Object();
/**
* add
*/
public synchronized void add() {
i++;
j++;
}
public void print() {
synchronized (lock) {
if (j > i) {
// If no instruction reordering occurs, this
line of code will never run
System.out.println("Wrong result");
}
}
}
}
If no instruction reordering occurs, this line of code will never run
`System.out.println("Wrong result");`
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] y987425112 commented on issue #145: Adding volatile keywords to member variables
y987425112 commented on issue #145: Adding volatile keywords to member variables
URL: https://github.com/apache/tomcat/pull/145#issuecomment-470504073
```
package com.ydy.thread.volatile2;
public class VolatileTest2 {
public static void main(String[] args) {
Task task = new Task();
Thread t1 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.add();
}
}
});
Thread t2 = new Thread(new Runnable() {
@Override
public void run() {
// TODO Auto-generated method stub
while (true) {
task.print();
}
}
});
t1.start();
t2.start();
}
}
class Task {
private int i = 0;
private int j = 0;
private Object lock = new Object();
/**
* add
*/
public synchronized void add() {
i++;
j++;
}
public void print() {
synchronized (lock) {
if (j > i) {
// If no instruction reordering occurs, this
line of code will never run
System.out.println("Wrong result");
}
}
}
}
```
// If no instruction reordering occurs, this line of code will never run
System.out.println("Wrong result");
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
[tomcat] branch master updated: Avoid keystores with OpenSSL and regular certificates
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new e87cf37 Avoid keystores with OpenSSL and regular certificates
e87cf37 is described below
commit e87cf37c16c162db6f6c546dcfd40dcc568bb648
Author: remm
AuthorDate: Thu Mar 7 13:18:48 2019 +0100
Avoid keystores with OpenSSL and regular certificates
The new harmonization code always processes certificates through a
keystore, even if the certificates will later be simply given to
OpenSSL. The problem is that this then restricts certificates to those
that JSSE can process.
---
java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java | 14 ++
1 file changed, 14 insertions(+)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
index 3d1e0eb..514aef2 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
@@ -80,6 +80,9 @@ public class OpenSSLUtil extends SSLUtilBase {
public static X509KeyManager chooseKeyManager(KeyManager[] managers)
throws Exception {
+if (managers == null) {
+return null;
+}
for (KeyManager manager : managers) {
if (manager instanceof JSSEKeyManager) {
return (JSSEKeyManager) manager;
@@ -92,4 +95,15 @@ public class OpenSSLUtil extends SSLUtilBase {
}
throw new
IllegalStateException(sm.getString("openssl.keyManagerMissing"));
}
+
+
+@Override
+public KeyManager[] getKeyManagers() throws Exception {
+if (certificate.getCertificateFile() == null) {
+return super.getKeyManagers();
+} else {
+return null;
+}
+}
+
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated: Avoid keystores with OpenSSL and regular certificates
On 07/03/2019 12:18, r...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new e87cf37 Avoid keystores with OpenSSL and regular certificates
Thanks for finding and fixing this.
Mark
> e87cf37 is described below
>
> commit e87cf37c16c162db6f6c546dcfd40dcc568bb648
> Author: remm
> AuthorDate: Thu Mar 7 13:18:48 2019 +0100
>
> Avoid keystores with OpenSSL and regular certificates
>
> The new harmonization code always processes certificates through a
> keystore, even if the certificates will later be simply given to
> OpenSSL. The problem is that this then restricts certificates to those
> that JSSE can process.
> ---
> java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> index 3d1e0eb..514aef2 100644
> --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> @@ -80,6 +80,9 @@ public class OpenSSLUtil extends SSLUtilBase {
>
>
> public static X509KeyManager chooseKeyManager(KeyManager[] managers)
> throws Exception {
> +if (managers == null) {
> +return null;
> +}
> for (KeyManager manager : managers) {
> if (manager instanceof JSSEKeyManager) {
> return (JSSEKeyManager) manager;
> @@ -92,4 +95,15 @@ public class OpenSSLUtil extends SSLUtilBase {
> }
> throw new
> IllegalStateException(sm.getString("openssl.keyManagerMissing"));
> }
> +
> +
> +@Override
> +public KeyManager[] getKeyManagers() throws Exception {
> +if (certificate.getCertificateFile() == null) {
> +return super.getKeyManagers();
> +} else {
> +return null;
> +}
> +}
> +
> }
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated: Avoid keystores with OpenSSL and regular certificates
On Thu, Mar 7, 2019 at 1:48 PM Mark Thomas wrote:
> On 07/03/2019 12:18, r...@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new e87cf37 Avoid keystores with OpenSSL and regular certificates
>
> Thanks for finding and fixing this.
>
No problem. I'll try to improve a bit on it, then I'll port it to 8.5.
Rémy
>
> Mark
>
>
> > e87cf37 is described below
> >
> > commit e87cf37c16c162db6f6c546dcfd40dcc568bb648
> > Author: remm
> > AuthorDate: Thu Mar 7 13:18:48 2019 +0100
> >
> > Avoid keystores with OpenSSL and regular certificates
> >
> > The new harmonization code always processes certificates through a
> > keystore, even if the certificates will later be simply given to
> > OpenSSL. The problem is that this then restricts certificates to
> those
> > that JSSE can process.
> > ---
> > java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java | 14
> ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> > index 3d1e0eb..514aef2 100644
> > --- a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> > +++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
> > @@ -80,6 +80,9 @@ public class OpenSSLUtil extends SSLUtilBase {
> >
> >
> > public static X509KeyManager chooseKeyManager(KeyManager[]
> managers) throws Exception {
> > +if (managers == null) {
> > +return null;
> > +}
> > for (KeyManager manager : managers) {
> > if (manager instanceof JSSEKeyManager) {
> > return (JSSEKeyManager) manager;
> > @@ -92,4 +95,15 @@ public class OpenSSLUtil extends SSLUtilBase {
> > }
> > throw new
> IllegalStateException(sm.getString("openssl.keyManagerMissing"));
> > }
> > +
> > +
> > +@Override
> > +public KeyManager[] getKeyManagers() throws Exception {
> > +if (certificate.getCertificateFile() == null) {
> > +return super.getKeyManagers();
> > +} else {
> > +return null;
> > +}
> > +}
> > +
> > }
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
[tomcat] branch master updated: Try to process certificates using JSSE before OpenSSL
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 7b41d6e Try to process certificates using JSSE before OpenSSL
7b41d6e is described below
commit 7b41d6edaf1f37c8741a06b1ac496e7faa8d1863
Author: remm
AuthorDate: Thu Mar 7 15:02:00 2019 +0100
Try to process certificates using JSSE before OpenSSL
Add logging if there is a key manager issue at info level (also with the
exception if at debug level). For example the issue occurred with a test
config with a PKCS1 private key (so pretty old) which couldn't be
processed with the JSSE code. Although valid, the user could probably
update to something more modern and the message gives a hint.
---
.../tomcat/util/net/openssl/LocalStrings.properties | 1 +
.../apache/tomcat/util/net/openssl/OpenSSLUtil.java | 19 ---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
b/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
index ff294c6..1dca2b5 100644
--- a/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
@@ -50,6 +50,7 @@ openssl.errMakeConf=Could not create OpenSSLConf context
openssl.errorSSLCtxInit=Error initializing SSL context
openssl.keyManagerMissing=No key manager found
openssl.makeConf=Creating OpenSSLConf context
+openssl.nonJsseCertficate=The certificate [{0}] or its private key [{1}] could
not be processed using a JSSE key manager and will be given directly to OpenSSL
openssl.trustManagerMissing=No trust manager found
opensslconf.applyCommand=OpenSSLConf applying command (name [{0}], value [{1}])
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
index 514aef2..6878deb 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLUtil.java
@@ -16,6 +16,7 @@
*/
package org.apache.tomcat.util.net.openssl;
+import java.security.KeyStoreException;
import java.util.List;
import java.util.Set;
@@ -99,10 +100,22 @@ public class OpenSSLUtil extends SSLUtilBase {
@Override
public KeyManager[] getKeyManagers() throws Exception {
-if (certificate.getCertificateFile() == null) {
+try {
return super.getKeyManagers();
-} else {
-return null;
+} catch (KeyStoreException e) {
+if (certificate.getCertificateFile() != null) {
+if (log.isDebugEnabled()) {
+log.info(sm.getString("openssl.nonJsseCertficate",
+certificate.getCertificateFile(),
certificate.getCertificateKeyFile()), e);
+} else {
+log.info(sm.getString("openssl.nonJsseCertficate",
+certificate.getCertificateFile(),
certificate.getCertificateKeyFile()));
+}
+// Assume JSSE processing of the certificate failed, try again
with OpenSSL
+// without a key manager
+return null;
+}
+throw e;
}
}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
https://tomcat.apache.org/migration-X.html "View Differences"-Button returns 403/404
Hi, The "View Differences"-Button on the https://tomcat.apache.org/migration-X.html returns a error page (403 for Versions 7 and 9 and a 404 for Version8) e.g. https://tomcat.apache.org/migration-9.html refering to https://gitbox.apache.org/repos/asf?p=tomcat.git&a=blobdiff&f=conf%2Fcatalina.policy&hpb=9.0.14&hb=9.0.16 Hope this is the right list to communicate this. Regards Chris - 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 (882a7b4 -> c98a643)
This is an automated email from the ASF dual-hosted git repository. remm pushed a change to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 882a7b4 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63223 new 25fb200 Avoid keystores with OpenSSL and regular certificates new c98a643 Try to process certificates using JSSE before OpenSSL The 19089 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: .../util/net/openssl/LocalStrings.properties | 1 + .../tomcat/util/net/openssl/OpenSSLUtil.java | 27 ++ 2 files changed, 28 insertions(+) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] philwebb commented on issue #142: Defer creation of encodingToCharsetCache
philwebb commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470546205 > Besides the 31ms startup time, it's still worse in every way compared to the current code. I'm really not sure what I'm supposed to do with a comment like that. As someone who's just trying to help contribute a fix for an identified problem, it really doesn't make me feel very welcome :( I'm happy to have another go at fixing this if you can provide something more constructive than "it's worse in every way". Having the current code call `availableCharsets()` which is specifically documented with "may cause time-consuming disk or network I/O operations to occur" in a static initializer with no way to opt-out doesn't seem ideal either. AFAICT this is something that neither Jetty or Undertow do, so I don't believe it's a problem that _can't_ be solved in another way. I understand that Tomcat has many users that start it only once, and aren't really bothered by these kind of optimizations. To them 30ms and not loading an extra 50-60 classes isn't really a bit deal. However, I strongly believe that there is another class of user that want to run Embedded Tomcat and will care deeply about this kind of thing. If the type of change I'm suggesting is never going to be acceptable, perhaps I can change my approach. Would it be better if I tried to make the charset lookup code plugabble? That way, the existing code would not need to change, but users who do care about this issue could replace the lookup code with something else. 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
Re: https://tomcat.apache.org/migration-X.html "View Differences"-Button returns 403/404
On 07/03/2019 14:03, Christoph Hanke wrote: > Hi, > > The "View Differences"-Button on the > https://tomcat.apache.org/migration-X.html returns a error page (403 for > Versions 7 and 9 and a 404 for Version8) > > e.g. > > https://tomcat.apache.org/migration-9.html refering to > > https://gitbox.apache.org/repos/asf?p=tomcat.git&a=blobdiff&f=conf%2Fcatalina.policy&hpb=9.0.14&hb=9.0.16 > > Hope this is the right list to communicate this. It is. Either I messed up the migration or the counter-measures infra recently deployed to tackle abusive traffic have blocked these requests. I'll take a look. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
buildbot success in on tomcat-7-trunk
The Buildbot has detected a restored build on builder tomcat-7-trunk while building tomcat. Full details are available at: https://ci.apache.org/builders/tomcat-7-trunk/builds/1281 Buildbot URL: https://ci.apache.org/ Buildslave for this Build: silvanus_ubuntu Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-7-commit' triggered this build Build Source Stamp: [branch 7.0.x] be024ab104271d88f8646139980d396deeab11af Blamelist: Mark Thomas Build succeeded! Sincerely, -The Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: https://tomcat.apache.org/migration-X.html "View Differences"-Button returns 403/404
On 07/03/2019 14:38, Mark Thomas wrote: > On 07/03/2019 14:03, Christoph Hanke wrote: >> Hi, >> >> The "View Differences"-Button on the >> https://tomcat.apache.org/migration-X.html returns a error page (403 for >> Versions 7 and 9 and a 404 for Version8) >> >> e.g. >> >> https://tomcat.apache.org/migration-9.html refering to >> >> https://gitbox.apache.org/repos/asf?p=tomcat.git&a=blobdiff&f=conf%2Fcatalina.policy&hpb=9.0.14&hb=9.0.16 >> >> Hope this is the right list to communicate this. > > It is. > > Either I messed up the migration or the counter-measures infra recently > deployed to tackle abusive traffic have blocked these requests. This is a result of the counter-measures infra have deployed to tackle abusive traffic. I have spoken with the infra team and they are expected to be temporary so normal service should be resumed at some point. Until then, I recommend the command line approach. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #142: Defer creation of encodingToCharsetCache
rmaucher commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470602748 You submitted three items, and indeed I am not convinced by this one. Looking back to the old BZs, there was also there the conclusion that the current code is the best solution for the general Tomcat use case. However, there is the "but I only want UTF-8" argument, in which case the solution is to hardcode UTF-8 and default to Charset.forName for the rest. I think system property configuration is supposed to be removed, but it would be a good solution for this pluggability (it's global, no one really actually needs it, etc). 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] markt-asf commented on issue #142: Defer creation of encodingToCharsetCache
markt-asf commented on issue #142: Defer creation of encodingToCharsetCache URL: https://github.com/apache/tomcat/pull/142#issuecomment-470607054 No. Not an system property ;). Seriously, I think there is a way that we can improve start-up time without impacting the vast majority of users. Phil's patch is heading in the general direction I was thinking. As ever, there are likely to be trade-offs involved. I hope to be able to have some hard numbers on which to base that discussion soon(ish). 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
Re: Tomcat Site Redesign
Konstantin, Thank you for your feedback: On Thu, Mar 7, 2019 at 3:29 AM Konstantin Kolinko wrote: > пн, 4 мар. 2019 г. в 09:02, Igal Sapir : > > > > I have uploaded the Tomcat site redesign to a temporary location for > review: > > http://people.apache.org/~isapir/mockups/tomcat-site/ > > 1. Overall: Thank you for your effort, but I do not like it. At all. > Fair enough, but I wish you had expressed that two months ago when we were going over the layout: https://www.mail-archive.com/dev@tomcat.apache.org/msg130453.html > > 2. Impressions: > > 1) The text is not as readable as the existing site. > It is hard to read it because of color. It blurs with background. > The color can be easily changed. It is common to use a dark gray color rather than black though as it's easier on the eyes and considered more readable. There was a discussion about the font size, which prompted me to add a slider at the bottom that allows you to change the font size. > > 2) Advertisement steals important part of the page. > Nobody likes advertisements, you know? Especially when it is thrown in > one's face. > > Who are our clients? Why people come to the site and what do they expect > to see? > Newspapers like to throw ads like that and hide the real text, but > they make money from that and people tolerate them (to a certain > degree). > The ad for ApacheCon is merely a placeholder to show how we can place prominent news on the side on larger screens, and collapse it above the body on smaller screens, i.e. phones. I'm not sure on what screen you saw that. It was added per Mark's feedback. It can be moved or removed. > > 3) I do not like menu at the top. Those were popular some time ago, > but I think that is a diminishing trend. > > (What is a concept behind such design element? It is not a material > element, not something from real world. How do people operate with > it?) > This is a mobile-first responsive layout that changes according to your screen size. If you tell me what size screen you saw it on then I would understand better what you mean. It utilizes the Bootstrap 4 framework and is very trendy. Again, I wish you had expressed that 2 months ago. > > It does not make sense that the menu is flying at top when I scroll. > This menu occupies precious space on the screen. A page title flying > like that makes more sense. > It doesn't have to be "sticky". That can be changed easily. > > 4) The "Apache Tomcat" in menu line is too prominent in font and > color. It steals attention from the real page title and hurts eyes. > Fonts and colors can be changed. I had to work with the constraints of ASF guidelines. There is a longer discussion about this in the thread mentioned above. > > 3. Technically: > 1) Printing =?. > > It is not printer-friendly! > Browser -> Print preview. > I see that it does not fit a paper page. > That can be fixed > > 2) Accessibility =?. > (Tab key navigation with keyboard. Accessing menus. Accessing links). > > Keyboard. How does one operate that menu with a keyboard? > Keyboard shortcuts can be added > > Mouse. Why do I need two clicks to open anything through that menu? > I can make the menu open on hover, but please keep in mind that not every device has a "hover" capability. For example on mobile devices you don't have that. > > 3) Titles and dates. > > "2019-02-21 Tomcat 7.0.93 Released" > > The dates should not be spelled like that if they are part of a > sentence. How do you read that sentence aloud (in English)? > > (Actually, the dates should not be in the sentence. They are a > separate auxiliary piece of information). > That is part of the body of the pages and was not modified. It has been like that on the Tomcat website for a long time now. > > 4) Search field (in menu bar): > > It is too small for any real text. > The search field can expand wider, but on tablets and phones we are limited in space. Thank you, Igal
Re: Tomcat Site Redesign
I made the text darker (#333) and pulled the date to the right as on the current site. On Thu, Mar 7, 2019 at 9:50 AM Igal Sapir wrote: > Konstantin, > > Thank you for your feedback: > > On Thu, Mar 7, 2019 at 3:29 AM Konstantin Kolinko > wrote: > >> пн, 4 мар. 2019 г. в 09:02, Igal Sapir : >> > >> > I have uploaded the Tomcat site redesign to a temporary location for >> review: >> > http://people.apache.org/~isapir/mockups/tomcat-site/ >> >> 1. Overall: Thank you for your effort, but I do not like it. At all. >> > > Fair enough, but I wish you had expressed that two months ago when we were > going over the layout: > https://www.mail-archive.com/dev@tomcat.apache.org/msg130453.html > > >> >> 2. Impressions: >> >> 1) The text is not as readable as the existing site. >> It is hard to read it because of color. It blurs with background. >> > > The color can be easily changed. It is common to use a dark gray color > rather than black though as it's easier on the eyes and considered more > readable. There was a discussion about the font size, which prompted me to > add a slider at the bottom that allows you to change the font size. > > >> >> 2) Advertisement steals important part of the page. >> Nobody likes advertisements, you know? Especially when it is thrown in >> one's face. >> >> Who are our clients? Why people come to the site and what do they expect >> to see? >> Newspapers like to throw ads like that and hide the real text, but >> they make money from that and people tolerate them (to a certain >> degree). >> > > The ad for ApacheCon is merely a placeholder to show how we can place > prominent news on the side on larger screens, and collapse it above the > body on smaller screens, i.e. phones. I'm not sure on what screen you saw > that. > > It was added per Mark's feedback. > > It can be moved or removed. > > >> >> 3) I do not like menu at the top. Those were popular some time ago, >> but I think that is a diminishing trend. >> >> (What is a concept behind such design element? It is not a material >> element, not something from real world. How do people operate with >> it?) >> > > This is a mobile-first responsive layout that changes according to your > screen size. If you tell me what size screen you saw it on then I would > understand better what you mean. > > It utilizes the Bootstrap 4 framework and is very trendy. > > Again, I wish you had expressed that 2 months ago. > > >> >> It does not make sense that the menu is flying at top when I scroll. >> This menu occupies precious space on the screen. A page title flying >> like that makes more sense. >> > > It doesn't have to be "sticky". That can be changed easily. > > >> >> 4) The "Apache Tomcat" in menu line is too prominent in font and >> color. It steals attention from the real page title and hurts eyes. >> > > Fonts and colors can be changed. > > I had to work with the constraints of ASF guidelines. There is a longer > discussion about this in the thread mentioned above. > > >> >> 3. Technically: >> 1) Printing =?. >> >> It is not printer-friendly! >> Browser -> Print preview. >> I see that it does not fit a paper page. >> > > That can be fixed > > >> >> 2) Accessibility =?. >> (Tab key navigation with keyboard. Accessing menus. Accessing links). >> >> Keyboard. How does one operate that menu with a keyboard? >> > > Keyboard shortcuts can be added > > >> >> Mouse. Why do I need two clicks to open anything through that menu? >> > > I can make the menu open on hover, but please keep in mind that not every > device has a "hover" capability. For example on mobile devices you don't > have that. > > >> >> 3) Titles and dates. >> >> "2019-02-21 Tomcat 7.0.93 Released" >> >> The dates should not be spelled like that if they are part of a >> sentence. How do you read that sentence aloud (in English)? >> >> (Actually, the dates should not be in the sentence. They are a >> separate auxiliary piece of information). >> > > That is part of the body of the pages and was not modified. It has been > like that on the Tomcat website for a long time now. > > >> >> 4) Search field (in menu bar): >> >> It is too small for any real text. >> > > The search field can expand wider, but on tablets and phones we are > limited in space. > > Thank you, > > Igal > >
[GitHub] [tomcat] markt-asf closed pull request #143: Apply deduplication to certain loaded and created Strings
markt-asf closed pull request #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143 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] markt-asf commented on issue #143: Apply deduplication to certain loaded and created Strings
markt-asf commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470662734 I've been running YourKit with allocation tracking enabled and tracking each of the duplicated strings to the root cause. I started with the biggest duplicates and stopped somewhere around 700 bytes / per duplicated String. I ended up with a slightly different patch that covers the same duplicates and a few more. Start-up time appears to remain unaffected. Total saving ~245k. I'm going to close this PR but I'll credit you in the commit message. 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
[tomcat] branch master updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
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 5ccd4eb Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
5ccd4eb is described below
commit 5ccd4eb4017ceb5fcabd1a8a11b81ca664f1d852
Author: Mark Thomas
AuthorDate: Thu Mar 7 19:36:43 2019 +
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
Use intern() as suggested by Phillip Webb to reduce memory wasted due to
string duplication. Saves ~254k on a clean install.
Thanks to YourKit for helping to track these down.
---
java/org/apache/catalina/util/LifecycleMBeanBase.java | 2 +-
java/org/apache/tomcat/util/digester/CallMethodRule.java | 2 +-
java/org/apache/tomcat/util/digester/Digester.java| 8 ++--
java/org/apache/tomcat/util/modeler/ManagedBean.java | 4 ++--
.../modeler/modules/MbeansDescriptorsIntrospectionSource.java | 4 ++--
webapps/docs/changelog.xml| 7 +++
6 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/util/LifecycleMBeanBase.java
b/java/org/apache/catalina/util/LifecycleMBeanBase.java
index 49770a1..def6453 100644
--- a/java/org/apache/catalina/util/LifecycleMBeanBase.java
+++ b/java/org/apache/catalina/util/LifecycleMBeanBase.java
@@ -271,7 +271,7 @@ public abstract class LifecycleMBeanBase extends
LifecycleBase
this.mserver = server;
this.oname = name;
-this.domain = name.getDomain();
+this.domain = name.getDomain().intern();
return oname;
}
diff --git a/java/org/apache/tomcat/util/digester/CallMethodRule.java
b/java/org/apache/tomcat/util/digester/CallMethodRule.java
index 7651776..288312e 100644
--- a/java/org/apache/tomcat/util/digester/CallMethodRule.java
+++ b/java/org/apache/tomcat/util/digester/CallMethodRule.java
@@ -257,7 +257,7 @@ public class CallMethodRule extends Rule {
throws Exception {
if (paramCount == 0) {
-this.bodyText = bodyText.trim();
+this.bodyText = bodyText.trim().intern();
}
}
diff --git a/java/org/apache/tomcat/util/digester/Digester.java
b/java/org/apache/tomcat/util/digester/Digester.java
index 1bb2d20..3b0f35e 100644
--- a/java/org/apache/tomcat/util/digester/Digester.java
+++ b/java/org/apache/tomcat/util/digester/Digester.java
@@ -903,7 +903,7 @@ public class Digester extends DefaultHandler2 {
// Fire "body" events for all relevant rules
List rules = matches.pop();
if ((rules != null) && (rules.size() > 0)) {
-String bodyText = this.bodyText.toString();
+String bodyText = this.bodyText.toString().intern();
for (int i = 0; i < rules.size(); i++) {
try {
Rule rule = rules.get(i);
@@ -1917,17 +1917,13 @@ public class Digester extends DefaultHandler2 {
for (int i = 0; i < nAttributes; ++i) {
String value = newAttrs.getValue(i);
try {
-String newValue = IntrospectionUtils.replaceProperties(value,
null, source);
-if (value != newValue) {
-newAttrs.setValue(i, newValue);
-}
+newAttrs.setValue(i,
IntrospectionUtils.replaceProperties(value, null, source).intern());
} catch (Exception e) {
log.warn(sm.getString("digester.failedToUpdateAttributes",
newAttrs.getLocalName(i), value), e);
}
}
return newAttrs;
-
}
diff --git a/java/org/apache/tomcat/util/modeler/ManagedBean.java
b/java/org/apache/tomcat/util/modeler/ManagedBean.java
index 49af85c..31e3d20 100644
--- a/java/org/apache/tomcat/util/modeler/ManagedBean.java
+++ b/java/org/apache/tomcat/util/modeler/ManagedBean.java
@@ -562,7 +562,7 @@ public class ManagedBean implements java.io.Serializable {
StringUtils.join(operation.getSignature(), ',', (x) -> x.getType(),
key);
key.append(')');
-return key.toString();
+return key.toString().intern();
}
@@ -572,6 +572,6 @@ public class ManagedBean implements java.io.Serializable {
StringUtils.join(parameterTypes, ',', key);
key.append(')');
-return key.toString();
+return key.toString().intern();
}
}
diff --git
a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
index 1a68c5e..630bde3 100644
---
a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
+++
b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
@@ -348,8 +348,8 @@ public class MbeansDescriptorsIn
[GitHub] [tomcat] philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings
philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470667817 @rmannibucau I've opened another issue for that https://bz.apache.org/bugzilla/show_bug.cgi?id=63237. If I get a second I'll try some experiments. 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] philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings
philwebb commented on issue #143: Apply deduplication to certain loaded and created Strings URL: https://github.com/apache/tomcat/pull/143#issuecomment-470668078 @markt-asf Thanks Mark! 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
[tomcat] branch master updated: SSL engines need additional buffer space on input
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 9e4932c SSL engines need additional buffer space on input
9e4932c is described below
commit 9e4932c7f051af389429cc754d913db325d8aed1
Author: remm
AuthorDate: Thu Mar 7 20:49:56 2019 +0100
SSL engines need additional buffer space on input
Ideally, I will refactor this to make it transparent, most likely the
main input buffer can be used as an extra overflow buffer. The value
comes from JSSE, while OpenSSL uses 16KB. Same behavior for both
engines.
---
java/org/apache/coyote/http2/Http2AsyncParser.java | 6 +-
webapps/docs/changelog.xml | 3 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java
b/java/org/apache/coyote/http2/Http2AsyncParser.java
index 92531bf..7bd24df 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -42,7 +42,11 @@ class Http2AsyncParser extends Http2Parser {
socketWrapper.getSocketBufferHandler().expand(input.getMaxFrameSize());
this.upgradeHandler = upgradeHandler;
header = ByteBuffer.allocate(9);
-framePaylod = ByteBuffer.allocate(input.getMaxFrameSize());
+int frameBufferSize = input.getMaxFrameSize();
+if (socketWrapper.isSecure()) {
+frameBufferSize += 16676;
+}
+framePaylod = ByteBuffer.allocate(frameBufferSize);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 47b7b59..db7a05f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -134,6 +134,9 @@
63223: Correctly account for push requests when tracking
currently active HTTP/2 streams. (markt)
+
+Ensure enough buffer space when using TLS with NIO2 and HTTP/2. (remm)
+
-
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=63236
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 2e13d46 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
2e13d46 is described below
commit 2e13d4653c405e4ed25c0824963faa18f4d6c84f
Author: Mark Thomas
AuthorDate: Thu Mar 7 19:36:43 2019 +
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
Use intern() as suggested by Phillip Webb to reduce memory wasted due to
string duplication. Saves ~254k on a clean install.
Thanks to YourKit for helping to track these down.
---
java/org/apache/catalina/util/LifecycleMBeanBase.java | 2 +-
java/org/apache/tomcat/util/digester/CallMethodRule.java | 2 +-
java/org/apache/tomcat/util/digester/Digester.java| 8 ++--
java/org/apache/tomcat/util/modeler/ManagedBean.java | 4 ++--
.../modeler/modules/MbeansDescriptorsIntrospectionSource.java | 4 ++--
webapps/docs/changelog.xml| 7 +++
6 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/util/LifecycleMBeanBase.java
b/java/org/apache/catalina/util/LifecycleMBeanBase.java
index 1386cd6..417015d 100644
--- a/java/org/apache/catalina/util/LifecycleMBeanBase.java
+++ b/java/org/apache/catalina/util/LifecycleMBeanBase.java
@@ -241,7 +241,7 @@ public abstract class LifecycleMBeanBase extends
LifecycleBase
this.mserver = server;
this.oname = name;
-this.domain = name.getDomain();
+this.domain = name.getDomain().intern();
return oname;
}
diff --git a/java/org/apache/tomcat/util/digester/CallMethodRule.java
b/java/org/apache/tomcat/util/digester/CallMethodRule.java
index 10ffc84..9221aae 100644
--- a/java/org/apache/tomcat/util/digester/CallMethodRule.java
+++ b/java/org/apache/tomcat/util/digester/CallMethodRule.java
@@ -303,7 +303,7 @@ public class CallMethodRule extends Rule {
throws Exception {
if (paramCount == 0) {
-this.bodyText = bodyText.trim();
+this.bodyText = bodyText.trim().intern();
}
}
diff --git a/java/org/apache/tomcat/util/digester/Digester.java
b/java/org/apache/tomcat/util/digester/Digester.java
index 093ef03..f0345a5 100644
--- a/java/org/apache/tomcat/util/digester/Digester.java
+++ b/java/org/apache/tomcat/util/digester/Digester.java
@@ -977,7 +977,7 @@ public class Digester extends DefaultHandler2 {
// Fire "body" events for all relevant rules
List rules = matches.pop();
if ((rules != null) && (rules.size() > 0)) {
-String bodyText = this.bodyText.toString();
+String bodyText = this.bodyText.toString().intern();
for (int i = 0; i < rules.size(); i++) {
try {
Rule rule = rules.get(i);
@@ -2030,17 +2030,13 @@ public class Digester extends DefaultHandler2 {
for (int i = 0; i < nAttributes; ++i) {
String value = newAttrs.getValue(i);
try {
-String newValue = IntrospectionUtils.replaceProperties(value,
null, source);
-if (value != newValue) {
-newAttrs.setValue(i, newValue);
-}
+newAttrs.setValue(i,
IntrospectionUtils.replaceProperties(value, null, source).intern());
} catch (Exception e) {
log.warn(sm.getString("digester.failedToUpdateAttributes",
newAttrs.getLocalName(i), value), e);
}
}
return newAttrs;
-
}
diff --git a/java/org/apache/tomcat/util/modeler/ManagedBean.java
b/java/org/apache/tomcat/util/modeler/ManagedBean.java
index 6162b7e..8b8e5ed 100644
--- a/java/org/apache/tomcat/util/modeler/ManagedBean.java
+++ b/java/org/apache/tomcat/util/modeler/ManagedBean.java
@@ -571,7 +571,7 @@ public class ManagedBean implements java.io.Serializable {
StringUtils.join(operation.getSignature(), ',', new
Function() {
@Override public String apply(ParameterInfo t) { return
t.getType(); }}, key);
key.append(')');
-return key.toString();
+return key.toString().intern();
}
@@ -581,6 +581,6 @@ public class ManagedBean implements java.io.Serializable {
StringUtils.join(parameterTypes, ',', key);
key.append(')');
-return key.toString();
+return key.toString().intern();
}
}
diff --git
a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
index f3637f4..b19f1fc 100644
---
a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java
+++
b/java/org/apache/tomcat/util/modeler/modules/MbeansDescript
[tomcat] branch 7.0.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
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 25b1c2c Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
25b1c2c is described below
commit 25b1c2c5b98d5cc2581a8a35249b2796558586be
Author: Mark Thomas
AuthorDate: Thu Mar 7 19:36:43 2019 +
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
Use intern() as suggested by Phillip Webb to reduce memory wasted due to
string duplication. Saves ~254k on a clean install.
Thanks to YourKit for helping to track these down.
---
.../apache/catalina/util/LifecycleMBeanBase.java | 2 +-
.../tomcat/util/digester/CallMethodRule.java | 2 +-
java/org/apache/tomcat/util/digester/Digester.java | 233 ++---
.../apache/tomcat/util/modeler/ManagedBean.java| 50 ++---
.../MbeansDescriptorsIntrospectionSource.java | 4 +-
webapps/docs/changelog.xml | 7 +
6 files changed, 150 insertions(+), 148 deletions(-)
diff --git a/java/org/apache/catalina/util/LifecycleMBeanBase.java
b/java/org/apache/catalina/util/LifecycleMBeanBase.java
index 2939d92..4ff80d5 100644
--- a/java/org/apache/catalina/util/LifecycleMBeanBase.java
+++ b/java/org/apache/catalina/util/LifecycleMBeanBase.java
@@ -238,7 +238,7 @@ public abstract class LifecycleMBeanBase extends
LifecycleBase
this.mserver = server;
this.oname = name;
-this.domain = name.getDomain();
+this.domain = name.getDomain().intern();
return oname;
}
diff --git a/java/org/apache/tomcat/util/digester/CallMethodRule.java
b/java/org/apache/tomcat/util/digester/CallMethodRule.java
index 4350ecc..8da692a 100644
--- a/java/org/apache/tomcat/util/digester/CallMethodRule.java
+++ b/java/org/apache/tomcat/util/digester/CallMethodRule.java
@@ -404,7 +404,7 @@ public class CallMethodRule extends Rule {
throws Exception {
if (paramCount == 0) {
-this.bodyText = bodyText.trim();
+this.bodyText = bodyText.trim().intern();
}
}
diff --git a/java/org/apache/tomcat/util/digester/Digester.java
b/java/org/apache/tomcat/util/digester/Digester.java
index dc92d90..f427b5b 100644
--- a/java/org/apache/tomcat/util/digester/Digester.java
+++ b/java/org/apache/tomcat/util/digester/Digester.java
@@ -5,15 +5,15 @@
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
- */
+ */
package org.apache.tomcat.util.digester;
@@ -200,12 +200,12 @@ public class Digester extends DefaultHandler2 {
* in the input is entered, the matching rules are pushed onto this
* stack. After the end tag is reached, the matches are popped again.
* The depth of is stack is therefore exactly the same as the current
- * "nesting" level of the input xml.
+ * "nesting" level of the input xml.
*
* @since 1.6
*/
protected ArrayStack> matches = new ArrayStack>(10);
-
+
/**
* The class loader to use for instantiating application objects.
* If not specified, the context class loader, or the class loader
@@ -225,7 +225,7 @@ public class Digester extends DefaultHandler2 {
* The EntityResolver used by the SAX parser. By default it use this class
*/
protected EntityResolver entityResolver;
-
+
/**
* The URLs of entityValidator that have been registered, keyed by the
public
* identifier that corresponds.
@@ -341,7 +341,7 @@ public class Digester extends DefaultHandler2 {
*/
protected boolean rulesValidation = false;
-
+
/**
* Fake attributes map (attributes are often used for object creation).
*/
@@ -360,12 +360,12 @@ public class Digester extends DefaultHandler2 {
*/
protected Log saxLog =
LogFactory.getLog("org.apache.tomcat.util.digester.Digester.sax");
-
-
+
+
/** Stacks used for interrule communication, indexed by name String */
private HashMap> stacksByName =
new HashMap>();
-
+
// - Properties
/**
@@ -376,7 +376,7 @@ public class Digester extends DefaultHandler2 {
* @param prefix Prefix to look up
*/
public String findNamespaceURI(String prefix) {
-
[Bug 63236] Reduce duplicate Strings
https://bz.apache.org/bugzilla/show_bug.cgi?id=63236 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #2 from Mark Thomas --- Fixed in: - trunk for 9.0.17 onwards - 8.5.x for 8.5.39 onwards - 7.0.x for 7.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
