From d4bdcefe17db96735c10f5e5fa8babda93610ce2 Mon Sep 17 00:00:00 2001 From: Jeff Pinner Date: Fri, 13 Dec 2013 12:09:45 -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 +++-- .../netty/handler/codec/spdy/SpdyVersion.java | 12 +++++++++--- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java b/src/main/java/org/jboss/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java index eda5a39fa7..5382085c4c 100644 --- a/src/main/java/org/jboss/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java +++ b/src/main/java/org/jboss/netty/handler/codec/spdy/DefaultSpdySettingsFrame.java @@ -52,7 +52,7 @@ public class DefaultSpdySettingsFrame implements SpdySettingsFrame { } public void 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 = id; diff --git a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyFrameDecoder.java b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyFrameDecoder.java index 0ef73e394a..8e2e2c7f9f 100644 --- a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyFrameDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyFrameDecoder.java @@ -163,14 +163,6 @@ public class SpdyFrameDecoder extends FrameDecoder { 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 null; - } - if (!spdySettingsFrame.isSet(ID)) { boolean persistVal = (ID_flags & SPDY_SETTINGS_PERSIST_VALUE) != 0; boolean persisted = (ID_flags & SPDY_SETTINGS_PERSISTED) != 0; diff --git a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySessionHandler.java b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySessionHandler.java index 222bf842e5..882f76ead8 100644 --- a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySessionHandler.java +++ b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySessionHandler.java @@ -62,6 +62,7 @@ public class SpdySessionHandler extends SimpleChannelUpstreamHandler private volatile ChannelFutureListener closeSessionFutureListener; private final boolean server; + private final int minorVersion; private final boolean sessionFlowControl; /** @@ -78,6 +79,7 @@ public class SpdySessionHandler extends SimpleChannelUpstreamHandler throw new NullPointerException("spdyVersion"); } this.server = server; + minorVersion = spdyVersion.getMinorVersion(); sessionFlowControl = spdyVersion.useSessionFlowControl(); } @@ -296,6 +298,13 @@ public class SpdySessionHandler extends SimpleChannelUpstreamHandler 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, e.getChannel(), e.getRemoteAddress(), SpdySessionStatus.PROTOCOL_ERROR); + return; + } + int newConcurrentStreams = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MAX_CONCURRENT_STREAMS); if (newConcurrentStreams >= 0) { @@ -584,6 +593,13 @@ public class SpdySessionHandler extends SimpleChannelUpstreamHandler SpdySettingsFrame spdySettingsFrame = (SpdySettingsFrame) msg; + int settingsMinorVersion = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MINOR_VERSION); + if (settingsMinorVersion >= 0 && settingsMinorVersion != minorVersion) { + // Settings frame had the wrong minor version + e.getFuture().setFailure(PROTOCOL_EXCEPTION); + return; + } + int newConcurrentStreams = spdySettingsFrame.getValue(SpdySettingsFrame.SETTINGS_MAX_CONCURRENT_STREAMS); if (newConcurrentStreams >= 0) { diff --git a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySettingsFrame.java b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySettingsFrame.java index 4957d7f65e..95e98bf711 100644 --- a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdySettingsFrame.java +++ b/src/main/java/org/jboss/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. */ void 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. */ void setValue(int id, int value, boolean persistVal, boolean persisted); diff --git a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyVersion.java b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyVersion.java index efc9b99476..fcff4ad400 100644 --- a/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyVersion.java +++ b/src/main/java/org/jboss/netty/handler/codec/spdy/SpdyVersion.java @@ -16,14 +16,16 @@ package org.jboss.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 minorVerison; private final boolean sessionFlowControl; - private SpdyVersion(int version, boolean sessionFlowControl) { + private SpdyVersion(int version, int minorVersion, boolean sessionFlowControl) { this.version = version; + this.minorVerison = minorVersion; this.sessionFlowControl = sessionFlowControl; } @@ -31,6 +33,10 @@ public enum SpdyVersion { return version; } + int getMinorVersion() { + return minorVerison; + } + boolean useSessionFlowControl() { return sessionFlowControl; }