From 78f3ee33966b8c0fa3639749cb931b3b1e752471 Mon Sep 17 00:00:00 2001 From: Jeff Pinner Date: Sat, 14 Dec 2013 10:27:14 -0800 Subject: [PATCH] SPDY: add SETTINGS_MINOR_VERSION --- .../codec/spdy/DefaultSpdySettingsFrame.java | 2 +- .../handler/codec/spdy/SpdyFrameDecoder.java | 8 -------- .../handler/codec/spdy/SpdySessionHandler.java | 16 ++++++++++++++++ .../handler/codec/spdy/SpdySettingsFrame.java | 5 +++-- .../io/netty/handler/codec/spdy/SpdyVersion.java | 12 +++++++++--- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java index 83c76a5641..7a9b2072a5 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java @@ -57,7 +57,7 @@ public class DefaultSpdySettingsFrame implements SpdySettingsFrame { @Override public SpdySettingsFrame setValue(int id, int value, boolean persistValue, boolean persisted) { - if (id <= 0 || id > SpdyCodecUtil.SPDY_SETTINGS_MAX_ID) { + if (id < 0 || id > SpdyCodecUtil.SPDY_SETTINGS_MAX_ID) { throw new IllegalArgumentException("Setting ID is not valid: " + id); } Integer key = Integer.valueOf(id); diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java index 5c1ca54150..c44fc277ee 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java @@ -191,14 +191,6 @@ public class SpdyFrameDecoder extends ByteToMessageDecoder { int value = getSignedInt(buffer, buffer.readerIndex() + 4); buffer.skipBytes(8); - // Check for invalid ID -- avoid IllegalArgumentException in setValue - if (ID == 0) { - state = State.FRAME_ERROR; - spdySettingsFrame = null; - fireInvalidFrameException(ctx); - return; - } - if (!spdySettingsFrame.isSet(ID)) { boolean persistVal = (ID_flags & SPDY_SETTINGS_PERSIST_VALUE) != 0; boolean persisted = (ID_flags & SPDY_SETTINGS_PERSISTED) != 0; diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java index be70763cba..cd71d930ce 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java @@ -60,6 +60,7 @@ public class SpdySessionHandler extends ChannelHandlerAdapter { private ChannelFutureListener closeSessionFutureListener; private final boolean server; + private final int minorVersion; private final boolean sessionFlowControl; /** @@ -76,6 +77,7 @@ public class SpdySessionHandler extends ChannelHandlerAdapter { throw new NullPointerException("version"); } this.server = server; + minorVersion = version.getMinorVersion(); sessionFlowControl = version.useSessionFlowControl(); } @@ -294,6 +296,13 @@ public class SpdySessionHandler extends ChannelHandlerAdapter { SpdySettingsFrame spdySettingsFrame = (SpdySettingsFrame) msg; + int settingsMinorVersion = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MINOR_VERSION); + if (settingsMinorVersion >= 0 && settingsMinorVersion != minorVersion) { + // Settings frame had the wrong minor version + issueSessionError(ctx, SpdySessionStatus.PROTOCOL_ERROR); + return; + } + int newConcurrentStreams = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MAX_CONCURRENT_STREAMS); if (newConcurrentStreams >= 0) { @@ -574,6 +583,13 @@ public class SpdySessionHandler extends ChannelHandlerAdapter { SpdySettingsFrame spdySettingsFrame = (SpdySettingsFrame) msg; + int settingsMinorVersion = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MINOR_VERSION); + if (settingsMinorVersion >= 0 && settingsMinorVersion != minorVersion) { + // Settings frame had the wrong minor version + promise.setFailure(PROTOCOL_EXCEPTION); + return; + } + int newConcurrentStreams = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MAX_CONCURRENT_STREAMS); if (newConcurrentStreams >= 0) { diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySettingsFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySettingsFrame.java index 6cdbb8afa4..92aa8cc563 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySettingsFrame.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySettingsFrame.java @@ -22,6 +22,7 @@ import java.util.Set; */ public interface SpdySettingsFrame extends SpdyFrame { + int SETTINGS_MINOR_VERSION = 0; int SETTINGS_UPLOAD_BANDWIDTH = 1; int SETTINGS_DOWNLOAD_BANDWIDTH = 2; int SETTINGS_ROUND_TRIP_TIME = 3; @@ -50,7 +51,7 @@ public interface SpdySettingsFrame extends SpdyFrame { /** * Sets the value of the setting ID. - * The ID must be positive and cannot exceed 16777215. + * The ID cannot be negative and cannot exceed 16777215. */ SpdySettingsFrame setValue(int id, int value); @@ -58,7 +59,7 @@ public interface SpdySettingsFrame extends SpdyFrame { * Sets the value of the setting ID. * Sets if the setting should be persisted (should only be set by the server). * Sets if the setting is persisted (should only be set by the client). - * The ID must be positive and cannot exceed 16777215. + * The ID cannot be negative and cannot exceed 16777215. */ SpdySettingsFrame setValue(int id, int value, boolean persistVal, boolean persisted); diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyVersion.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyVersion.java index 0e4c4fbe28..f3adac287c 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyVersion.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyVersion.java @@ -16,14 +16,16 @@ package io.netty.handler.codec.spdy; public enum SpdyVersion { - SPDY_3 (3, false), - SPDY_3_1 (3, true); + SPDY_3 (3, 0, false), + SPDY_3_1 (3, 1, true); private final int version; + private final int minorVersion; private final boolean sessionFlowControl; - private SpdyVersion(int version, boolean sessionFlowControl) { + private SpdyVersion(int version, int minorVersion, boolean sessionFlowControl) { this.version = version; + this.minorVersion = minorVersion; this.sessionFlowControl = sessionFlowControl; } @@ -31,6 +33,10 @@ public enum SpdyVersion { return version; } + int getMinorVersion() { + return minorVersion; + } + boolean useSessionFlowControl() { return sessionFlowControl; }