You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/03/11 16:41:18 UTC
[tomcat] 02/05: Refactoring - align 8.5.x with 9.0.x/10.0.x
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
commit 302aab0c48c49328999ebe72c30974fa4d4706b9
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Mar 11 13:46:35 2021 +0000
Refactoring - align 8.5.x with 9.0.x/10.0.x
Mostly removing debug code from tests and a small amount of alignment
---
java/org/apache/coyote/http2/ByteUtil.java | 27 ++---
java/org/apache/coyote/http2/Stream.java | 8 +-
test/org/apache/coyote/http2/Http2TestBase.java | 9 +-
test/org/apache/coyote/http2/TestHttp2Limits.java | 1 -
.../apache/coyote/http2/TestHttp2Section_5_1.java | 119 ++++++++++-----------
.../apache/coyote/http2/TestHttp2Section_5_3.java | 2 +-
.../apache/coyote/http2/TestHttp2Section_6_1.java | 61 +++++------
.../apache/coyote/http2/TestHttp2Section_6_9.java | 2 +-
8 files changed, 103 insertions(+), 126 deletions(-)
diff --git a/java/org/apache/coyote/http2/ByteUtil.java b/java/org/apache/coyote/http2/ByteUtil.java
index 0be3603..d1e3060 100644
--- a/java/org/apache/coyote/http2/ByteUtil.java
+++ b/java/org/apache/coyote/http2/ByteUtil.java
@@ -19,25 +19,25 @@ package org.apache.coyote.http2;
/**
* Utility class for extracting values from byte arrays.
*/
-public class ByteUtil {
+class ByteUtil {
private ByteUtil() {
// Hide default constructor
}
- public static boolean isBit7Set(byte input) {
+ static boolean isBit7Set(byte input) {
return (input & 0x80) != 0;
}
- public static int get31Bits(byte[] input, int firstByte) {
+ static int get31Bits(byte[] input, int firstByte) {
return ((input[firstByte] & 0x7F) << 24) + ((input[firstByte + 1] & 0xFF) << 16) +
((input[firstByte + 2] & 0xFF) << 8) + (input[firstByte + 3] & 0xFF);
}
- public static void set31Bits(byte[] output, int firstByte, int value) {
+ static void set31Bits(byte[] output, int firstByte, int value) {
output[firstByte] = (byte) ((value & 0x7F000000) >> 24);
output[firstByte + 1] = (byte) ((value & 0xFF0000) >> 16);
output[firstByte + 2] = (byte) ((value & 0xFF00) >> 8);
@@ -45,47 +45,42 @@ public class ByteUtil {
}
- public static int getOneByte(byte[] input, int pos) {
+ static int getOneByte(byte[] input, int pos) {
return (input[pos] & 0xFF);
}
- public static int getTwoBytes(byte[] input, int firstByte) {
+ static int getTwoBytes(byte[] input, int firstByte) {
return ((input[firstByte] & 0xFF) << 8) + (input[firstByte + 1] & 0xFF);
}
- public static int getThreeBytes(byte[] input, int firstByte) {
+ static int getThreeBytes(byte[] input, int firstByte) {
return ((input[firstByte] & 0xFF) << 16) + ((input[firstByte + 1] & 0xFF) << 8) +
(input[firstByte + 2] & 0xFF);
}
- public static void setOneBytes(byte[] output, int firstByte, int value) {
- output[firstByte] = (byte) (value & 0xFF);
- }
-
-
- public static void setTwoBytes(byte[] output, int firstByte, int value) {
+ static void setTwoBytes(byte[] output, int firstByte, int value) {
output[firstByte] = (byte) ((value & 0xFF00) >> 8);
output[firstByte + 1] = (byte) (value & 0xFF);
}
- public static void setThreeBytes(byte[] output, int firstByte, int value) {
+ static void setThreeBytes(byte[] output, int firstByte, int value) {
output[firstByte] = (byte) ((value & 0xFF0000) >> 16);
output[firstByte + 1] = (byte) ((value & 0xFF00) >> 8);
output[firstByte + 2] = (byte) (value & 0xFF);
}
- public static long getFourBytes(byte[] input, int firstByte) {
+ static long getFourBytes(byte[] input, int firstByte) {
return ((long)(input[firstByte] & 0xFF) << 24) + ((input[firstByte + 1] & 0xFF) << 16) +
((input[firstByte + 2] & 0xFF) << 8) + (input[firstByte + 3] & 0xFF);
}
- public static void setFourBytes(byte[] output, int firstByte, long value) {
+ static void setFourBytes(byte[] output, int firstByte, long value) {
output[firstByte] = (byte) ((value & 0xFF000000) >> 24);
output[firstByte + 1] = (byte) ((value & 0xFF0000) >> 16);
output[firstByte + 2] = (byte) ((value & 0xFF00) >> 8);
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 1018ff2..8069517 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -651,7 +651,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
- boolean isPushSupported() {
+ final boolean isPushSupported() {
return handler.getRemoteSettings().getEnablePush();
}
@@ -784,7 +784,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
@Override
- public synchronized int doWrite(ByteBuffer chunk) throws IOException {
+ public final synchronized int doWrite(ByteBuffer chunk) throws IOException {
if (closed) {
throw new IllegalStateException(
sm.getString("stream.closed", getConnectionId(), getIdAsString()));
@@ -931,10 +931,6 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
}
- public boolean isClosed() {
- return closed;
- }
-
/**
* @return <code>true</code> if it is certain that the associated
* response has no body.
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index 39ce41f..6db8263 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -793,7 +793,7 @@ public abstract class Http2TestBase extends TomcatBaseTest {
pingHeader[3] = FrameType.PING.getIdByte();
// Flags
if (ack) {
- ByteUtil.setOneBytes(pingHeader, 4, 0x01);
+ setOneBytes(pingHeader, 4, 0x01);
}
// Stream
ByteUtil.set31Bits(pingHeader, 5, streamId);
@@ -868,7 +868,7 @@ public abstract class Http2TestBase extends TomcatBaseTest {
// Payload
ByteUtil.set31Bits(priorityFrame, 9, streamDependencyId);
- ByteUtil.setOneBytes(priorityFrame, 13, weight);
+ setOneBytes(priorityFrame, 13, weight);
os.write(priorityFrame);
os.flush();
@@ -946,6 +946,11 @@ public abstract class Http2TestBase extends TomcatBaseTest {
}
+ static void setOneBytes(byte[] output, int firstByte, int value) {
+ output[firstByte] = (byte) (value & 0xFF);
+ }
+
+
private static class TestInput implements Http2Parser.Input {
private final InputStream is;
diff --git a/test/org/apache/coyote/http2/TestHttp2Limits.java b/test/org/apache/coyote/http2/TestHttp2Limits.java
index 1ef8a29..07ff7ed 100644
--- a/test/org/apache/coyote/http2/TestHttp2Limits.java
+++ b/test/org/apache/coyote/http2/TestHttp2Limits.java
@@ -143,7 +143,6 @@ public class TestHttp2Limits extends Http2TestBase {
// Bug 60232
doTestHeaderLimits(1, 12*1024, 1024, FailureMode.STREAM_RESET);
-
output.clearTrace();
sendSimpleGetRequest(5);
parser.readFrame(true);
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
index f9b73b4..4a8b357 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
@@ -17,8 +17,6 @@
package org.apache.coyote.http2;
import java.nio.ByteBuffer;
-import java.util.logging.Level;
-import java.util.logging.LogManager;
import org.junit.Assert;
import org.junit.Test;
@@ -264,77 +262,68 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
@Test
public void testExceedMaxActiveStreams02() throws Exception {
+ // http2Connect() - modified
+ enableHttp2(1);
+ configureAndStartWebApplication();
+ openClientConnection();
+ doHttpUpgrade();
+ sendClientPreface();
- LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.ALL);
- try {
- // temporary debug logging to investigate intermittent CI failures
+ // validateHttp2InitialResponse() - modified
+ parser.readFrame(true);
+ parser.readFrame(true);
+ parser.readFrame(true);
+ parser.readFrame(true);
+ parser.readFrame(true);
+ Assert.assertEquals("0-Settings-[3]-[1]\n" +
+ "0-Settings-End\n" +
+ "0-Settings-Ack\n" +
+ "0-Ping-[0,0,0,0,0,0,0,1]\n" +
+ getSimpleResponseTrace(1)
+ , output.getTrace());
+ output.clearTrace();
- // http2Connect() - modified
- enableHttp2(1);
- configureAndStartWebApplication();
- openClientConnection();
- doHttpUpgrade();
- sendClientPreface();
+ sendLargeGetRequest(3);
- // validateHttp2InitialResponse() - modified
- parser.readFrame(true);
- parser.readFrame(true);
- parser.readFrame(true);
- parser.readFrame(true);
- parser.readFrame(true);
+ sendSimpleGetRequest(5);
- Assert.assertEquals("0-Settings-[3]-[1]\n" +
- "0-Settings-End\n" +
- "0-Settings-Ack\n" +
- "0-Ping-[0,0,0,0,0,0,0,1]\n" +
- getSimpleResponseTrace(1)
- , output.getTrace());
- output.clearTrace();
-
- sendLargeGetRequest(3);
-
- sendSimpleGetRequest(5);
-
- // Default connection window size is 64k-1.
- // Initial request will have used 8k leaving 56k-1.
- // Stream window will be 64k-1.
- // Expecting
- // 1 * headers
- // 56k-1 of body (7 * ~8k)
- // 1 * error
- // for a total of 9 frames (could be in any order)
- for (int i = 0; i < 9; i++) {
- parser.readFrame(true);
- }
-
- Assert.assertTrue(output.getTrace(),
- output.getTrace().contains("5-RST-[" +
- Http2Error.REFUSED_STREAM.getCode() + "]"));
- output.clearTrace();
-
- // Connection window is zero.
- // Stream window is 8k
-
- // Reset stream 3 (client cancel)
- sendRst(3, Http2Error.NO_ERROR.getCode());
- // Client reset triggers a write error which in turn triggers a server
- // reset
+ // Default connection window size is 64k-1.
+ // Initial request will have used 8k leaving 56k-1.
+ // Stream window will be 64k-1.
+ // Expecting
+ // 1 * headers
+ // 56k-1 of body (7 * ~8k)
+ // 1 * error
+ // for a total of 9 frames (could be in any order)
+ for (int i = 0; i < 9; i++) {
parser.readFrame(true);
- Assert.assertEquals("3-RST-[5]\n", output.getTrace());
- output.clearTrace();
+ }
- // Open up the connection window.
- sendWindowUpdate(0, (1 << 31) - 2);
+ Assert.assertTrue(output.getTrace(),
+ output.getTrace().contains("5-RST-[" +
+ Http2Error.REFUSED_STREAM.getCode() + "]"));
+ output.clearTrace();
- // Confirm another request can be sent once concurrency falls back below limit
- sendSimpleGetRequest(7);
- parser.readFrame(true);
- parser.readFrame(true);
- Assert.assertEquals(getSimpleResponseTrace(7), output.getTrace());
- } finally {
- LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.INFO);
- }
+ // Connection window is zero.
+ // Stream window is 8k
+
+ // Reset stream 3 (client cancel)
+ sendRst(3, Http2Error.NO_ERROR.getCode());
+ // Client reset triggers a write error which in turn triggers a server
+ // reset
+ parser.readFrame(true);
+ Assert.assertEquals("3-RST-[5]\n", output.getTrace());
+ output.clearTrace();
+
+ // Open up the connection window.
+ sendWindowUpdate(0, (1 << 31) - 2);
+
+ // Confirm another request can be sent once concurrency falls back below limit
+ sendSimpleGetRequest(7);
+ parser.readFrame(true);
+ parser.readFrame(true);
+ Assert.assertEquals(getSimpleResponseTrace(7), output.getTrace());
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_3.java b/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
index 6c76cb4..d85c620 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
@@ -172,7 +172,7 @@ public class TestHttp2Section_5_3 extends Http2TestBase {
// on ci.apache.org so allow a margin and use 30.
if (data[1] > 30) {
// Larger than expected body size
- Assert.fail("Larger than expected body: [" + output.getTrace() + "]");
+ Assert.fail("Larger than expected body: [" + output.getTrace() + "] " + data[1]);
}
output.clearTrace();
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_6_1.java b/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
index 32d2de9..8e3d948 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_6_1.java
@@ -157,44 +157,37 @@ public class TestHttp2Section_6_1 extends Http2TestBase {
@Test
public void testDataFrameWithZeroLengthPadding() throws Exception {
- LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.ALL);
- LogManager.getLogManager().getLogger("org.apache.tomcat.util.net").setLevel(Level.ALL);
- try {
- http2Connect();
+ http2Connect();
- byte[] padding = new byte[0];
+ byte[] padding = new byte[0];
- sendSimplePostRequest(3, padding);
- readSimplePostResponse(true);
+ sendSimplePostRequest(3, padding);
+ readSimplePostResponse(true);
- // The window updates for padding could occur anywhere since they
- // happen on a different thread to the response.
- // The connection window update is always present if there is
- // padding.
- String trace = output.getTrace();
- String paddingWindowUpdate = "0-WindowSize-[1]\n";
- Assert.assertTrue(trace, trace.contains(paddingWindowUpdate));
+ // The window updates for padding could occur anywhere since they
+ // happen on a different thread to the response.
+ // The connection window update is always present if there is
+ // padding.
+ String trace = output.getTrace();
+ String paddingWindowUpdate = "0-WindowSize-[1]\n";
+ Assert.assertTrue(trace, trace.contains(paddingWindowUpdate));
+ trace = trace.replace(paddingWindowUpdate, "");
+
+ // The stream window update may or may not be present depending on
+ // timing. Remove it if present.
+ paddingWindowUpdate = "3-WindowSize-[1]\n";
+ if (trace.contains(paddingWindowUpdate)) {
trace = trace.replace(paddingWindowUpdate, "");
-
- // The stream window update may or may not be present depending on
- // timing. Remove it if present.
- paddingWindowUpdate = "3-WindowSize-[1]\n";
- if (trace.contains(paddingWindowUpdate)) {
- trace = trace.replace(paddingWindowUpdate, "");
- }
-
- Assert.assertEquals("0-WindowSize-[127]\n" +
- "3-WindowSize-[127]\n" +
- "3-HeadersStart\n" +
- "3-Header-[:status]-[200]\n" +
- "3-Header-[content-length]-[127]\n" +
- "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
- "3-HeadersEnd\n" +
- "3-Body-127\n" +
- "3-EndOfStream\n", trace);
- } finally {
- LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.INFO);
- LogManager.getLogManager().getLogger("org.apache.tomcat.util.net").setLevel(Level.INFO);
}
+
+ Assert.assertEquals("0-WindowSize-[127]\n" +
+ "3-WindowSize-[127]\n" +
+ "3-HeadersStart\n" +
+ "3-Header-[:status]-[200]\n" +
+ "3-Header-[content-length]-[127]\n" +
+ "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
+ "3-HeadersEnd\n" +
+ "3-Body-127\n" +
+ "3-EndOfStream\n", trace);
}
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_6_9.java b/test/org/apache/coyote/http2/TestHttp2Section_6_9.java
index 467d69a..02d24eb 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_6_9.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_6_9.java
@@ -78,7 +78,7 @@ public class TestHttp2Section_6_9 extends Http2TestBase {
byte[] zeroLengthWindowFrame = new byte[9];
// Length zero
- ByteUtil.setOneBytes(zeroLengthWindowFrame, 3, FrameType.WINDOW_UPDATE.getIdByte());
+ setOneBytes(zeroLengthWindowFrame, 3, FrameType.WINDOW_UPDATE.getIdByte());
// No flags
// Stream 1
ByteUtil.set31Bits(zeroLengthWindowFrame, 5, 1);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org