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 c3cd99d54d2ef63310e532858fa164d5c26a0f7c Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Mar 11 13:09:26 2021 +0000 Re-factoring to align with 9.0.x / 10.0.x First pass. Mainly aligning visibility and removing deprecated/unused code that is specific to the o.a.c.http2 package --- .../apache/coyote/http2/AbstractNonZeroStream.java | 2 +- java/org/apache/coyote/http2/AbstractStream.java | 29 +++++------ .../apache/coyote/http2/ConnectionException.java | 2 +- .../coyote/http2/ConnectionSettingsBase.java | 44 ++++++++-------- .../coyote/http2/ConnectionSettingsLocal.java | 12 ++--- .../coyote/http2/ConnectionSettingsRemote.java | 6 +-- java/org/apache/coyote/http2/Flags.java | 12 ++--- java/org/apache/coyote/http2/FrameType.java | 8 +-- java/org/apache/coyote/http2/HeaderSink.java | 2 +- java/org/apache/coyote/http2/HpackDecoder.java | 8 +-- java/org/apache/coyote/http2/HpackEncoder.java | 36 +++++++------ java/org/apache/coyote/http2/HpackException.java | 9 ++-- java/org/apache/coyote/http2/Http2Error.java | 6 +-- java/org/apache/coyote/http2/Http2Exception.java | 2 +- .../apache/coyote/http2/Http2UpgradeHandler.java | 10 +--- java/org/apache/coyote/http2/RecycledStream.java | 11 +--- java/org/apache/coyote/http2/Setting.java | 8 +-- java/org/apache/coyote/http2/Stream.java | 60 ++++++++++------------ java/org/apache/coyote/http2/StreamException.java | 6 +-- java/org/apache/coyote/http2/StreamProcessor.java | 10 ++-- .../apache/coyote/http2/StreamStateMachine.java | 30 +++++------ webapps/docs/changelog.xml | 4 ++ 22 files changed, 149 insertions(+), 168 deletions(-) diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java b/java/org/apache/coyote/http2/AbstractNonZeroStream.java index 930f131..38761ad 100644 --- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java +++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java @@ -49,7 +49,7 @@ abstract class AbstractNonZeroStream extends AbstractStream { @Override - protected final int getWeight() { + final int getWeight() { return weight; } diff --git a/java/org/apache/coyote/http2/AbstractStream.java b/java/org/apache/coyote/http2/AbstractStream.java index 43b8cde..deaaa6a 100644 --- a/java/org/apache/coyote/http2/AbstractStream.java +++ b/java/org/apache/coyote/http2/AbstractStream.java @@ -42,13 +42,13 @@ abstract class AbstractStream { private long windowSize = ConnectionSettingsBase.DEFAULT_INITIAL_WINDOW_SIZE; - public AbstractStream(Integer identifier) { + AbstractStream(Integer identifier) { this.identifier = identifier; this.idAsString = identifier.toString(); } - public Integer getIdentifier() { + final Integer getIdentifier() { return identifier; } @@ -58,12 +58,12 @@ abstract class AbstractStream { } - public int getIdAsInt() { + final int getIdAsInt() { return identifier.intValue(); } - void detachFromParent() { + final void detachFromParent() { if (parentStream != null) { parentStream.getChildStreams().remove(this); parentStream = null; @@ -77,7 +77,7 @@ abstract class AbstractStream { } - boolean isDescendant(AbstractStream stream) { + final boolean isDescendant(AbstractStream stream) { // Is the passed in Stream a descendant of this Stream? // Start at the passed in Stream and work up AbstractStream parent = stream.getParentStream(); @@ -88,12 +88,12 @@ abstract class AbstractStream { } - AbstractStream getParentStream() { + final AbstractStream getParentStream() { return parentStream; } - void setParentStream(AbstractStream parentStream) { + final void setParentStream(AbstractStream parentStream) { this.parentStream = parentStream; } @@ -103,12 +103,12 @@ abstract class AbstractStream { } - protected synchronized void setWindowSize(long windowSize) { + final synchronized void setWindowSize(long windowSize) { this.windowSize = windowSize; } - protected synchronized long getWindowSize() { + final synchronized long getWindowSize() { return windowSize; } @@ -119,7 +119,7 @@ abstract class AbstractStream { * @throws Http2Exception If the window size is now higher than * the maximum allowed */ - protected synchronized void incrementWindowSize(int increment) throws Http2Exception { + synchronized void incrementWindowSize(int increment) throws Http2Exception { // No need for overflow protection here. // Increment can't be more than Integer.MAX_VALUE and once windowSize // goes beyond 2^31-1 an error is triggered. @@ -143,7 +143,7 @@ abstract class AbstractStream { } - protected synchronized void decrementWindowSize(int decrement) { + final synchronized void decrementWindowSize(int decrement) { // No need for overflow protection here. Decrement can never be larger // the Integer.MAX_VALUE and once windowSize goes negative no further // decrements are permitted @@ -155,10 +155,7 @@ abstract class AbstractStream { } - protected abstract String getConnectionId(); + abstract String getConnectionId(); - protected abstract int getWeight(); - - @Deprecated // Unused - protected abstract void doNotifyAll(); + abstract int getWeight(); } diff --git a/java/org/apache/coyote/http2/ConnectionException.java b/java/org/apache/coyote/http2/ConnectionException.java index db754ac..6f48a31 100644 --- a/java/org/apache/coyote/http2/ConnectionException.java +++ b/java/org/apache/coyote/http2/ConnectionException.java @@ -19,7 +19,7 @@ package org.apache.coyote.http2; /** * Thrown when an HTTP/2 connection error occurs. */ -public class ConnectionException extends Http2Exception { +class ConnectionException extends Http2Exception { private static final long serialVersionUID = 1L; diff --git a/java/org/apache/coyote/http2/ConnectionSettingsBase.java b/java/org/apache/coyote/http2/ConnectionSettingsBase.java index 5b04905..2e67fbc 100644 --- a/java/org/apache/coyote/http2/ConnectionSettingsBase.java +++ b/java/org/apache/coyote/http2/ConnectionSettingsBase.java @@ -23,7 +23,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.res.StringManager; -public abstract class ConnectionSettingsBase<T extends Throwable> { +abstract class ConnectionSettingsBase<T extends Throwable> { private final Log log = LogFactory.getLog(ConnectionSettingsBase.class); // must not be static private final StringManager sm = StringManager.getManager(ConnectionSettingsBase.class); @@ -31,25 +31,25 @@ public abstract class ConnectionSettingsBase<T extends Throwable> { private final String connectionId; // Limits - protected static final int MAX_WINDOW_SIZE = (1 << 31) - 1; - protected static final int MIN_MAX_FRAME_SIZE = 1 << 14; - protected static final int MAX_MAX_FRAME_SIZE = (1 << 24) - 1; - protected static final long UNLIMITED = ((long)1 << 32); // Use the maximum possible - protected static final int MAX_HEADER_TABLE_SIZE = 1 << 16; + static final int MAX_WINDOW_SIZE = (1 << 31) - 1; + static final int MIN_MAX_FRAME_SIZE = 1 << 14; + static final int MAX_MAX_FRAME_SIZE = (1 << 24) - 1; + static final long UNLIMITED = ((long)1 << 32); // Use the maximum possible + static final int MAX_HEADER_TABLE_SIZE = 1 << 16; // Defaults (defined by the specification) - protected static final int DEFAULT_HEADER_TABLE_SIZE = Hpack.DEFAULT_TABLE_SIZE; - protected static final boolean DEFAULT_ENABLE_PUSH = true; - protected static final long DEFAULT_MAX_CONCURRENT_STREAMS = UNLIMITED; - protected static final int DEFAULT_INITIAL_WINDOW_SIZE = (1 << 16) - 1; - protected static final int DEFAULT_MAX_FRAME_SIZE = MIN_MAX_FRAME_SIZE; - protected static final long DEFAULT_MAX_HEADER_LIST_SIZE = 1 << 15; + static final int DEFAULT_HEADER_TABLE_SIZE = Hpack.DEFAULT_TABLE_SIZE; + static final boolean DEFAULT_ENABLE_PUSH = true; + static final long DEFAULT_MAX_CONCURRENT_STREAMS = UNLIMITED; + static final int DEFAULT_INITIAL_WINDOW_SIZE = (1 << 16) - 1; + static final int DEFAULT_MAX_FRAME_SIZE = MIN_MAX_FRAME_SIZE; + static final long DEFAULT_MAX_HEADER_LIST_SIZE = 1 << 15; - protected Map<Setting,Long> current = new ConcurrentHashMap<>(); - protected Map<Setting,Long> pending = new ConcurrentHashMap<>(); + Map<Setting, Long> current = new ConcurrentHashMap<>(); + Map<Setting, Long> pending = new ConcurrentHashMap<>(); - public ConnectionSettingsBase(String connectionId) { + ConnectionSettingsBase(String connectionId) { this.connectionId = connectionId; // Set up the defaults current.put(Setting.HEADER_TABLE_SIZE, Long.valueOf(DEFAULT_HEADER_TABLE_SIZE)); @@ -61,7 +61,7 @@ public abstract class ConnectionSettingsBase<T extends Throwable> { } - public void set(Setting setting, long value) throws T { + final void set(Setting setting, long value) throws T { if (log.isDebugEnabled()) { log.debug(sm.getString("connectionSettings.debug", connectionId, getEndpointName(), setting, Long.toString(value))); @@ -102,33 +102,33 @@ public abstract class ConnectionSettingsBase<T extends Throwable> { } - public int getHeaderTableSize() { + final int getHeaderTableSize() { return getMinInt(Setting.HEADER_TABLE_SIZE); } - public boolean getEnablePush() { + final boolean getEnablePush() { long result = getMin(Setting.ENABLE_PUSH); return result != 0; } - public long getMaxConcurrentStreams() { + final long getMaxConcurrentStreams() { return getMax(Setting.MAX_CONCURRENT_STREAMS); } - public int getInitialWindowSize() { + final int getInitialWindowSize() { return getMaxInt(Setting.INITIAL_WINDOW_SIZE); } - public int getMaxFrameSize() { + final int getMaxFrameSize() { return getMaxInt(Setting.MAX_FRAME_SIZE); } - public long getMaxHeaderListSize() { + final long getMaxHeaderListSize() { return getMax(Setting.MAX_HEADER_LIST_SIZE); } diff --git a/java/org/apache/coyote/http2/ConnectionSettingsLocal.java b/java/org/apache/coyote/http2/ConnectionSettingsLocal.java index dd2e449..64e68bb 100644 --- a/java/org/apache/coyote/http2/ConnectionSettingsLocal.java +++ b/java/org/apache/coyote/http2/ConnectionSettingsLocal.java @@ -30,20 +30,20 @@ import java.util.Map; * client will respond (almost certainly by closing the connection) as defined * in the HTTP/2 specification. */ -public class ConnectionSettingsLocal extends ConnectionSettingsBase<IllegalArgumentException> { +class ConnectionSettingsLocal extends ConnectionSettingsBase<IllegalArgumentException> { private static final String ENDPOINT_NAME = "Local(client->server)"; private boolean sendInProgress = false; - public ConnectionSettingsLocal(String connectionId) { + ConnectionSettingsLocal(String connectionId) { super(connectionId); } @Override - protected synchronized void set(Setting setting, Long value) { + final synchronized void set(Setting setting, Long value) { checkSend(); if (current.get(setting).longValue() == value.longValue()) { pending.remove(setting); @@ -53,7 +53,7 @@ public class ConnectionSettingsLocal extends ConnectionSettingsBase<IllegalArgum } - synchronized byte[] getSettingsFrameForPending() { + final synchronized byte[] getSettingsFrameForPending() { checkSend(); int payloadSize = pending.size() * 6; byte[] result = new byte[9 + payloadSize]; @@ -75,7 +75,7 @@ public class ConnectionSettingsLocal extends ConnectionSettingsBase<IllegalArgum } - synchronized boolean ack() { + final synchronized boolean ack() { if (sendInProgress) { sendInProgress = false; current.putAll(pending); @@ -96,7 +96,7 @@ public class ConnectionSettingsLocal extends ConnectionSettingsBase<IllegalArgum @Override - void throwException(String msg, Http2Error error) throws IllegalArgumentException { + final void throwException(String msg, Http2Error error) throws IllegalArgumentException { throw new IllegalArgumentException(msg); } diff --git a/java/org/apache/coyote/http2/ConnectionSettingsRemote.java b/java/org/apache/coyote/http2/ConnectionSettingsRemote.java index 7380ee4..235446a 100644 --- a/java/org/apache/coyote/http2/ConnectionSettingsRemote.java +++ b/java/org/apache/coyote/http2/ConnectionSettingsRemote.java @@ -20,17 +20,17 @@ package org.apache.coyote.http2; * Represents the remote connection settings: i.e. the settings the server must * use when communicating with the client. */ -public class ConnectionSettingsRemote extends ConnectionSettingsBase<ConnectionException> { +class ConnectionSettingsRemote extends ConnectionSettingsBase<ConnectionException> { private static final String ENDPOINT_NAME = "Remote(server->client)"; - public ConnectionSettingsRemote(String connectionId) { + ConnectionSettingsRemote(String connectionId) { super(connectionId); } @Override - void throwException(String msg, Http2Error error) throws ConnectionException { + final void throwException(String msg, Http2Error error) throws ConnectionException { throw new ConnectionException(msg, error); } diff --git a/java/org/apache/coyote/http2/Flags.java b/java/org/apache/coyote/http2/Flags.java index f1d883f..d7ff181 100644 --- a/java/org/apache/coyote/http2/Flags.java +++ b/java/org/apache/coyote/http2/Flags.java @@ -16,34 +16,34 @@ */ package org.apache.coyote.http2; -public class Flags { +class Flags { private Flags() { // Utility class. Hide default constructor } - public static boolean isEndOfStream(int flags) { + static boolean isEndOfStream(int flags) { return (flags & 0x01) != 0; } - public static boolean isAck(int flags) { + static boolean isAck(int flags) { return (flags & 0x01) != 0; } - public static boolean isEndOfHeaders(int flags) { + static boolean isEndOfHeaders(int flags) { return (flags & 0x04) != 0; } - public static boolean hasPadding(int flags) { + static boolean hasPadding(int flags) { return (flags & 0x08) != 0; } - public static boolean hasPriority(int flags) { + static boolean hasPriority(int flags) { return (flags & 0x20) != 0; } } diff --git a/java/org/apache/coyote/http2/FrameType.java b/java/org/apache/coyote/http2/FrameType.java index 76f655d..d671f10 100644 --- a/java/org/apache/coyote/http2/FrameType.java +++ b/java/org/apache/coyote/http2/FrameType.java @@ -18,7 +18,7 @@ package org.apache.coyote.http2; import org.apache.tomcat.util.res.StringManager; -public enum FrameType { +enum FrameType { DATA (0, false, true, null, false), HEADERS (1, false, true, null, true), @@ -51,12 +51,12 @@ public enum FrameType { } - public byte getIdByte() { + byte getIdByte() { return (byte) id; } - public void check(int streamId, int payloadSize) throws Http2Exception { + void check(int streamId, int payloadSize) throws Http2Exception { // Is FrameType valid for the given stream? if (streamId == 0 && !streamZero || streamId != 0 && !streamNonZero) { throw new ConnectionException(sm.getString("frameType.checkStream", this), @@ -78,7 +78,7 @@ public enum FrameType { } - public static FrameType valueOf(int i) { + static FrameType valueOf(int i) { switch(i) { case 0: return DATA; diff --git a/java/org/apache/coyote/http2/HeaderSink.java b/java/org/apache/coyote/http2/HeaderSink.java index a41e73a..d9fe699 100644 --- a/java/org/apache/coyote/http2/HeaderSink.java +++ b/java/org/apache/coyote/http2/HeaderSink.java @@ -23,7 +23,7 @@ import org.apache.coyote.http2.HpackDecoder.HeaderEmitter; * the connection close process has started if headers for new streams are * received. */ -public class HeaderSink implements HeaderEmitter { +class HeaderSink implements HeaderEmitter { @Override public void emitHeader(String name, String value) { diff --git a/java/org/apache/coyote/http2/HpackDecoder.java b/java/org/apache/coyote/http2/HpackDecoder.java index 0fa5963..517dc0b 100644 --- a/java/org/apache/coyote/http2/HpackDecoder.java +++ b/java/org/apache/coyote/http2/HpackDecoder.java @@ -72,13 +72,13 @@ public class HpackDecoder { private volatile boolean countedCookie; private volatile int headerSize = 0; - public HpackDecoder(int maxMemorySize) { + HpackDecoder(int maxMemorySize) { this.maxMemorySizeHard = maxMemorySize; this.maxMemorySizeSoft = maxMemorySize; headerTable = new Hpack.HeaderField[DEFAULT_RING_BUFFER_SIZE]; } - public HpackDecoder() { + HpackDecoder() { this(Hpack.DEFAULT_TABLE_SIZE); } @@ -91,7 +91,7 @@ public class HpackDecoder { * * @throws HpackException If the packed data is not valid */ - public void decode(ByteBuffer buffer) throws HpackException { + void decode(ByteBuffer buffer) throws HpackException { while (buffer.hasRemaining()) { int originalPos = buffer.position(); byte b = buffer.get(); @@ -382,7 +382,7 @@ public class HpackDecoder { } - public HeaderEmitter getHeaderEmitter() { + HeaderEmitter getHeaderEmitter() { return headerEmitter; } diff --git a/java/org/apache/coyote/http2/HpackEncoder.java b/java/org/apache/coyote/http2/HpackEncoder.java index 7331ca6..ae81033 100644 --- a/java/org/apache/coyote/http2/HpackEncoder.java +++ b/java/org/apache/coyote/http2/HpackEncoder.java @@ -34,12 +34,12 @@ import org.apache.tomcat.util.res.StringManager; /** * Encoder for HPACK frames. */ -public class HpackEncoder { +class HpackEncoder { private static final Log log = LogFactory.getLog(HpackEncoder.class); private static final StringManager sm = StringManager.getManager(HpackEncoder.class); - public static final HpackHeaderFunction DEFAULT_HEADER_FUNCTION = new HpackHeaderFunction() { + private static final HpackHeaderFunction DEFAULT_HEADER_FUNCTION = new HpackHeaderFunction() { @Override public boolean shouldUseIndexing(String headerName, String value) { //content length and date change all the time @@ -122,7 +122,7 @@ public class HpackEncoder { * * @return The state of the encoding process */ - public State encode(MimeHeaders headers, ByteBuffer target) { + State encode(MimeHeaders headers, ByteBuffer target) { int it = headersIterator; if (headersIterator == -1) { handleTableSizeChange(target); @@ -251,7 +251,7 @@ public class HpackEncoder { } existing.add(d); evictionQueue.add(d); - currentTableSize += d.size; + currentTableSize += d.getSize(); runEvictionIfRequired(); if (entryPositionCounter == Integer.MAX_VALUE) { //prevent rollover @@ -336,19 +336,19 @@ public class HpackEncoder { minNewMaxHeaderSize = -1; } - public enum State { + enum State { COMPLETE, UNDERFLOW, } - static class TableEntry { - final String name; - final String value; - final int size; - int position; + private static class TableEntry { + private final String name; + private final String value; + private final int size; + private int position; - TableEntry(String name, String value, int position) { + private TableEntry(String name, String value, int position) { this.name = name; this.value = value; this.position = position; @@ -359,24 +359,28 @@ public class HpackEncoder { } } - public int getPosition() { + int getPosition() { return position; } + + int getSize() { + return size; + } } - class DynamicTableEntry extends TableEntry { + private class DynamicTableEntry extends TableEntry { - DynamicTableEntry(String name, String value, int position) { + private DynamicTableEntry(String name, String value, int position) { super(name, value, position); } @Override - public int getPosition() { + int getPosition() { return super.getPosition() + entryPositionCounter + Hpack.STATIC_TABLE_LENGTH; } } - public interface HpackHeaderFunction { + private interface HpackHeaderFunction { boolean shouldUseIndexing(String header, String value); /** diff --git a/java/org/apache/coyote/http2/HpackException.java b/java/org/apache/coyote/http2/HpackException.java index 1dc3f7c..f5dd55c 100644 --- a/java/org/apache/coyote/http2/HpackException.java +++ b/java/org/apache/coyote/http2/HpackException.java @@ -20,17 +20,14 @@ package org.apache.coyote.http2; * Exception that is thrown when the HPACK compress context is broken. In this * case the connection must be closed. */ -public class HpackException extends Exception { +class HpackException extends Exception { private static final long serialVersionUID = 1L; - public HpackException(String message, Throwable cause) { - super(message, cause); - } - public HpackException(String message) { + HpackException(String message) { super(message); } - public HpackException() { + HpackException() { super(); } } diff --git a/java/org/apache/coyote/http2/Http2Error.java b/java/org/apache/coyote/http2/Http2Error.java index 9db65ac..b826b57 100644 --- a/java/org/apache/coyote/http2/Http2Error.java +++ b/java/org/apache/coyote/http2/Http2Error.java @@ -16,7 +16,7 @@ */ package org.apache.coyote.http2; -public enum Http2Error { +enum Http2Error { NO_ERROR (0x00), PROTOCOL_ERROR (0x01), @@ -40,12 +40,12 @@ public enum Http2Error { } - public long getCode() { + long getCode() { return code; } - public byte[] getCodeBytes() { + byte[] getCodeBytes() { byte[] codeByte = new byte[4]; ByteUtil.setFourBytes(codeByte, 0, code); return codeByte; diff --git a/java/org/apache/coyote/http2/Http2Exception.java b/java/org/apache/coyote/http2/Http2Exception.java index 5abaa9c..583af37 100644 --- a/java/org/apache/coyote/http2/Http2Exception.java +++ b/java/org/apache/coyote/http2/Http2Exception.java @@ -16,7 +16,7 @@ */ package org.apache.coyote.http2; -public abstract class Http2Exception extends Exception { +abstract class Http2Exception extends Exception { private static final long serialVersionUID = 1L; diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 4ee1104..8ec1a1b 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -73,7 +73,7 @@ import org.apache.tomcat.util.res.StringManager; * </li> * </ul> */ -public class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeHandler, +class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeHandler, Input, Output { private static final Log log = LogFactory.getLog(Http2UpgradeHandler.class); @@ -1012,14 +1012,6 @@ public class Http2UpgradeHandler extends AbstractStream implements InternalHttpU } - - @Override - @Deprecated - protected synchronized void doNotifyAll() { - // NO-OP. Unused. - } - - private int allocate(AbstractStream stream, int allocation) { if (log.isDebugEnabled()) { log.debug(sm.getString("upgradeHandler.allocate.debug", getConnectionId(), diff --git a/java/org/apache/coyote/http2/RecycledStream.java b/java/org/apache/coyote/http2/RecycledStream.java index 8f9cc06..b2b3e32 100644 --- a/java/org/apache/coyote/http2/RecycledStream.java +++ b/java/org/apache/coyote/http2/RecycledStream.java @@ -31,22 +31,15 @@ class RecycledStream extends AbstractNonZeroStream { @Override - protected String getConnectionId() { + String getConnectionId() { return connectionId; } @SuppressWarnings("sync-override") @Override - protected void incrementWindowSize(int increment) throws Http2Exception { + void incrementWindowSize(int increment) throws Http2Exception { // NO-OP } - - - @Override - @Deprecated - protected synchronized void doNotifyAll() { - // NO-OP. Unused. - } } diff --git a/java/org/apache/coyote/http2/Setting.java b/java/org/apache/coyote/http2/Setting.java index edfde16..54b99da 100644 --- a/java/org/apache/coyote/http2/Setting.java +++ b/java/org/apache/coyote/http2/Setting.java @@ -16,7 +16,7 @@ */ package org.apache.coyote.http2; -public enum Setting { +enum Setting { HEADER_TABLE_SIZE(1), ENABLE_PUSH(2), MAX_CONCURRENT_STREAMS(3), @@ -31,16 +31,16 @@ public enum Setting { this.id = id; } - public int getId() { + final int getId() { return id; } @Override - public String toString() { + public final String toString() { return Integer.toString(id); } - public static Setting valueOf(int i) { + static final Setting valueOf(int i) { switch(i) { case 1: { return HEADER_TABLE_SIZE; diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index d7dad0e..1018ff2 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -42,7 +42,7 @@ import org.apache.tomcat.util.net.ApplicationBufferHandler; import org.apache.tomcat.util.net.WriteBuffer; import org.apache.tomcat.util.res.StringManager; -public class Stream extends AbstractNonZeroStream implements HeaderEmitter { +class Stream extends AbstractNonZeroStream implements HeaderEmitter { private static final Log log = LogFactory.getLog(Stream.class); private static final StringManager sm = StringManager.getManager(Stream.class); @@ -83,12 +83,12 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { new Http2OutputBuffer(coyoteResponse, streamOutputBuffer); - public Stream(Integer identifier, Http2UpgradeHandler handler) { + Stream(Integer identifier, Http2UpgradeHandler handler) { this(identifier, handler, null); } - public Stream(Integer identifier, Http2UpgradeHandler handler, Request coyoteRequest) { + Stream(Integer identifier, Http2UpgradeHandler handler, Request coyoteRequest) { super(handler.getConnectionId(), identifier); this.handler = handler; handler.addChild(this); @@ -190,7 +190,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { @Override - protected final synchronized void incrementWindowSize(int windowSizeIncrement) throws Http2Exception { + final synchronized void incrementWindowSize(int windowSizeIncrement) throws Http2Exception { // If this is zero then any thread that has been trying to write for // this stream will be waiting. Notify that thread it can continue. Use // notify all even though only one thread is waiting to be on the safe @@ -269,13 +269,6 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { @Override - @Deprecated - protected synchronized void doNotifyAll() { - // NO-OP. Unused. - } - - - @Override public final void emitHeader(String name, String value) throws HpackException { if (log.isDebugEnabled()) { log.debug(sm.getString("stream.header.debug", getConnectionId(), getIdAsString(), @@ -456,7 +449,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - void writeHeaders() throws IOException { + final void writeHeaders() throws IOException { boolean endOfStream = streamOutputBuffer.hasNoBody(); handler.writeHeaders(this, 0, coyoteResponse.getMimeHeaders(), endOfStream, Constants.DEFAULT_HEADERS_FRAME_SIZE); } @@ -473,7 +466,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { @Override - protected final String getConnectionId() { + final String getConnectionId() { return handler.getConnectionId(); } @@ -483,12 +476,12 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - Response getCoyoteResponse() { + final Response getCoyoteResponse() { return coyoteResponse; } - ByteBuffer getInputByteBuffer() { + final ByteBuffer getInputByteBuffer() { // Avoid NPE if Stream has been closed on Stream specific thread StreamInputBuffer inputBuffer = this.inputBuffer; if (inputBuffer == null) { @@ -580,7 +573,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - StreamInputBuffer getInputBuffer() { + final StreamInputBuffer getInputBuffer() { return inputBuffer; } @@ -590,17 +583,17 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - void sentPushPromise() { + final void sentPushPromise() { state.sentPushPromise(); } - boolean isActive() { + final boolean isActive() { return state.isActive(); } - boolean canWrite() { + final boolean canWrite() { return state.canWrite(); } @@ -610,12 +603,12 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - boolean isInputFinished() { + final boolean isInputFinished() { return !state.isFrameTypePermitted(FrameType.DATA); } - void close(Http2Exception http2Exception) { + final void close(Http2Exception http2Exception) { if (http2Exception instanceof StreamException) { try { StreamException se = (StreamException) http2Exception; @@ -824,7 +817,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { return totalThisTime; } - public synchronized boolean flush(boolean block) throws IOException { + final synchronized boolean flush(boolean block) throws IOException { /* * Need to ensure that there is exactly one call to flush even when * there is no data to write. @@ -859,7 +852,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { return dataLeft; } - private synchronized boolean flush(boolean writeInProgress, boolean block) + private final synchronized boolean flush(boolean writeInProgress, boolean block) throws IOException { if (log.isDebugEnabled()) { log.debug(sm.getString("stream.outputBuffer.flush.debug", getConnectionId(), @@ -909,7 +902,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { return false; } - synchronized boolean isReady() { + final synchronized boolean isReady() { // Bug 63682 // Only want to return false if the window size is zero AND we are // already waiting for an allocation. @@ -923,7 +916,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } @Override - public long getBytesWritten() { + public final long getBytesWritten() { return written; } @@ -946,7 +939,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { * @return <code>true</code> if it is certain that the associated * response has no body. */ - public boolean hasNoBody() { + final boolean hasNoBody() { return ((written == 0) && closed); } @@ -1089,7 +1082,8 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } @Override - public int doRead(ApplicationBufferHandler applicationBufferHandler) throws IOException { + public final int doRead(ApplicationBufferHandler applicationBufferHandler) + throws IOException { ensureBuffersExist(); @@ -1178,7 +1172,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } } - synchronized boolean isRequestBodyFullyRead() { + final synchronized boolean isRequestBodyFullyRead() { return (inBuffer == null || inBuffer.position() == 0) && isInputFinished(); } @@ -1195,7 +1189,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { /* * Called after placing some data in the inBuffer. */ - synchronized boolean onDataAvailable() { + final synchronized boolean onDataAvailable() { if (readInterest) { if (log.isDebugEnabled()) { log.debug(sm.getString("stream.inputBuffer.dispatch")); @@ -1219,18 +1213,18 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - public ByteBuffer getInBuffer() { + private final ByteBuffer getInBuffer() { ensureBuffersExist(); return inBuffer; } - protected synchronized void insertReplayedBody(ByteChunk body) { + final synchronized void insertReplayedBody(ByteChunk body) { inBuffer = ByteBuffer.wrap(body.getBytes(), body.getOffset(), body.getLength()); } - private void ensureBuffersExist() { + private final void ensureBuffersExist() { if (inBuffer == null) { // The client must obey Tomcat's window size when sending so // this is the initial window size set by Tomcat that the client @@ -1246,7 +1240,7 @@ public class Stream extends AbstractNonZeroStream implements HeaderEmitter { } - protected void receiveReset() { + private final void receiveReset() { if (inBuffer != null) { synchronized (inBuffer) { resetReceived = true; diff --git a/java/org/apache/coyote/http2/StreamException.java b/java/org/apache/coyote/http2/StreamException.java index efb3748..ec35b3c 100644 --- a/java/org/apache/coyote/http2/StreamException.java +++ b/java/org/apache/coyote/http2/StreamException.java @@ -19,19 +19,19 @@ package org.apache.coyote.http2; /** * Thrown when an HTTP/2 stream error occurs. */ -public class StreamException extends Http2Exception { +class StreamException extends Http2Exception { private static final long serialVersionUID = 1L; private final int streamId; - public StreamException(String msg, Http2Error error, int streamId) { + StreamException(String msg, Http2Error error, int streamId) { super(msg, error); this.streamId = streamId; } - public int getStreamId() { + int getStreamId() { return streamId; } } diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java index 9b5a948..7690380 100644 --- a/java/org/apache/coyote/http2/StreamProcessor.java +++ b/java/org/apache/coyote/http2/StreamProcessor.java @@ -331,7 +331,7 @@ class StreamProcessor extends AbstractProcessor { @Override - public void recycle() { + public final void recycle() { // StreamProcessor instances are not re-used. // Calling removeRequestProcessor even though the RequestProcesser was @@ -350,19 +350,19 @@ class StreamProcessor extends AbstractProcessor { @Override - protected Log getLog() { + protected final Log getLog() { return log; } @Override - public void pause() { + public final void pause() { // NO-OP. Handled by the Http2UpgradeHandler } @Override - public SocketState service(SocketWrapperBase<?> socket) throws IOException { + public final SocketState service(SocketWrapperBase<?> socket) throws IOException { try { adapter.service(request, response); } catch (Exception e) { @@ -395,7 +395,7 @@ class StreamProcessor extends AbstractProcessor { @Override - protected boolean flushBufferedWrite() throws IOException { + protected final boolean flushBufferedWrite() throws IOException { if (log.isDebugEnabled()) { log.debug(sm.getString("streamProcessor.flushBufferedWrite.entry", stream.getConnectionId(), stream.getIdAsString())); diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java b/java/org/apache/coyote/http2/StreamStateMachine.java index fdc06a4..fa420b9 100644 --- a/java/org/apache/coyote/http2/StreamStateMachine.java +++ b/java/org/apache/coyote/http2/StreamStateMachine.java @@ -34,7 +34,7 @@ import org.apache.tomcat.util.res.StringManager; * </ul> * */ -public class StreamStateMachine { +class StreamStateMachine { private static final Log log = LogFactory.getLog(StreamStateMachine.class); private static final StringManager sm = StringManager.getManager(StreamStateMachine.class); @@ -45,42 +45,42 @@ public class StreamStateMachine { private State state; - public StreamStateMachine(String connectionId, String streamId) { + StreamStateMachine(String connectionId, String streamId) { this.connectionId = connectionId; this.streamId = streamId; stateChange(null, State.IDLE); } - public synchronized void sentPushPromise() { + final synchronized void sentPushPromise() { stateChange(State.IDLE, State.RESERVED_LOCAL); } - public synchronized void receivedPushPromise() { + final synchronized void receivedPushPromise() { stateChange(State.IDLE, State.RESERVED_REMOTE); } - public synchronized void sentStartOfHeaders() { + final synchronized void sentStartOfHeaders() { stateChange(State.IDLE, State.OPEN); stateChange(State.RESERVED_LOCAL, State.HALF_CLOSED_REMOTE); } - public synchronized void receivedStartOfHeaders() { + final synchronized void receivedStartOfHeaders() { stateChange(State.IDLE, State.OPEN); stateChange(State.RESERVED_REMOTE, State.HALF_CLOSED_LOCAL); } - public synchronized void sentEndOfStream() { + final synchronized void sentEndOfStream() { stateChange(State.OPEN, State.HALF_CLOSED_LOCAL); stateChange(State.HALF_CLOSED_REMOTE, State.CLOSED_TX); } - public synchronized void receivedEndOfStream() { + final synchronized void receivedEndOfStream() { stateChange(State.OPEN, State.HALF_CLOSED_REMOTE); stateChange(State.HALF_CLOSED_LOCAL, State.CLOSED_RX); } @@ -124,7 +124,7 @@ public class StreamStateMachine { } - public synchronized void checkFrameType(FrameType frameType) throws Http2Exception { + final synchronized void checkFrameType(FrameType frameType) throws Http2Exception { // No state change. Checks that receiving the frame type is valid for // the current state of this stream. if (!isFrameTypePermitted(frameType)) { @@ -141,31 +141,31 @@ public class StreamStateMachine { } - public synchronized boolean isFrameTypePermitted(FrameType frameType) { + final synchronized boolean isFrameTypePermitted(FrameType frameType) { return state.isFrameTypePermitted(frameType); } - public synchronized boolean isActive() { + final synchronized boolean isActive() { return state.isActive(); } - public synchronized boolean canRead() { + final synchronized boolean canRead() { return state.canRead(); } - public synchronized boolean canWrite() { + final synchronized boolean canWrite() { return state.canWrite(); } - public synchronized boolean isClosedFinal() { + final synchronized boolean isClosedFinal() { return state == State.CLOSED_FINAL; } - public synchronized void closeIfIdle() { + final synchronized void closeIfIdle() { stateChange(State.IDLE, State.CLOSED_FINAL); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index f9e45a5..8cd6be7 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -110,6 +110,10 @@ Improve consistency of OpenSSL error stack handling in the TLS engine, and log all errors found as debug. (remm) </fix> + <scode> + Re-factor the HTTP/2 implementation classes to better align with 9.0.x + and 10.0.x to make maintenance simpler. (markt) + </scode> </changelog> </subsection> </section> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org