From b42017b942aefec505860c854406896b7a5c6fa8 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 8 Jul 2018 14:44:38 +0000 Subject: [PATCH 01/26] WIP Signed-off-by: Greg Wilkins --- jetty-home/src/main/resources/bin/jetty.sh | 618 ------------------ .../jetty/http2/hpack/HpackDecoder.java | 6 +- .../jetty/http2/hpack/MetaDataBuilder.java | 120 +++- .../src/main/webapp/META-INF/MANIFEST.MF | 21 - .../src/main/webapp/stylesheet.css~ | 7 - .../src/main/webapp/META-INF/MANIFEST.MF | 3 - 6 files changed, 104 insertions(+), 671 deletions(-) delete mode 100755 jetty-home/src/main/resources/bin/jetty.sh delete mode 100644 tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF delete mode 100644 tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/webapp/stylesheet.css~ delete mode 100644 tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF diff --git a/jetty-home/src/main/resources/bin/jetty.sh b/jetty-home/src/main/resources/bin/jetty.sh deleted file mode 100755 index b2b69a3d75c..00000000000 --- a/jetty-home/src/main/resources/bin/jetty.sh +++ /dev/null @@ -1,618 +0,0 @@ -#!/usr/bin/env bash - -# LSB Tags -### BEGIN INIT INFO -# Provides: jetty -# Required-Start: $local_fs $network -# Required-Stop: $local_fs $network -# Default-Start: 2 3 4 5 -# Default-Stop: 0 1 6 -# Short-Description: Jetty start script. -# Description: Start Jetty web server. -### END INIT INFO - -# Startup script for jetty under *nix systems (it works under NT/cygwin too). - -################################################## -# Set the name which is used by other variables. -# Defaults to the file name without extension. -################################################## -NAME=$(echo $(basename $0) | sed -e 's/^[SK][0-9]*//' -e 's/\.sh$//') - -# To get the service to restart correctly on reboot, uncomment below (3 lines): -# ======================== -# chkconfig: 3 99 99 -# description: Jetty 9 webserver -# processname: jetty -# ======================== - -# Configuration files -# -# /etc/default/$NAME -# If it exists, this is read at the start of script. It may perform any -# sequence of shell commands, like setting relevant environment variables. -# -# $HOME/.$NAMErc (e.g. $HOME/.jettyrc) -# If it exists, this is read at the start of script. It may perform any -# sequence of shell commands, like setting relevant environment variables. -# -# /etc/$NAME.conf -# If found, and no configurations were given on the command line, -# the file will be used as this script's configuration. -# Each line in the file may contain: -# - A comment denoted by the pound (#) sign as first non-blank character. -# - The path to a regular file, which will be passed to jetty as a -# config.xml file. -# - The path to a directory. Each *.xml file in the directory will be -# passed to jetty as a config.xml file. -# - All other lines will be passed, as-is to the start.jar -# -# The files will be checked for existence before being passed to jetty. -# -# Configuration variables -# -# JAVA -# Command to invoke Java. If not set, java (from the PATH) will be used. -# -# JAVA_OPTIONS -# Extra options to pass to the JVM -# -# JETTY_HOME -# Where Jetty is installed. If not set, the script will try go -# guess it by looking at the invocation path for the script -# The java system property "jetty.home" will be -# set to this value for use by configure.xml files, f.e.: -# -# /webapps/jetty.war -# -# JETTY_BASE -# Where your Jetty base directory is. If not set, the value from -# $JETTY_HOME will be used. -# -# JETTY_RUN -# Where the $NAME.pid file should be stored. It defaults to the -# first available of /var/run, /usr/var/run, JETTY_BASE and /tmp -# if not set. -# -# JETTY_PID -# The Jetty PID file, defaults to $JETTY_RUN/$NAME.pid -# -# JETTY_ARGS -# The default arguments to pass to jetty. -# For example -# JETTY_ARGS=jetty.http.port=8080 jetty.ssl.port=8443 -# -# JETTY_USER -# if set, then used as a username to run the server as -# -# JETTY_SHELL -# If set, then used as the shell by su when starting the server. Will have -# no effect if start-stop-daemon exists. Useful when JETTY_USER does not -# have shell access, e.g. /bin/false -# -# JETTY_START_TIMEOUT -# Time spent waiting to see if startup was successful/failed. Defaults to 60 seconds -# - -usage() -{ - echo "Usage: ${0##*/} [-d] {start|stop|run|restart|check|supervise} [ CONFIGS ... ] " - exit 1 -} - -[ $# -gt 0 ] || usage - - -################################################## -# Some utility functions -################################################## -findDirectory() -{ - local L OP=$1 - shift - for L in "$@"; do - [ "$OP" "$L" ] || continue - printf %s "$L" - break - done -} - -running() -{ - if [ -f "$1" ] - then - local PID=$(cat "$1" 2>/dev/null) || return 1 - kill -0 "$PID" 2>/dev/null - return - fi - rm -f "$1" - return 1 -} - -started() -{ - # wait for 60s to see "STARTED" in PID file, needs jetty-started.xml as argument - for ((T = 0; T < $(($3 / 4)); T++)) - do - sleep 4 - [ -z "$(grep STARTED $1 2>/dev/null)" ] || return 0 - [ -z "$(grep STOPPED $1 2>/dev/null)" ] || return 1 - [ -z "$(grep FAILED $1 2>/dev/null)" ] || return 1 - local PID=$(cat "$2" 2>/dev/null) || return 1 - kill -0 "$PID" 2>/dev/null || return 1 - echo -n ". " - done - - return 1; -} - - -readConfig() -{ - (( DEBUG )) && echo "Reading $1.." - source "$1" -} - -dumpEnv() -{ - echo "JAVA = $JAVA" - echo "JAVA_OPTIONS = ${JAVA_OPTIONS[*]}" - echo "JETTY_HOME = $JETTY_HOME" - echo "JETTY_BASE = $JETTY_BASE" - echo "START_D = $START_D" - echo "START_INI = $START_INI" - echo "JETTY_START = $JETTY_START" - echo "JETTY_CONF = $JETTY_CONF" - echo "JETTY_ARGS = ${JETTY_ARGS[*]}" - echo "JETTY_RUN = $JETTY_RUN" - echo "JETTY_PID = $JETTY_PID" - echo "JETTY_START_LOG = $JETTY_START_LOG" - echo "JETTY_STATE = $JETTY_STATE" - echo "JETTY_START_TIMEOUT = $JETTY_START_TIMEOUT" - echo "RUN_CMD = ${RUN_CMD[*]}" -} - - - -################################################## -# Get the action & configs -################################################## -CONFIGS=() -NO_START=0 -DEBUG=0 - -while [[ $1 = -* ]]; do - case $1 in - -d) DEBUG=1 ;; - esac - shift -done -ACTION=$1 -shift - -################################################## -# Read any configuration files -################################################## -ETC=/etc -if [ $UID != 0 ] -then - ETC=$HOME/etc -fi - -for CONFIG in {/etc,~/etc}/default/${NAME}{,9} $HOME/.${NAME}rc; do - if [ -f "$CONFIG" ] ; then - readConfig "$CONFIG" - fi -done - - -################################################## -# Set tmp if not already set. -################################################## -TMPDIR=${TMPDIR:-/tmp} - -################################################## -# Jetty's hallmark -################################################## -JETTY_INSTALL_TRACE_FILE="start.jar" - - -################################################## -# Try to determine JETTY_HOME if not set -################################################## -if [ -z "$JETTY_HOME" ] -then - JETTY_SH=$0 - case "$JETTY_SH" in - /*) JETTY_HOME=${JETTY_SH%/*/*} ;; - ./*/*) JETTY_HOME=${JETTY_SH%/*/*} ;; - ./*) JETTY_HOME=.. ;; - */*/*) JETTY_HOME=./${JETTY_SH%/*/*} ;; - */*) JETTY_HOME=. ;; - *) JETTY_HOME=.. ;; - esac - - if [ ! -f "$JETTY_HOME/$JETTY_INSTALL_TRACE_FILE" ] - then - JETTY_HOME= - fi -fi - - -################################################## -# No JETTY_HOME yet? We're out of luck! -################################################## -if [ -z "$JETTY_HOME" ]; then - echo "** ERROR: JETTY_HOME not set, you need to set it or install in a standard location" - exit 1 -fi - -cd "$JETTY_HOME" -JETTY_HOME=$PWD - - -################################################## -# Set JETTY_BASE -################################################## -if [ -z "$JETTY_BASE" ]; then - JETTY_BASE=$JETTY_HOME -fi - -cd "$JETTY_BASE" -JETTY_BASE=$PWD - - -##################################################### -# Check that jetty is where we think it is -##################################################### -if [ ! -r "$JETTY_HOME/$JETTY_INSTALL_TRACE_FILE" ] -then - echo "** ERROR: Oops! Jetty doesn't appear to be installed in $JETTY_HOME" - echo "** ERROR: $JETTY_HOME/$JETTY_INSTALL_TRACE_FILE is not readable!" - exit 1 -fi - -################################################## -# Try to find this script's configuration file, -# but only if no configurations were given on the -# command line. -################################################## -if [ -z "$JETTY_CONF" ] -then - if [ -f $ETC/${NAME}.conf ] - then - JETTY_CONF=$ETC/${NAME}.conf - elif [ -f "$JETTY_BASE/etc/jetty.conf" ] - then - JETTY_CONF=$JETTY_BASE/etc/jetty.conf - elif [ -f "$JETTY_HOME/etc/jetty.conf" ] - then - JETTY_CONF=$JETTY_HOME/etc/jetty.conf - fi -fi - -##################################################### -# Find a location for the pid file -##################################################### -if [ -z "$JETTY_RUN" ] -then - JETTY_RUN=$(findDirectory -w /var/run /usr/var/run $JETTY_BASE /tmp)/jetty - [ -d "$JETTY_RUN" ] || mkdir $JETTY_RUN -fi - -##################################################### -# define start log location -##################################################### -if [ -z "$JETTY_START_LOG" ] -then - JETTY_START_LOG="$JETTY_RUN/$NAME-start.log" -fi - -##################################################### -# Find a pid and state file -##################################################### -if [ -z "$JETTY_PID" ] -then - JETTY_PID="$JETTY_RUN/${NAME}.pid" -fi - -if [ -z "$JETTY_STATE" ] -then - JETTY_STATE=$JETTY_BASE/${NAME}.state -fi - -case "`uname`" in -CYGWIN*) JETTY_STATE="`cygpath -w $JETTY_STATE`";; -esac - - -JETTY_ARGS=(${JETTY_ARGS[*]} "jetty.state=$JETTY_STATE") - -################################################## -# Get the list of config.xml files from jetty.conf -################################################## -if [ -f "$JETTY_CONF" ] && [ -r "$JETTY_CONF" ] -then - while read -r CONF - do - if expr "$CONF" : '#' >/dev/null ; then - continue - fi - - if [ -d "$CONF" ] - then - # assume it's a directory with configure.xml files - # for example: /etc/jetty.d/ - # sort the files before adding them to the list of JETTY_ARGS - for XMLFILE in "$CONF/"*.xml - do - if [ -r "$XMLFILE" ] && [ -f "$XMLFILE" ] - then - JETTY_ARGS=(${JETTY_ARGS[*]} "$XMLFILE") - else - echo "** WARNING: Cannot read '$XMLFILE' specified in '$JETTY_CONF'" - fi - done - else - # assume it's a command line parameter (let start.jar deal with its validity) - JETTY_ARGS=(${JETTY_ARGS[*]} "$CONF") - fi - done < "$JETTY_CONF" -fi - -################################################## -# Setup JAVA if unset -################################################## -if [ -z "$JAVA" ] -then - JAVA=$(which java) -fi - -if [ -z "$JAVA" ] -then - echo "Cannot find a Java JDK. Please set either set JAVA or put java (>=1.5) in your PATH." >&2 - exit 1 -fi - -##################################################### -# See if Deprecated JETTY_LOGS is defined -##################################################### -if [ "$JETTY_LOGS" ] -then - echo "** WARNING: JETTY_LOGS is Deprecated. Please configure logging within the jetty base." >&2 -fi - -##################################################### -# Set STARTED timeout -##################################################### -if [ -z "$JETTY_START_TIMEOUT"] -then - JETTY_START_TIMEOUT=60 -fi - -##################################################### -# Are we running on Windows? Could be, with Cygwin/NT. -##################################################### -case "`uname`" in -CYGWIN*) PATH_SEPARATOR=";";; -*) PATH_SEPARATOR=":";; -esac - - -##################################################### -# Add jetty properties to Java VM options. -##################################################### - -case "`uname`" in -CYGWIN*) -JETTY_HOME="`cygpath -w $JETTY_HOME`" -JETTY_BASE="`cygpath -w $JETTY_BASE`" -TMPDIR="`cygpath -w $TMPDIR`" -;; -esac - -JAVA_OPTIONS=(${JAVA_OPTIONS[*]} "-Djetty.home=$JETTY_HOME" "-Djetty.base=$JETTY_BASE" "-Djava.io.tmpdir=$TMPDIR") - -##################################################### -# This is how the Jetty server will be started -##################################################### - -JETTY_START=$JETTY_HOME/start.jar -START_INI=$JETTY_BASE/start.ini -START_D=$JETTY_BASE/start.d -if [ ! -f "$START_INI" -a ! -d "$START_D" ] -then - echo "Cannot find a start.ini file or a start.d directory in your JETTY_BASE directory: $JETTY_BASE" >&2 - exit 1 -fi - -case "`uname`" in -CYGWIN*) JETTY_START="`cygpath -w $JETTY_START`";; -esac - -RUN_ARGS=(${JAVA_OPTIONS[@]} -jar "$JETTY_START" ${JETTY_ARGS[*]}) -RUN_CMD=("$JAVA" ${RUN_ARGS[@]}) - -##################################################### -# Comment these out after you're happy with what -# the script is doing. -##################################################### -if (( DEBUG )) -then - dumpEnv -fi - -################################################## -# Do the action -################################################## -case "$ACTION" in - start) - echo -n "Starting Jetty: " - - if (( NO_START )); then - echo "Not starting ${NAME} - NO_START=1"; - exit - fi - - if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1 - then - unset CH_USER - if [ -n "$JETTY_USER" ] - then - CH_USER="-c$JETTY_USER" - fi - - start-stop-daemon -S -p"$JETTY_PID" $CH_USER -d"$JETTY_BASE" -b -m -a "$JAVA" -- "${RUN_ARGS[@]}" start-log-file="$JETTY_START_LOG" - - else - - if running $JETTY_PID - then - echo "Already Running $(cat $JETTY_PID)!" - exit 1 - fi - - if [ -n "$JETTY_USER" ] && [ `whoami` != "$JETTY_USER" ] - then - unset SU_SHELL - if [ "$JETTY_SHELL" ] - then - SU_SHELL="-s $JETTY_SHELL" - fi - - touch "$JETTY_PID" - chown "$JETTY_USER" "$JETTY_PID" - # FIXME: Broken solution: wordsplitting, pathname expansion, arbitrary command execution, etc. - su - "$JETTY_USER" $SU_SHELL -c " - cd \"$JETTY_BASE\" - exec ${RUN_CMD[*]} start-log-file=\"$JETTY_START_LOG\" > /dev/null & - disown \$! - echo \$! > \"$JETTY_PID\"" - else - "${RUN_CMD[@]}" > /dev/null & - disown $! - echo $! > "$JETTY_PID" - fi - - fi - - if expr "${JETTY_ARGS[*]}" : '.*jetty-started.xml.*' >/dev/null - then - if started "$JETTY_STATE" "$JETTY_PID" "$JETTY_START_TIMEOUT" - then - echo "OK `date`" - else - echo "FAILED `date`" - exit 1 - fi - else - echo "ok `date`" - fi - - ;; - - stop) - echo -n "Stopping Jetty: " - if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1; then - start-stop-daemon -K -p"$JETTY_PID" -d"$JETTY_HOME" -a "$JAVA" -s HUP - - TIMEOUT=30 - while running "$JETTY_PID"; do - if (( TIMEOUT-- == 0 )); then - start-stop-daemon -K -p"$JETTY_PID" -d"$JETTY_HOME" -a "$JAVA" -s KILL - fi - - sleep 1 - done - else - if [ ! -f "$JETTY_PID" ] ; then - echo "ERROR: no pid found at $JETTY_PID" - exit 1 - fi - - PID=$(cat "$JETTY_PID" 2>/dev/null) - if [ -z "$PID" ] ; then - echo "ERROR: no pid id found in $JETTY_PID" - exit 1 - fi - kill "$PID" 2>/dev/null - - TIMEOUT=30 - while running $JETTY_PID; do - if (( TIMEOUT-- == 0 )); then - kill -KILL "$PID" 2>/dev/null - fi - - sleep 1 - done - fi - - rm -f "$JETTY_PID" - rm -f "$JETTY_STATE" - echo OK - - ;; - - restart) - JETTY_SH=$0 - > "$JETTY_STATE" - if [ ! -f $JETTY_SH ]; then - if [ ! -f $JETTY_HOME/bin/jetty.sh ]; then - echo "$JETTY_HOME/bin/jetty.sh does not exist." - exit 1 - fi - JETTY_SH=$JETTY_HOME/bin/jetty.sh - fi - - "$JETTY_SH" stop "$@" - "$JETTY_SH" start "$@" - - ;; - - supervise) - # - # Under control of daemontools supervise monitor which - # handles restarts and shutdowns via the svc program. - # - exec "${RUN_CMD[@]}" - - ;; - - run|demo) - echo "Running Jetty: " - - if running "$JETTY_PID" - then - echo Already Running $(cat "$JETTY_PID")! - exit 1 - fi - - exec "${RUN_CMD[@]}" - ;; - - check|status) - if running "$JETTY_PID" - then - echo "Jetty running pid=$(< "$JETTY_PID")" - else - echo "Jetty NOT running" - fi - echo - dumpEnv - echo - - if running "$JETTY_PID" - then - exit 0 - fi - exit 1 - - ;; - - *) - usage - - ;; -esac - -exit 0 diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 28cb913c34f..e730718c643 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.hpack.HpackContext.Entry; + import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -66,14 +67,15 @@ public class HpackDecoder _localMaxDynamicTableSize=localMaxdynamciTableSize; } - public MetaData decode(ByteBuffer buffer) + public MetaData decode(ByteBuffer buffer) throws HpackException { if (LOG.isDebugEnabled()) LOG.debug(String.format("CtxTbl[%x] decoding %d octets",_context.hashCode(),buffer.remaining())); // If the buffer is big, don't even think about decoding it if (buffer.remaining()>_builder.getMaxSize()) - throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,"Header frame size "+buffer.remaining()+">"+_builder.getMaxSize()); + throw new HpackException.Session("431 Request Header Fields too large",6); + //throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,"Header frame size "+buffer.remaining()+">"+_builder.getMaxSize()); while(buffer.hasRemaining()) { diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index b9e72e7352f..c0f5266473e 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -29,18 +29,22 @@ import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.hpack.HpackException.Session; public class MetaDataBuilder { private final int _maxSize; private int _size; - private int _status; + private int _status=-1; private String _method; private HttpScheme _scheme; private HostPortHttpField _authority; private String _path; private long _contentLength=Long.MIN_VALUE; private HttpFields _fields = new HttpFields(10); + private HpackException.Stream _streamException; + private boolean _request; + private boolean _response; /** * @param maxHeadersSize The maximum size of the headers, expressed as total name and value characters. @@ -66,7 +70,7 @@ public class MetaDataBuilder return _size; } - public void emit(HttpField field) + public void emit(HttpField field) throws HpackException.Session { HttpHeader header = field.getHeader(); String name = field.getName(); @@ -74,7 +78,7 @@ public class MetaDataBuilder int field_size = name.length() + (value == null ? 0 : value.length()); _size+=field_size+32; if (_size>_maxSize) - throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,"Header size "+_size+">"+_maxSize); + throw new HpackException.Session("Header Size %d > %d",_size,_maxSize); if (field instanceof StaticTableHttpField) { @@ -82,15 +86,21 @@ public class MetaDataBuilder switch(header) { case C_STATUS: - _status=(Integer)staticField.getStaticValue(); + if(checkHeader(header, _status)) + _status = (Integer)staticField.getStaticValue(); + _response = true; break; case C_METHOD: - _method=value; + if(checkPseudoHeader(header, _method)) + _method = value; + _request = true; break; case C_SCHEME: - _scheme = (HttpScheme)staticField.getStaticValue(); + if(checkPseudoHeader(header, _scheme)) + _scheme = (HttpScheme)staticField.getStaticValue(); + _request = true; break; default: @@ -102,23 +112,32 @@ public class MetaDataBuilder switch(header) { case C_STATUS: - _status=field.getIntValue(); + if(checkHeader(header, _status)) + _status = field.getIntValue(); + _response = true; break; case C_METHOD: - _method=value; + if(checkPseudoHeader(header, _method)) + _method = value; + _request = true; break; case C_SCHEME: - if (value != null) + if(checkPseudoHeader(header, _scheme) && value != null) _scheme = HttpScheme.CACHE.get(value); + _request = true; break; case C_AUTHORITY: - if (field instanceof HostPortHttpField) - _authority = (HostPortHttpField)field; - else if (value != null) - _authority = new AuthorityHttpField(value); + if(checkPseudoHeader(header, _authority)) + { + if (field instanceof HostPortHttpField) + _authority = (HostPortHttpField)field; + else if (value != null) + _authority = new AuthorityHttpField(value); + } + _request = true; break; case HOST: @@ -134,29 +153,89 @@ public class MetaDataBuilder break; case C_PATH: - _path = value; + if(checkPseudoHeader(header, _path)) + _path = value; + _request = true; break; case CONTENT_LENGTH: _contentLength = field.getLongValue(); _fields.add(field); break; + + case TE: + if ("trailors".equalsIgnoreCase(value)) + _fields.add(field); + else + streamException("unsupported TE value %s", value); + break; + + case CONNECTION: + // TODO should other connection specific fields be listed here? + streamException("Connection specific field %s", header); + break; - default: - if (name.charAt(0)!=':') + default: + if (name.charAt(0)==':') + streamException("Unknown psuodo header %s", name); + else _fields.add(field); break; } } else { - if (name.charAt(0)!=':') + if (name.charAt(0)==':') + streamException("Unknown psuedo header %s",name); + else _fields.add(field); } } - public MetaData build() + private void streamException(String messageFormat, Object... args) { + HpackException.Stream stream = new HpackException.Stream(messageFormat, args); + if (_streamException==null) + _streamException = stream; + else + _streamException.addSuppressed(stream); + } + + private boolean checkHeader(HttpHeader header, int value) + { + if (_fields.size()>0) + { + streamException("Psuedo header %s after fields", header.asString()); + return false; + } + if (value==-1) + return true; + streamException("Duplicate psuedo header %s", header.asString()); + return false; + } + + private boolean checkPseudoHeader(HttpHeader header, Object value) + { + if (_fields.size()>0) + { + streamException("Psuedo header %s after fields", header.asString()); + return false; + } + if (value==null) + return true; + streamException("Duplicate psuedo header %s", header.asString()); + return false; + } + + + public MetaData build() throws HpackException.Stream + { + if (_streamException!=null) + throw _streamException; + + if (_request && _response) + throw new HpackException.Stream("Request and Response headers"); + try { HttpFields fields = _fields; @@ -189,13 +268,14 @@ public class MetaDataBuilder * Check that the max size will not be exceeded. * @param length the length * @param huffman the huffman name + * @throws Session */ - public void checkSize(int length, boolean huffman) + public void checkSize(int length, boolean huffman) throws Session { // Apply a huffman fudge factor if (huffman) length=(length*4)/3; if ((_size+length)>_maxSize) - throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,"Header size "+(_size+length)+">"+_maxSize); + throw new HpackException.Session("Header too large %d > %d", _size+length, _maxSize); } } diff --git a/tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF b/tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF deleted file mode 100644 index 886d243c096..00000000000 --- a/tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF +++ /dev/null @@ -1,21 +0,0 @@ -Manifest-Version: 1.0 -Bundle-ManifestVersion: 2 -Bundle-Name: TestIt -Bundle-SymbolicName: TestIt -Bundle-Version: 1.0.0.qualifier -Bundle-Activator: testit.Activator -Import-Package: javax.servlet;version="2.6", - javax.servlet.http;version="2.6", - javax.servlet.jsp, - javax.servlet.jsp.tagext -Require-Bundle: org.eclipse.jetty.client, - org.eclipse.jetty.proxy, - org.eclipse.jetty.http, - org.eclipse.jetty.io, - org.eclipse.jetty.util -Bundle-ClassPath: WEB-INF/classes -Bundle-RequiredExecutionEnvironment: JavaSE-1.7 -Bundle-ActivationPolicy: lazy -Web-ContextPath: / -Class-Path: - diff --git a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/webapp/stylesheet.css~ b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/webapp/stylesheet.css~ deleted file mode 100644 index def6847d14c..00000000000 --- a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/webapp/stylesheet.css~ +++ /dev/null @@ -1,7 +0,0 @@ -body {color: #2E2E2E; font-family:sans-serif; font-size:90%;} -h1 {font-variant: small-caps; font-size:130%; letter-spacing: 0.1em;} -h2 {font-variant: small-caps; font-size:100%; letter-spacing: 0.1em; margin-top:2em} -h3 {font-size:100%; letter-spacing: 0.1em;} - -span.pass { color: green; } -span.fail { color:red; } diff --git a/tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF b/tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF deleted file mode 100644 index 5e9495128c0..00000000000 --- a/tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF +++ /dev/null @@ -1,3 +0,0 @@ -Manifest-Version: 1.0 -Class-Path: - From 10ec53319ab4c2b10471eb142365b6a02423f626 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 8 Jul 2018 14:56:42 +0000 Subject: [PATCH 02/26] WIP Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/MetaDataBuilder.java | 2 - .../jetty/http2/hpack/HpackDecoderTest.java | 46 +++++++++---------- .../eclipse/jetty/http2/hpack/HpackTest.java | 6 +-- 3 files changed, 24 insertions(+), 30 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index c0f5266473e..7a5d7e158e3 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -20,13 +20,11 @@ package org.eclipse.jetty.http2.hpack; -import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.hpack.HpackException.Session; diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index d6cb341aca9..1b3acc7338f 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -19,24 +19,6 @@ package org.eclipse.jetty.http2.hpack; -import java.nio.ByteBuffer; -import java.util.Iterator; - -import org.eclipse.jetty.http.BadMessageException; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpScheme; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.http.MetaData; -import org.eclipse.jetty.util.BufferUtil; -import org.eclipse.jetty.util.TypeUtil; -import org.eclipse.jetty.util.log.Log; -import org.hamcrest.Matchers; -import org.junit.Assert; -import org.junit.Test; - import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; @@ -44,10 +26,24 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import java.nio.ByteBuffer; +import java.util.Iterator; + +import org.eclipse.jetty.http.BadMessageException; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.util.TypeUtil; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Test; + public class HpackDecoderTest { @Test - public void testDecodeD_3() + public void testDecodeD_3() throws Exception { HpackDecoder decoder = new HpackDecoder(4096,8192); @@ -95,7 +91,7 @@ public class HpackDecoderTest } @Test - public void testDecodeD_4() + public void testDecodeD_4() throws Exception { HpackDecoder decoder = new HpackDecoder(4096,8192); @@ -128,7 +124,7 @@ public class HpackDecoderTest } @Test - public void testDecodeWithArrayOffset() + public void testDecodeWithArrayOffset() throws Exception { String value = "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ=="; @@ -152,7 +148,7 @@ public class HpackDecoderTest } @Test - public void testDecodeHuffmanWithArrayOffset() + public void testDecodeHuffmanWithArrayOffset() throws Exception { HpackDecoder decoder = new HpackDecoder(4096,8192); @@ -172,7 +168,7 @@ public class HpackDecoderTest } @Test - public void testNghttpx() + public void testNghttpx() throws Exception { // Response encoded by nghttpx String encoded="886196C361Be940b6a65B6850400B8A00571972e080a62D1Bf5f87497cA589D34d1f9a0f0d0234327690Aa69D29aFcA954D3A5358980Ae112e0f7c880aE152A9A74a6bF3"; @@ -204,7 +200,7 @@ public class HpackDecoderTest } @Test - public void testTooBigToIndex() + public void testTooBigToIndex() throws Exception { String encoded = "44FfEc02Df3990A190A0D4Ee5b3d2940Ec98Aa4a62D127D29e273a0aA20dEcAa190a503b262d8a2671D4A2672a927aA874988a2471D05510750c951139EdA2452a3a548cAa1aA90bE4B228342864A9E0D450A5474a92992a1aA513395448E3A0Aa17B96cFe3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f14E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F353F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F54f"; ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); @@ -217,7 +213,7 @@ public class HpackDecoderTest } @Test - public void testUnknownIndex() + public void testUnknownIndex() throws Exception { String encoded = "BE"; ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java index 31c1ee21d8f..9c7d2a24eb5 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java @@ -46,7 +46,7 @@ public class HpackTest final static HttpField Date = new PreEncodedHttpField(HttpHeader.DATE,DateGenerator.formatDate(TimeUnit.NANOSECONDS.toMillis(System.nanoTime()))); @Test - public void encodeDecodeResponseTest() + public void encodeDecodeResponseTest() throws Exception { HpackEncoder encoder = new HpackEncoder(); HpackDecoder decoder = new HpackDecoder(4096,8192); @@ -99,7 +99,7 @@ public class HpackTest } @Test - public void encodeDecodeTooLargeTest() + public void encodeDecodeTooLargeTest() throws Exception { HpackEncoder encoder = new HpackEncoder(); HpackDecoder decoder = new HpackDecoder(4096,164); @@ -138,7 +138,7 @@ public class HpackTest } @Test - public void evictReferencedFieldTest() + public void evictReferencedFieldTest() throws Exception { HpackEncoder encoder = new HpackEncoder(200,200); HpackDecoder decoder = new HpackDecoder(200,1024); From 9fd80e8524d62ec2ebf8f8235b50fbf49e571752 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 8 Jul 2018 15:17:39 +0000 Subject: [PATCH 03/26] WIP Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/HpackException.java | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java new file mode 100644 index 00000000000..6eebf27c92b --- /dev/null +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java @@ -0,0 +1,54 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.hpack; + +import java.io.IOException; + +public abstract class HpackException extends IOException +{ + HpackException(String messageFormat, Object... args) + { + super(String.format(messageFormat, args)); + } + + public static class Stream extends HpackException + { + Stream(String messageFormat, Object... args) + { + super(messageFormat,args); + } + } + + + public static class Session extends HpackException + { + Session(String messageFormat, Object... args) + { + super(messageFormat,args); + } + } + + public static class Compression extends Session + { + public Compression(String messageFormat, Object... args) + { + super(messageFormat,args); + } + } +} From cb6c582333fbb7a3645033f45325f92a6add56e2 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 8 Jul 2018 15:24:55 +0000 Subject: [PATCH 04/26] WIP Signed-off-by: Greg Wilkins --- jetty-home/src/main/resources/bin/jetty.sh | 618 ++++++++++++++++++ .../src/main/webapp/META-INF/MANIFEST.MF | 3 + 2 files changed, 621 insertions(+) create mode 100755 jetty-home/src/main/resources/bin/jetty.sh create mode 100644 tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF diff --git a/jetty-home/src/main/resources/bin/jetty.sh b/jetty-home/src/main/resources/bin/jetty.sh new file mode 100755 index 00000000000..b2b69a3d75c --- /dev/null +++ b/jetty-home/src/main/resources/bin/jetty.sh @@ -0,0 +1,618 @@ +#!/usr/bin/env bash + +# LSB Tags +### BEGIN INIT INFO +# Provides: jetty +# Required-Start: $local_fs $network +# Required-Stop: $local_fs $network +# Default-Start: 2 3 4 5 +# Default-Stop: 0 1 6 +# Short-Description: Jetty start script. +# Description: Start Jetty web server. +### END INIT INFO + +# Startup script for jetty under *nix systems (it works under NT/cygwin too). + +################################################## +# Set the name which is used by other variables. +# Defaults to the file name without extension. +################################################## +NAME=$(echo $(basename $0) | sed -e 's/^[SK][0-9]*//' -e 's/\.sh$//') + +# To get the service to restart correctly on reboot, uncomment below (3 lines): +# ======================== +# chkconfig: 3 99 99 +# description: Jetty 9 webserver +# processname: jetty +# ======================== + +# Configuration files +# +# /etc/default/$NAME +# If it exists, this is read at the start of script. It may perform any +# sequence of shell commands, like setting relevant environment variables. +# +# $HOME/.$NAMErc (e.g. $HOME/.jettyrc) +# If it exists, this is read at the start of script. It may perform any +# sequence of shell commands, like setting relevant environment variables. +# +# /etc/$NAME.conf +# If found, and no configurations were given on the command line, +# the file will be used as this script's configuration. +# Each line in the file may contain: +# - A comment denoted by the pound (#) sign as first non-blank character. +# - The path to a regular file, which will be passed to jetty as a +# config.xml file. +# - The path to a directory. Each *.xml file in the directory will be +# passed to jetty as a config.xml file. +# - All other lines will be passed, as-is to the start.jar +# +# The files will be checked for existence before being passed to jetty. +# +# Configuration variables +# +# JAVA +# Command to invoke Java. If not set, java (from the PATH) will be used. +# +# JAVA_OPTIONS +# Extra options to pass to the JVM +# +# JETTY_HOME +# Where Jetty is installed. If not set, the script will try go +# guess it by looking at the invocation path for the script +# The java system property "jetty.home" will be +# set to this value for use by configure.xml files, f.e.: +# +# /webapps/jetty.war +# +# JETTY_BASE +# Where your Jetty base directory is. If not set, the value from +# $JETTY_HOME will be used. +# +# JETTY_RUN +# Where the $NAME.pid file should be stored. It defaults to the +# first available of /var/run, /usr/var/run, JETTY_BASE and /tmp +# if not set. +# +# JETTY_PID +# The Jetty PID file, defaults to $JETTY_RUN/$NAME.pid +# +# JETTY_ARGS +# The default arguments to pass to jetty. +# For example +# JETTY_ARGS=jetty.http.port=8080 jetty.ssl.port=8443 +# +# JETTY_USER +# if set, then used as a username to run the server as +# +# JETTY_SHELL +# If set, then used as the shell by su when starting the server. Will have +# no effect if start-stop-daemon exists. Useful when JETTY_USER does not +# have shell access, e.g. /bin/false +# +# JETTY_START_TIMEOUT +# Time spent waiting to see if startup was successful/failed. Defaults to 60 seconds +# + +usage() +{ + echo "Usage: ${0##*/} [-d] {start|stop|run|restart|check|supervise} [ CONFIGS ... ] " + exit 1 +} + +[ $# -gt 0 ] || usage + + +################################################## +# Some utility functions +################################################## +findDirectory() +{ + local L OP=$1 + shift + for L in "$@"; do + [ "$OP" "$L" ] || continue + printf %s "$L" + break + done +} + +running() +{ + if [ -f "$1" ] + then + local PID=$(cat "$1" 2>/dev/null) || return 1 + kill -0 "$PID" 2>/dev/null + return + fi + rm -f "$1" + return 1 +} + +started() +{ + # wait for 60s to see "STARTED" in PID file, needs jetty-started.xml as argument + for ((T = 0; T < $(($3 / 4)); T++)) + do + sleep 4 + [ -z "$(grep STARTED $1 2>/dev/null)" ] || return 0 + [ -z "$(grep STOPPED $1 2>/dev/null)" ] || return 1 + [ -z "$(grep FAILED $1 2>/dev/null)" ] || return 1 + local PID=$(cat "$2" 2>/dev/null) || return 1 + kill -0 "$PID" 2>/dev/null || return 1 + echo -n ". " + done + + return 1; +} + + +readConfig() +{ + (( DEBUG )) && echo "Reading $1.." + source "$1" +} + +dumpEnv() +{ + echo "JAVA = $JAVA" + echo "JAVA_OPTIONS = ${JAVA_OPTIONS[*]}" + echo "JETTY_HOME = $JETTY_HOME" + echo "JETTY_BASE = $JETTY_BASE" + echo "START_D = $START_D" + echo "START_INI = $START_INI" + echo "JETTY_START = $JETTY_START" + echo "JETTY_CONF = $JETTY_CONF" + echo "JETTY_ARGS = ${JETTY_ARGS[*]}" + echo "JETTY_RUN = $JETTY_RUN" + echo "JETTY_PID = $JETTY_PID" + echo "JETTY_START_LOG = $JETTY_START_LOG" + echo "JETTY_STATE = $JETTY_STATE" + echo "JETTY_START_TIMEOUT = $JETTY_START_TIMEOUT" + echo "RUN_CMD = ${RUN_CMD[*]}" +} + + + +################################################## +# Get the action & configs +################################################## +CONFIGS=() +NO_START=0 +DEBUG=0 + +while [[ $1 = -* ]]; do + case $1 in + -d) DEBUG=1 ;; + esac + shift +done +ACTION=$1 +shift + +################################################## +# Read any configuration files +################################################## +ETC=/etc +if [ $UID != 0 ] +then + ETC=$HOME/etc +fi + +for CONFIG in {/etc,~/etc}/default/${NAME}{,9} $HOME/.${NAME}rc; do + if [ -f "$CONFIG" ] ; then + readConfig "$CONFIG" + fi +done + + +################################################## +# Set tmp if not already set. +################################################## +TMPDIR=${TMPDIR:-/tmp} + +################################################## +# Jetty's hallmark +################################################## +JETTY_INSTALL_TRACE_FILE="start.jar" + + +################################################## +# Try to determine JETTY_HOME if not set +################################################## +if [ -z "$JETTY_HOME" ] +then + JETTY_SH=$0 + case "$JETTY_SH" in + /*) JETTY_HOME=${JETTY_SH%/*/*} ;; + ./*/*) JETTY_HOME=${JETTY_SH%/*/*} ;; + ./*) JETTY_HOME=.. ;; + */*/*) JETTY_HOME=./${JETTY_SH%/*/*} ;; + */*) JETTY_HOME=. ;; + *) JETTY_HOME=.. ;; + esac + + if [ ! -f "$JETTY_HOME/$JETTY_INSTALL_TRACE_FILE" ] + then + JETTY_HOME= + fi +fi + + +################################################## +# No JETTY_HOME yet? We're out of luck! +################################################## +if [ -z "$JETTY_HOME" ]; then + echo "** ERROR: JETTY_HOME not set, you need to set it or install in a standard location" + exit 1 +fi + +cd "$JETTY_HOME" +JETTY_HOME=$PWD + + +################################################## +# Set JETTY_BASE +################################################## +if [ -z "$JETTY_BASE" ]; then + JETTY_BASE=$JETTY_HOME +fi + +cd "$JETTY_BASE" +JETTY_BASE=$PWD + + +##################################################### +# Check that jetty is where we think it is +##################################################### +if [ ! -r "$JETTY_HOME/$JETTY_INSTALL_TRACE_FILE" ] +then + echo "** ERROR: Oops! Jetty doesn't appear to be installed in $JETTY_HOME" + echo "** ERROR: $JETTY_HOME/$JETTY_INSTALL_TRACE_FILE is not readable!" + exit 1 +fi + +################################################## +# Try to find this script's configuration file, +# but only if no configurations were given on the +# command line. +################################################## +if [ -z "$JETTY_CONF" ] +then + if [ -f $ETC/${NAME}.conf ] + then + JETTY_CONF=$ETC/${NAME}.conf + elif [ -f "$JETTY_BASE/etc/jetty.conf" ] + then + JETTY_CONF=$JETTY_BASE/etc/jetty.conf + elif [ -f "$JETTY_HOME/etc/jetty.conf" ] + then + JETTY_CONF=$JETTY_HOME/etc/jetty.conf + fi +fi + +##################################################### +# Find a location for the pid file +##################################################### +if [ -z "$JETTY_RUN" ] +then + JETTY_RUN=$(findDirectory -w /var/run /usr/var/run $JETTY_BASE /tmp)/jetty + [ -d "$JETTY_RUN" ] || mkdir $JETTY_RUN +fi + +##################################################### +# define start log location +##################################################### +if [ -z "$JETTY_START_LOG" ] +then + JETTY_START_LOG="$JETTY_RUN/$NAME-start.log" +fi + +##################################################### +# Find a pid and state file +##################################################### +if [ -z "$JETTY_PID" ] +then + JETTY_PID="$JETTY_RUN/${NAME}.pid" +fi + +if [ -z "$JETTY_STATE" ] +then + JETTY_STATE=$JETTY_BASE/${NAME}.state +fi + +case "`uname`" in +CYGWIN*) JETTY_STATE="`cygpath -w $JETTY_STATE`";; +esac + + +JETTY_ARGS=(${JETTY_ARGS[*]} "jetty.state=$JETTY_STATE") + +################################################## +# Get the list of config.xml files from jetty.conf +################################################## +if [ -f "$JETTY_CONF" ] && [ -r "$JETTY_CONF" ] +then + while read -r CONF + do + if expr "$CONF" : '#' >/dev/null ; then + continue + fi + + if [ -d "$CONF" ] + then + # assume it's a directory with configure.xml files + # for example: /etc/jetty.d/ + # sort the files before adding them to the list of JETTY_ARGS + for XMLFILE in "$CONF/"*.xml + do + if [ -r "$XMLFILE" ] && [ -f "$XMLFILE" ] + then + JETTY_ARGS=(${JETTY_ARGS[*]} "$XMLFILE") + else + echo "** WARNING: Cannot read '$XMLFILE' specified in '$JETTY_CONF'" + fi + done + else + # assume it's a command line parameter (let start.jar deal with its validity) + JETTY_ARGS=(${JETTY_ARGS[*]} "$CONF") + fi + done < "$JETTY_CONF" +fi + +################################################## +# Setup JAVA if unset +################################################## +if [ -z "$JAVA" ] +then + JAVA=$(which java) +fi + +if [ -z "$JAVA" ] +then + echo "Cannot find a Java JDK. Please set either set JAVA or put java (>=1.5) in your PATH." >&2 + exit 1 +fi + +##################################################### +# See if Deprecated JETTY_LOGS is defined +##################################################### +if [ "$JETTY_LOGS" ] +then + echo "** WARNING: JETTY_LOGS is Deprecated. Please configure logging within the jetty base." >&2 +fi + +##################################################### +# Set STARTED timeout +##################################################### +if [ -z "$JETTY_START_TIMEOUT"] +then + JETTY_START_TIMEOUT=60 +fi + +##################################################### +# Are we running on Windows? Could be, with Cygwin/NT. +##################################################### +case "`uname`" in +CYGWIN*) PATH_SEPARATOR=";";; +*) PATH_SEPARATOR=":";; +esac + + +##################################################### +# Add jetty properties to Java VM options. +##################################################### + +case "`uname`" in +CYGWIN*) +JETTY_HOME="`cygpath -w $JETTY_HOME`" +JETTY_BASE="`cygpath -w $JETTY_BASE`" +TMPDIR="`cygpath -w $TMPDIR`" +;; +esac + +JAVA_OPTIONS=(${JAVA_OPTIONS[*]} "-Djetty.home=$JETTY_HOME" "-Djetty.base=$JETTY_BASE" "-Djava.io.tmpdir=$TMPDIR") + +##################################################### +# This is how the Jetty server will be started +##################################################### + +JETTY_START=$JETTY_HOME/start.jar +START_INI=$JETTY_BASE/start.ini +START_D=$JETTY_BASE/start.d +if [ ! -f "$START_INI" -a ! -d "$START_D" ] +then + echo "Cannot find a start.ini file or a start.d directory in your JETTY_BASE directory: $JETTY_BASE" >&2 + exit 1 +fi + +case "`uname`" in +CYGWIN*) JETTY_START="`cygpath -w $JETTY_START`";; +esac + +RUN_ARGS=(${JAVA_OPTIONS[@]} -jar "$JETTY_START" ${JETTY_ARGS[*]}) +RUN_CMD=("$JAVA" ${RUN_ARGS[@]}) + +##################################################### +# Comment these out after you're happy with what +# the script is doing. +##################################################### +if (( DEBUG )) +then + dumpEnv +fi + +################################################## +# Do the action +################################################## +case "$ACTION" in + start) + echo -n "Starting Jetty: " + + if (( NO_START )); then + echo "Not starting ${NAME} - NO_START=1"; + exit + fi + + if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1 + then + unset CH_USER + if [ -n "$JETTY_USER" ] + then + CH_USER="-c$JETTY_USER" + fi + + start-stop-daemon -S -p"$JETTY_PID" $CH_USER -d"$JETTY_BASE" -b -m -a "$JAVA" -- "${RUN_ARGS[@]}" start-log-file="$JETTY_START_LOG" + + else + + if running $JETTY_PID + then + echo "Already Running $(cat $JETTY_PID)!" + exit 1 + fi + + if [ -n "$JETTY_USER" ] && [ `whoami` != "$JETTY_USER" ] + then + unset SU_SHELL + if [ "$JETTY_SHELL" ] + then + SU_SHELL="-s $JETTY_SHELL" + fi + + touch "$JETTY_PID" + chown "$JETTY_USER" "$JETTY_PID" + # FIXME: Broken solution: wordsplitting, pathname expansion, arbitrary command execution, etc. + su - "$JETTY_USER" $SU_SHELL -c " + cd \"$JETTY_BASE\" + exec ${RUN_CMD[*]} start-log-file=\"$JETTY_START_LOG\" > /dev/null & + disown \$! + echo \$! > \"$JETTY_PID\"" + else + "${RUN_CMD[@]}" > /dev/null & + disown $! + echo $! > "$JETTY_PID" + fi + + fi + + if expr "${JETTY_ARGS[*]}" : '.*jetty-started.xml.*' >/dev/null + then + if started "$JETTY_STATE" "$JETTY_PID" "$JETTY_START_TIMEOUT" + then + echo "OK `date`" + else + echo "FAILED `date`" + exit 1 + fi + else + echo "ok `date`" + fi + + ;; + + stop) + echo -n "Stopping Jetty: " + if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1; then + start-stop-daemon -K -p"$JETTY_PID" -d"$JETTY_HOME" -a "$JAVA" -s HUP + + TIMEOUT=30 + while running "$JETTY_PID"; do + if (( TIMEOUT-- == 0 )); then + start-stop-daemon -K -p"$JETTY_PID" -d"$JETTY_HOME" -a "$JAVA" -s KILL + fi + + sleep 1 + done + else + if [ ! -f "$JETTY_PID" ] ; then + echo "ERROR: no pid found at $JETTY_PID" + exit 1 + fi + + PID=$(cat "$JETTY_PID" 2>/dev/null) + if [ -z "$PID" ] ; then + echo "ERROR: no pid id found in $JETTY_PID" + exit 1 + fi + kill "$PID" 2>/dev/null + + TIMEOUT=30 + while running $JETTY_PID; do + if (( TIMEOUT-- == 0 )); then + kill -KILL "$PID" 2>/dev/null + fi + + sleep 1 + done + fi + + rm -f "$JETTY_PID" + rm -f "$JETTY_STATE" + echo OK + + ;; + + restart) + JETTY_SH=$0 + > "$JETTY_STATE" + if [ ! -f $JETTY_SH ]; then + if [ ! -f $JETTY_HOME/bin/jetty.sh ]; then + echo "$JETTY_HOME/bin/jetty.sh does not exist." + exit 1 + fi + JETTY_SH=$JETTY_HOME/bin/jetty.sh + fi + + "$JETTY_SH" stop "$@" + "$JETTY_SH" start "$@" + + ;; + + supervise) + # + # Under control of daemontools supervise monitor which + # handles restarts and shutdowns via the svc program. + # + exec "${RUN_CMD[@]}" + + ;; + + run|demo) + echo "Running Jetty: " + + if running "$JETTY_PID" + then + echo Already Running $(cat "$JETTY_PID")! + exit 1 + fi + + exec "${RUN_CMD[@]}" + ;; + + check|status) + if running "$JETTY_PID" + then + echo "Jetty running pid=$(< "$JETTY_PID")" + else + echo "Jetty NOT running" + fi + echo + dumpEnv + echo + + if running "$JETTY_PID" + then + exit 0 + fi + exit 1 + + ;; + + *) + usage + + ;; +esac + +exit 0 diff --git a/tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF b/tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF new file mode 100644 index 00000000000..5e9495128c0 --- /dev/null +++ b/tests/test-webapps/test-webapp-rfc2616/src/main/webapp/META-INF/MANIFEST.MF @@ -0,0 +1,3 @@ +Manifest-Version: 1.0 +Class-Path: + From 09eb303d2bf8ea3d81e84b86db6d633097a9f26a Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 8 Jul 2018 15:28:05 +0000 Subject: [PATCH 05/26] WIP Signed-off-by: Greg Wilkins --- .../src/main/webapp/META-INF/MANIFEST.MF | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF diff --git a/tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF b/tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF new file mode 100644 index 00000000000..886d243c096 --- /dev/null +++ b/tests/test-webapps/test-proxy-webapp/src/main/webapp/META-INF/MANIFEST.MF @@ -0,0 +1,21 @@ +Manifest-Version: 1.0 +Bundle-ManifestVersion: 2 +Bundle-Name: TestIt +Bundle-SymbolicName: TestIt +Bundle-Version: 1.0.0.qualifier +Bundle-Activator: testit.Activator +Import-Package: javax.servlet;version="2.6", + javax.servlet.http;version="2.6", + javax.servlet.jsp, + javax.servlet.jsp.tagext +Require-Bundle: org.eclipse.jetty.client, + org.eclipse.jetty.proxy, + org.eclipse.jetty.http, + org.eclipse.jetty.io, + org.eclipse.jetty.util +Bundle-ClassPath: WEB-INF/classes +Bundle-RequiredExecutionEnvironment: JavaSE-1.7 +Bundle-ActivationPolicy: lazy +Web-ContextPath: / +Class-Path: + From 44801d8ff3130250e784126086c7f13bd177d9e9 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 8 Jul 2018 18:08:36 +0000 Subject: [PATCH 06/26] WIP Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http2/hpack/HpackDecoder.java | 12 +++++------- .../eclipse/jetty/http2/hpack/HpackException.java | 2 +- .../eclipse/jetty/http2/hpack/MetaDataBuilder.java | 2 +- .../eclipse/jetty/http2/hpack/HpackDecoderTest.java | 5 ++--- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index e730718c643..0f6ff926361 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -21,7 +21,6 @@ package org.eclipse.jetty.http2.hpack; import java.nio.ByteBuffer; -import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; @@ -75,7 +74,6 @@ public class HpackDecoder // If the buffer is big, don't even think about decoding it if (buffer.remaining()>_builder.getMaxSize()) throw new HpackException.Session("431 Request Header Fields too large",6); - //throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,"Header frame size "+buffer.remaining()+">"+_builder.getMaxSize()); while(buffer.hasRemaining()) { @@ -94,10 +92,9 @@ public class HpackDecoder int index = NBitInteger.decode(buffer,7); Entry entry=_context.get(index); if (entry==null) - { - throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Unknown index "+index); - } - else if (entry.isStatic()) + throw new HpackException.Session("Unknown index %d",index); + + if (entry.isStatic()) { if (LOG.isDebugEnabled()) LOG.debug("decode IdxStatic {}",entry); @@ -180,7 +177,8 @@ public class HpackDecoder char c=name.charAt(i); if (c>='A'&&c<='Z') { - throw new BadMessageException(400,"Uppercase header name"); + _builder.streamException("Uppercase header name %s",name); + break; } } header=HttpHeader.CACHE.get(name); diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java index 6eebf27c92b..6e3655c2b70 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java @@ -20,7 +20,7 @@ package org.eclipse.jetty.http2.hpack; import java.io.IOException; -public abstract class HpackException extends IOException +public abstract class HpackException extends RuntimeException { HpackException(String messageFormat, Object... args) { diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index 7a5d7e158e3..dbfb698beba 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -190,7 +190,7 @@ public class MetaDataBuilder } } - private void streamException(String messageFormat, Object... args) + void streamException(String messageFormat, Object... args) { HpackException.Stream stream = new HpackException.Stream(messageFormat, args); if (_streamException==null) diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 1b3acc7338f..1fb9d4ced9c 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -224,10 +224,9 @@ public class HpackDecoderTest decoder.decode(buffer); Assert.fail(); } - catch (BadMessageException e) + catch (HpackException.Session e) { - assertThat(e.getCode(),equalTo(HttpStatus.BAD_REQUEST_400)); - assertThat(e.getReason(),Matchers.startsWith("Unknown index")); + assertThat(e.getMessage(),Matchers.startsWith("Unknown index")); } } From d8fcd874eef1a9a0096a6aef24d1598620dd3fc3 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Jul 2018 09:43:52 +0200 Subject: [PATCH 07/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed unknown frame type behavior to follow the spec, i.e. ignore it. Signed-off-by: Simone Bordet --- .../eclipse/jetty/http2/parser/Parser.java | 12 +++- .../jetty/http2/parser/UnknownBodyParser.java | 54 ++++++++++++++ .../jetty/http2/frames/UnknownParseTest.java | 72 +++++++++++++++++++ 3 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/UnknownBodyParser.java create mode 100644 jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/UnknownParseTest.java diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java index 41872115124..60ffd501313 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java @@ -52,6 +52,7 @@ public class Parser private final HeaderParser headerParser; private final HeaderBlockParser headerBlockParser; private final BodyParser[] bodyParsers; + private final UnknownBodyParser unknownBodyParser; private boolean continuation; private State state = State.HEADER; @@ -60,6 +61,7 @@ public class Parser this.listener = listener; this.headerParser = new HeaderParser(); this.headerBlockParser = new HeaderBlockParser(byteBufferPool, new HpackDecoder(maxDynamicTableSize, maxHeaderSize)); + this.unknownBodyParser = new UnknownBodyParser(headerParser, listener); this.bodyParsers = new BodyParser[FrameType.values().length]; } @@ -172,9 +174,13 @@ public class Parser int type = getFrameType(); if (type < 0 || type >= bodyParsers.length) { - BufferUtil.clear(buffer); - notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unknown_frame_type_" + type); - return false; + // Unknown frame types must be ignored. + if (LOG.isDebugEnabled()) + LOG.debug("Ignoring unknown frame type {}", Integer.toHexString(type)); + if (!unknownBodyParser.parse(buffer)) + return false; + reset(); + return true; } BodyParser bodyParser = bodyParsers[type]; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/UnknownBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/UnknownBodyParser.java new file mode 100644 index 00000000000..97a418c2ec4 --- /dev/null +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/UnknownBodyParser.java @@ -0,0 +1,54 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.parser; + +import java.nio.ByteBuffer; + +public class UnknownBodyParser extends BodyParser +{ + private int cursor; + + public UnknownBodyParser(HeaderParser headerParser, Parser.Listener listener) + { + super(headerParser, listener); + } + + @Override + public boolean parse(ByteBuffer buffer) + { + int length = cursor == 0 ? getBodyLength() : cursor; + cursor = consume(buffer, length); + return cursor == 0; + } + + private int consume(ByteBuffer buffer, int length) + { + int remaining = buffer.remaining(); + if (remaining >= length) + { + buffer.position(buffer.position() + length); + return 0; + } + else + { + buffer.position(buffer.limit()); + return length - remaining; + } + } +} diff --git a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/UnknownParseTest.java b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/UnknownParseTest.java new file mode 100644 index 00000000000..bbaa6c43347 --- /dev/null +++ b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/UnknownParseTest.java @@ -0,0 +1,72 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.frames; + +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; +import java.util.function.UnaryOperator; + +import org.eclipse.jetty.http2.parser.Parser; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.MappedByteBufferPool; +import org.junit.Assert; +import org.junit.Test; + +public class UnknownParseTest +{ + private final ByteBufferPool byteBufferPool = new MappedByteBufferPool(); + + @Test + public void testParse() + { + testParse(Function.identity()); + } + + @Test + public void testParseOneByteAtATime() + { + testParse(buffer -> ByteBuffer.wrap(new byte[]{buffer.get()})); + } + + private void testParse(Function fn) + { + AtomicBoolean failure = new AtomicBoolean(); + Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter() + { + @Override + public void onConnectionFailure(int error, String reason) + { + failure.set(true); + } + }, 4096, 8192); + parser.init(UnaryOperator.identity()); + + // Iterate a few times to be sure the parser is properly reset. + for (int i = 0; i < 2; ++i) + { + byte[] bytes = new byte[]{0, 0, 4, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + ByteBuffer buffer = ByteBuffer.wrap(bytes); + while (buffer.hasRemaining()) + parser.parse(fn.apply(buffer)); + } + + Assert.assertFalse(failure.get()); + } +} From 5836c50a20f813a1346bdd5c83bebc7ba68e0f2c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Jul 2018 10:39:43 +0200 Subject: [PATCH 08/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed max frame length handling. Signed-off-by: Simone Bordet --- .../jetty/http2/client/HTTP2Client.java | 13 ++++ .../client/HTTP2ClientConnectionFactory.java | 5 ++ .../jetty/http2/parser/HeaderParser.java | 7 ++ .../eclipse/jetty/http2/parser/Parser.java | 27 +++++++- .../http2/frames/MaxFrameSizeParseTest.java | 66 +++++++++++++++++++ .../AbstractHTTP2ServerConnectionFactory.java | 13 ++++ 6 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java diff --git a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java index 83ff1c487e7..da00dc0094c 100644 --- a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java +++ b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java @@ -34,6 +34,7 @@ import org.eclipse.jetty.alpn.client.ALPNClientConnectionFactory; import org.eclipse.jetty.http2.BufferingFlowControlStrategy; import org.eclipse.jetty.http2.FlowControlStrategy; import org.eclipse.jetty.http2.api.Session; +import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.Connection; @@ -129,6 +130,7 @@ public class HTTP2Client extends ContainerLifeCycle private List protocols = Arrays.asList("h2", "h2-17", "h2-16", "h2-15", "h2-14"); private int initialSessionRecvWindow = 16 * 1024 * 1024; private int initialStreamRecvWindow = 8 * 1024 * 1024; + private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH; private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F); @Override @@ -334,6 +336,17 @@ public class HTTP2Client extends ContainerLifeCycle this.initialStreamRecvWindow = initialStreamRecvWindow; } + @ManagedAttribute("The max frame length in bytes") + public int getMaxFrameLength() + { + return maxFrameLength; + } + + public void setMaxFrameLength(int maxFrameLength) + { + this.maxFrameLength = maxFrameLength; + } + public void connect(InetSocketAddress address, Session.Listener listener, Promise promise) { connect(null, address, listener, promise); diff --git a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java index 9875d602526..9d5ec06954b 100644 --- a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java +++ b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java @@ -67,6 +67,7 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory FlowControlStrategy flowControl = client.getFlowControlStrategyFactory().newFlowControlStrategy(); HTTP2ClientSession session = new HTTP2ClientSession(scheduler, endPoint, generator, listener, flowControl); Parser parser = new Parser(byteBufferPool, session, 4096, 8192); + parser.setMaxFrameLength(client.getMaxFrameLength()); HTTP2ClientConnection connection = new HTTP2ClientConnection(client, byteBufferPool, executor, endPoint, parser, session, client.getInputBufferSize(), promise, listener); @@ -110,6 +111,10 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory settings = new HashMap<>(); settings.computeIfAbsent(SettingsFrame.INITIAL_WINDOW_SIZE, k -> client.getInitialStreamRecvWindow()); + Integer maxFrameLength = settings.get(SettingsFrame.MAX_FRAME_SIZE); + if (maxFrameLength != null) + getParser().setMaxFrameLength(maxFrameLength); + PrefaceFrame prefaceFrame = new PrefaceFrame(); SettingsFrame settingsFrame = new SettingsFrame(settings, false); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java index a9d3b9e2771..df717084674 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.http2.parser; import java.nio.ByteBuffer; import org.eclipse.jetty.http2.frames.Frame; +import org.eclipse.jetty.http2.frames.FrameType; /** *

The parser for the frame header of HTTP/2 frames.

@@ -144,6 +145,12 @@ public class HeaderParser return streamId; } + @Override + public String toString() + { + return String.format("[%s|%d|%d|%d]", FrameType.from(getFrameType()), getLength(), flags, getStreamId()); + } + private enum State { LENGTH, TYPE, FLAGS, STREAM_ID, STREAM_ID_BYTES diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java index 60ffd501313..029e1170574 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java @@ -24,6 +24,7 @@ import java.util.function.UnaryOperator; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.Flags; import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.GoAwayFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; @@ -53,6 +54,7 @@ public class Parser private final HeaderBlockParser headerBlockParser; private final BodyParser[] bodyParsers; private final UnknownBodyParser unknownBodyParser; + private int maxFrameLength; private boolean continuation; private State state = State.HEADER; @@ -62,6 +64,7 @@ public class Parser this.headerParser = new HeaderParser(); this.headerBlockParser = new HeaderBlockParser(byteBufferPool, new HpackDecoder(maxDynamicTableSize, maxHeaderSize)); this.unknownBodyParser = new UnknownBodyParser(headerParser, listener); + this.maxFrameLength = Frame.DEFAULT_MAX_LENGTH; this.bodyParsers = new BodyParser[FrameType.values().length]; } @@ -139,10 +142,13 @@ public class Parser if (!headerParser.parse(buffer)) return false; - FrameType frameType = FrameType.from(getFrameType()); if (LOG.isDebugEnabled()) - LOG.debug("Parsed {} frame header from {}", frameType, buffer); + LOG.debug("Parsed {} frame header from {}", headerParser, buffer); + if (headerParser.getLength() > getMaxFrameLength()) + return handleFrameTooLarge(buffer); + + FrameType frameType = FrameType.from(getFrameType()); if (continuation) { if (frameType != FrameType.CONTINUATION) @@ -169,6 +175,13 @@ public class Parser return true; } + private boolean handleFrameTooLarge(ByteBuffer buffer) + { + BufferUtil.clear(buffer); + notifyConnectionFailure(ErrorCode.FRAME_SIZE_ERROR.code, "invalid_frame_length"); + return false; + } + protected boolean parseBody(ByteBuffer buffer) { int type = getFrameType(); @@ -209,6 +222,16 @@ public class Parser return headerParser.hasFlag(bit); } + public int getMaxFrameLength() + { + return maxFrameLength; + } + + public void setMaxFrameLength(int maxFrameLength) + { + this.maxFrameLength = maxFrameLength; + } + protected void notifyConnectionFailure(int error, String reason) { try diff --git a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java new file mode 100644 index 00000000000..b902c1d925c --- /dev/null +++ b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java @@ -0,0 +1,66 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.frames; + +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.UnaryOperator; + +import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.parser.Parser; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.MappedByteBufferPool; +import org.junit.Assert; +import org.junit.Test; + +public class MaxFrameSizeParseTest +{ + private final ByteBufferPool byteBufferPool = new MappedByteBufferPool(); + + @Test + public void testMaxFrameSize() + { + int maxFrameLength = Frame.DEFAULT_MAX_LENGTH + 16; + + AtomicInteger failure = new AtomicInteger(); + Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter() + { + @Override + public void onConnectionFailure(int error, String reason) + { + failure.set(error); + } + }, 4096, 8192); + parser.setMaxFrameLength(maxFrameLength); + parser.init(UnaryOperator.identity()); + + // Iterate a few times to be sure the parser is properly reset. + for (int i = 0; i < 2; ++i) + { + byte[] bytes = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0}; + ByteBuffer buffer = ByteBuffer.wrap(bytes); + buffer.putInt(0, maxFrameLength + 1); + buffer.position(1); + while (buffer.hasRemaining()) + parser.parse(buffer); + } + + Assert.assertEquals(ErrorCode.FRAME_SIZE_ERROR.code, failure.get()); + } +} diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java index b2cc1ad504e..753da94ef57 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java @@ -50,6 +50,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne private int initialStreamRecvWindow = 512 * 1024; private int maxConcurrentStreams = 128; private int maxHeaderBlockFragment = 0; + private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH; private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F); private long streamIdleTimeout; @@ -145,6 +146,17 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne this.streamIdleTimeout = streamIdleTimeout; } + @ManagedAttribute("The max frame length in bytes") + public int getMaxFrameLength() + { + return maxFrameLength; + } + + public void setMaxFrameLength(int maxFrameLength) + { + this.maxFrameLength = maxFrameLength; + } + /** * @return -1 * @deprecated feature removed, no replacement @@ -205,6 +217,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize()); ServerParser parser = newServerParser(connector, session); + parser.setMaxFrameLength(getMaxFrameLength()); HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(), endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener); connection.addListener(connectionListener); From d06d5f5a71cb5e72004b35cc83564a7acbd67a9b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Jul 2018 16:35:37 +0200 Subject: [PATCH 09/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed PRIORITY self-dependency. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http2/frames/HeadersFrame.java | 3 ++- .../eclipse/jetty/http2/parser/HeadersBodyParser.java | 9 +++------ .../eclipse/jetty/http2/parser/PriorityBodyParser.java | 1 - 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/HeadersFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/HeadersFrame.java index 709e28f0f44..478dae889e9 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/HeadersFrame.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/HeadersFrame.java @@ -83,6 +83,7 @@ public class HeadersFrame extends Frame @Override public String toString() { - return String.format("%s#%d{end=%b}", super.toString(), streamId, endStream); + return String.format("%s#%d{end=%b}%s", super.toString(), streamId, endStream, + priority == null ? "" : String.format("+%s", priority)); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java index 784c2369d71..f31c937b33b 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java @@ -92,17 +92,11 @@ public class HeadersBodyParser extends BodyParser length = getBodyLength(); if (isPadding()) - { state = State.PADDING_LENGTH; - } else if (hasFlag(Flags.PRIORITY)) - { state = State.EXCLUSIVE; - } else - { state = State.HEADERS; - } break; } case PADDING_LENGTH: @@ -162,6 +156,9 @@ public class HeadersBodyParser extends BodyParser } case WEIGHT: { + // SPEC: stream cannot depend on itself. + if (getStreamId() == parentStreamId) + return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_priority_frame"); weight = (buffer.get() & 0xFF) + 1; --length; state = State.HEADERS; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PriorityBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PriorityBodyParser.java index daa76602e20..6c1f94d956c 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PriorityBodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PriorityBodyParser.java @@ -102,7 +102,6 @@ public class PriorityBodyParser extends BodyParser // SPEC: stream cannot depend on itself. if (getStreamId() == parentStreamId) return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_priority_frame"); - int weight = (buffer.get() & 0xFF) + 1; return onPriority(parentStreamId, weight, exclusive); } From 47506250c8c66ff567f0dee81a3f8d9aaa764b77 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 4 Jul 2018 16:07:31 +0200 Subject: [PATCH 10/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed sanity checks for SETTINGS values. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http2/HTTP2Session.java | 28 ++++++------------- .../http2/parser/SettingsBodyParser.java | 24 ++++++++++++---- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 600c96b1c66..ff085915340 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -306,54 +306,42 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio case SettingsFrame.HEADER_TABLE_SIZE: { if (LOG.isDebugEnabled()) - LOG.debug("Update HPACK header table size to {} for {}", value, this); + LOG.debug("Updating HPACK header table size to {} for {}", value, this); generator.setHeaderTableSize(value); break; } case SettingsFrame.ENABLE_PUSH: { - // SPEC: check the value is sane. - if (value != 0 && value != 1) - { - onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_enable_push"); - return; - } - pushEnabled = value == 1; if (LOG.isDebugEnabled()) - LOG.debug("{} push for {}", pushEnabled ? "Enable" : "Disable", this); + LOG.debug("{} push for {}", pushEnabled ? "Enabling" : "Disabling", this); + pushEnabled = value == 1; break; } case SettingsFrame.MAX_CONCURRENT_STREAMS: { - maxLocalStreams = value; if (LOG.isDebugEnabled()) - LOG.debug("Update max local concurrent streams to {} for {}", maxLocalStreams, this); + LOG.debug("Updating max local concurrent streams to {} for {}", maxLocalStreams, this); + maxLocalStreams = value; break; } case SettingsFrame.INITIAL_WINDOW_SIZE: { if (LOG.isDebugEnabled()) - LOG.debug("Update initial window size to {} for {}", value, this); + LOG.debug("Updating initial window size to {} for {}", value, this); flowControl.updateInitialStreamWindow(this, value, false); break; } case SettingsFrame.MAX_FRAME_SIZE: { if (LOG.isDebugEnabled()) - LOG.debug("Update max frame size to {} for {}", value, this); - // SPEC: check the max frame size is sane. - if (value < Frame.DEFAULT_MAX_LENGTH || value > Frame.MAX_MAX_LENGTH) - { - onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_frame_size"); - return; - } + LOG.debug("Updating max frame size to {} for {}", value, this); generator.setMaxFrameSize(value); break; } case SettingsFrame.MAX_HEADER_LIST_SIZE: { if (LOG.isDebugEnabled()) - LOG.debug("Update max header list size to {} for {}", value, this); + LOG.debug("Updating max header list size to {} for {}", value, this); generator.setMaxHeaderListSize(value); break; } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java index 7452f2bd17a..6a89ec51e32 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java @@ -25,6 +25,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.Flags; +import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.SettingsFrame; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -57,7 +58,7 @@ public class SettingsBodyParser extends BodyParser @Override protected void emptyBody(ByteBuffer buffer) { - onSettings(new HashMap<>()); + onSettings(buffer, new HashMap<>()); } @Override @@ -120,7 +121,7 @@ public class SettingsBodyParser extends BodyParser state = State.SETTING_ID; length -= 4; if (length == 0) - return onSettings(settings); + return onSettings(buffer, settings); } else { @@ -145,7 +146,7 @@ public class SettingsBodyParser extends BodyParser settings.put(settingId, settingValue); state = State.SETTING_ID; if (length == 0) - return onSettings(settings); + return onSettings(buffer, settings); } break; } @@ -158,8 +159,21 @@ public class SettingsBodyParser extends BodyParser return false; } - protected boolean onSettings(Map settings) + protected boolean onSettings(ByteBuffer buffer, Map settings) { + Integer enablePush = settings.get(SettingsFrame.ENABLE_PUSH); + if (enablePush != null && enablePush != 0 && enablePush != 1) + return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_enable_push"); + + Integer initialWindowSize = settings.get(SettingsFrame.INITIAL_WINDOW_SIZE); + // Values greater than Integer.MAX_VALUE will overflow to negative. + if (initialWindowSize != null && initialWindowSize < 0) + return connectionFailure(buffer, ErrorCode.FLOW_CONTROL_ERROR.code, "invalid_settings_initial_window_size"); + + Integer maxFrameLength = settings.get(SettingsFrame.MAX_FRAME_SIZE); + if (maxFrameLength != null && (maxFrameLength < Frame.DEFAULT_MAX_LENGTH || maxFrameLength > Frame.MAX_MAX_LENGTH)) + return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_frame_size"); + SettingsFrame frame = new SettingsFrame(settings, hasFlag(Flags.ACK)); reset(); notifySettings(frame); @@ -185,7 +199,7 @@ public class SettingsBodyParser extends BodyParser } @Override - protected boolean onSettings(Map settings) + protected boolean onSettings(ByteBuffer buffer, Map settings) { frameRef.set(new SettingsFrame(settings, false)); return true; From 7aa7dceb894f4d0456c88b7766657fb175f37b83 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 4 Jul 2018 16:20:21 +0200 Subject: [PATCH 11/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed sanity checks for the WINDOW_UPDATE delta. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http2/HTTP2Session.java | 50 +++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index ff085915340..bbf951f6eb8 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -436,18 +436,58 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio LOG.debug("Received {}", frame); int streamId = frame.getStreamId(); + int windowDelta = frame.getWindowDelta(); if (streamId > 0) { - IStream stream = getStream(streamId); - if (stream != null) + if (windowDelta == 0) { - stream.process(frame, Callback.NOOP); - onWindowUpdate(stream, frame); + reset(new ResetFrame(streamId, ErrorCode.PROTOCOL_ERROR.code), Callback.NOOP); + } + else + { + IStream stream = getStream(streamId); + if (stream != null) + { + int streamSendWindow = stream.updateSendWindow(0); + if (overflows(streamSendWindow, windowDelta)) + { + reset(new ResetFrame(streamId, ErrorCode.FLOW_CONTROL_ERROR.code), Callback.NOOP); + } + else + { + stream.process(frame, Callback.NOOP); + onWindowUpdate(stream, frame); + } + } } } else { - onWindowUpdate(null, frame); + if (windowDelta == 0) + { + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "invalid_window_update_frame"); + } + else + { + int sessionSendWindow = updateSendWindow(0); + if (overflows(sessionSendWindow, windowDelta)) + onConnectionFailure(ErrorCode.FLOW_CONTROL_ERROR.code, "invalid_flow_control_window"); + else + onWindowUpdate(null, frame); + } + } + } + + private boolean overflows(int a, int b) + { + try + { + Math.addExact(a, b); + return false; + } + catch (ArithmeticException x) + { + return true; } } From d35fa69e1fdc0142d07bf2ee64d6caa289e13f78 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 4 Jul 2018 17:47:35 +0200 Subject: [PATCH 12/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed handling of CONTINUATION frames. Signed-off-by: Simone Bordet --- .../eclipse/jetty/http2/parser/Parser.java | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java index 029e1170574..912669d101a 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java @@ -36,7 +36,6 @@ import org.eclipse.jetty.http2.frames.SettingsFrame; import org.eclipse.jetty.http2.frames.WindowUpdateFrame; import org.eclipse.jetty.http2.hpack.HpackDecoder; import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -132,8 +131,7 @@ public class Parser { if (LOG.isDebugEnabled()) LOG.debug(x); - BufferUtil.clear(buffer); - notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "parser_error"); + connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR, "parser_error"); } } @@ -146,42 +144,28 @@ public class Parser LOG.debug("Parsed {} frame header from {}", headerParser, buffer); if (headerParser.getLength() > getMaxFrameLength()) - return handleFrameTooLarge(buffer); + return connectionFailure(buffer, ErrorCode.FRAME_SIZE_ERROR, "invalid_frame_length"); FrameType frameType = FrameType.from(getFrameType()); if (continuation) { + // SPEC: CONTINUATION frames must be consecutive. if (frameType != FrameType.CONTINUATION) - { - // SPEC: CONTINUATION frames must be consecutive. - BufferUtil.clear(buffer); - notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "continuation_frame_expected"); - return false; - } + return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR, "expected_continuation_frame"); if (headerParser.hasFlag(Flags.END_HEADERS)) - { continuation = false; - } } else { - if (frameType == FrameType.HEADERS && - !headerParser.hasFlag(Flags.END_HEADERS)) - { - continuation = true; - } + if (frameType == FrameType.HEADERS) + continuation = !headerParser.hasFlag(Flags.END_HEADERS); + else if (frameType == FrameType.CONTINUATION) + return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR, "unexpected_continuation_frame"); } state = State.BODY; return true; } - private boolean handleFrameTooLarge(ByteBuffer buffer) - { - BufferUtil.clear(buffer); - notifyConnectionFailure(ErrorCode.FRAME_SIZE_ERROR.code, "invalid_frame_length"); - return false; - } - protected boolean parseBody(ByteBuffer buffer) { int type = getFrameType(); @@ -212,6 +196,11 @@ public class Parser return true; } + private boolean connectionFailure(ByteBuffer buffer, ErrorCode error, String reason) + { + return unknownBodyParser.connectionFailure(buffer, error.code, reason); + } + protected int getFrameType() { return headerParser.getFrameType(); From 0ec8f312f6c89aa836d645624b468243832dcea3 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Jul 2018 15:51:14 +0200 Subject: [PATCH 13/26] Issue #2679 - HTTP/2 Spec Compliance. Fixed stream ID validation and stream state handling. Signed-off-by: Simone Bordet --- .../http2/client/HTTP2ClientSession.java | 19 +++- .../org/eclipse/jetty/http2/HTTP2Session.java | 91 +++++++++++++------ .../http2/generator/GoAwayGenerator.java | 2 +- .../http2/server/HTTP2ServerSession.java | 40 ++++++-- 4 files changed, 113 insertions(+), 39 deletions(-) diff --git a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java index 617a13fbba4..0fbaf32fd81 100644 --- a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java +++ b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java @@ -78,6 +78,7 @@ public class HTTP2ClientSession extends HTTP2Session if (LOG.isDebugEnabled()) LOG.debug("Received {}", frame); + // HEADERS can be received for normal and pushed responses. int streamId = frame.getStreamId(); IStream stream = getStream(streamId); if (stream != null) @@ -96,7 +97,23 @@ public class HTTP2ClientSession extends HTTP2Session else { if (LOG.isDebugEnabled()) - LOG.debug("Ignoring {}, stream #{} not found", frame, streamId); + LOG.debug("Stream #{} not found", streamId); + if ((streamId & 1) == 1) + { + // Normal stream. + // Headers or trailers arriving after + // the stream has been reset are ignored. + if (!isLocalStreamClosed(streamId)) + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_headers_frame"); + } + else + { + // Pushed stream. + // Headers or trailers arriving after + // the stream has been reset are ignored. + if (!isRemoteStreamClosed(streamId)) + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_headers_frame"); + } } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index bbf951f6eb8..53fdda5a4b7 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -72,8 +72,8 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio private static final Logger LOG = Log.getLogger(HTTP2Session.class); private final ConcurrentMap streams = new ConcurrentHashMap<>(); - private final AtomicInteger streamIds = new AtomicInteger(); - private final AtomicInteger lastStreamId = new AtomicInteger(); + private final AtomicInteger localStreamIds = new AtomicInteger(); + private final AtomicInteger lastRemoteStreamId = new AtomicInteger(); private final AtomicInteger localStreamCount = new AtomicInteger(); private final AtomicBiInteger remoteStreamCount = new AtomicBiInteger(); private final AtomicInteger sendWindow = new AtomicInteger(); @@ -105,7 +105,8 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio this.flusher = new HTTP2Flusher(this); this.maxLocalStreams = -1; this.maxRemoteStreams = -1; - this.streamIds.set(initialStreamId); + this.localStreamIds.set(initialStreamId); + this.lastRemoteStreamId.set((initialStreamId & 0x01) == 0x01 ? 0 : -1); this.streamIdleTimeout = endPoint.getIdleTimeout(); this.sendWindow.set(FlowControlStrategy.DEFAULT_WINDOW_SIZE); this.recvWindow.set(FlowControlStrategy.DEFAULT_WINDOW_SIZE); @@ -229,35 +230,44 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio LOG.debug("Received {}", frame); int streamId = frame.getStreamId(); - final IStream stream = getStream(streamId); + IStream stream = getStream(streamId); // SPEC: the session window must be updated even if the stream is null. // The flow control length includes the padding bytes. - final int flowControlLength = frame.remaining() + frame.padding(); + int flowControlLength = frame.remaining() + frame.padding(); flowControl.onDataReceived(this, stream, flowControlLength); if (stream != null) { if (getRecvWindow() < 0) - { - close(ErrorCode.FLOW_CONTROL_ERROR.code, "session_window_exceeded", callback); - } + onConnectionFailure(ErrorCode.FLOW_CONTROL_ERROR.code, "session_window_exceeded", callback); else - { stream.process(frame, new DataCallback(callback, stream, flowControlLength)); - } } else { if (LOG.isDebugEnabled()) - LOG.debug("Ignoring {}, stream #{} not found", frame, streamId); + LOG.debug("Stream #{} not found", streamId); // We must enlarge the session flow control window, // otherwise other requests will be stalled. flowControl.onDataConsumed(this, null, flowControlLength); - callback.succeeded(); + if (isRemoteStreamClosed(streamId)) + reset(new ResetFrame(streamId, ErrorCode.STREAM_CLOSED_ERROR.code), callback); + else + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_data_frame", callback); } } + protected boolean isLocalStreamClosed(int streamId) + { + return streamId <= localStreamIds.get(); + } + + protected boolean isRemoteStreamClosed(int streamId) + { + return streamId <= getLastRemoteStreamId(); + } + @Override public abstract void onHeaders(HeadersFrame frame); @@ -274,11 +284,19 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio if (LOG.isDebugEnabled()) LOG.debug("Received {}", frame); - IStream stream = getStream(frame.getStreamId()); + int streamId = frame.getStreamId(); + IStream stream = getStream(streamId); if (stream != null) + { stream.process(frame, new ResetCallback()); + } else - notifyReset(this, frame); + { + if (isRemoteStreamClosed(streamId)) + notifyReset(this, frame); + else + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_rst_stream_frame"); + } } @Override @@ -449,7 +467,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio if (stream != null) { int streamSendWindow = stream.updateSendWindow(0); - if (overflows(streamSendWindow, windowDelta)) + if (sumOverflows(streamSendWindow, windowDelta)) { reset(new ResetFrame(streamId, ErrorCode.FLOW_CONTROL_ERROR.code), Callback.NOOP); } @@ -459,6 +477,11 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio onWindowUpdate(stream, frame); } } + else + { + if (!isRemoteStreamClosed(streamId)) + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_window_update_frame"); + } } } else @@ -470,7 +493,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio else { int sessionSendWindow = updateSendWindow(0); - if (overflows(sessionSendWindow, windowDelta)) + if (sumOverflows(sessionSendWindow, windowDelta)) onConnectionFailure(ErrorCode.FLOW_CONTROL_ERROR.code, "invalid_flow_control_window"); else onWindowUpdate(null, frame); @@ -478,7 +501,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } } - private boolean overflows(int a, int b) + private boolean sumOverflows(int a, int b) { try { @@ -494,7 +517,12 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio @Override public void onConnectionFailure(int error, String reason) { - notifyFailure(this, new IOException(String.format("%d/%s", error, reason)), new CloseCallback(error, reason)); + onConnectionFailure(error, reason, Callback.NOOP); + } + + private void onConnectionFailure(int error, String reason, Callback callback) + { + notifyFailure(this, new IOException(String.format("%d/%s", error, reason)), new CloseCallback(error, reason, callback)); } @Override @@ -510,7 +538,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio int streamId = frame.getStreamId(); if (streamId <= 0) { - streamId = streamIds.getAndAdd(2); + streamId = localStreamIds.getAndAdd(2); PriorityFrame priority = frame.getPriority(); priority = priority == null ? null : new PriorityFrame(streamId, priority.getParentStreamId(), priority.getWeight(), priority.isExclusive()); @@ -539,7 +567,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio IStream stream = streams.get(streamId); if (stream == null) { - streamId = streamIds.getAndAdd(2); + streamId = localStreamIds.getAndAdd(2); frame = new PriorityFrame(streamId, frame.getParentStreamId(), frame.getWeight(), frame.isExclusive()); } @@ -557,7 +585,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio boolean queued; synchronized (this) { - int streamId = streamIds.getAndAdd(2); + int streamId = localStreamIds.getAndAdd(2); frame = new PushPromiseFrame(frame.getStreamId(), streamId, frame.getMetaData()); IStream pushStream = createLocalStream(streamId); @@ -657,7 +685,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio reason = reason.substring(0, Math.min(reason.length(), 32)); payload = reason.getBytes(StandardCharsets.UTF_8); } - return new GoAwayFrame(closeState, lastStreamId.get(), error, payload); + return new GoAwayFrame(closeState, getLastRemoteStreamId(), error, payload); } @Override @@ -764,7 +792,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio // SPEC: duplicate stream is treated as connection error. if (streams.putIfAbsent(streamId, stream) == null) { - updateLastStreamId(streamId); + updateLastRemoteStreamId(streamId); stream.setIdleTimeout(getStreamIdleTimeout()); flowControl.onStreamCreated(stream); if (LOG.isDebugEnabled()) @@ -773,7 +801,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } else { - close(ErrorCode.PROTOCOL_ERROR.code, "duplicate_stream", Callback.NOOP); + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "duplicate_stream"); return null; } } @@ -1042,9 +1070,14 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio return !endPoint.isOpen(); } - private void updateLastStreamId(int streamId) + protected int getLastRemoteStreamId() { - Atomics.updateMax(lastStreamId, streamId); + return lastRemoteStreamId.get(); + } + + private void updateLastRemoteStreamId(int streamId) + { + Atomics.updateMax(lastRemoteStreamId, streamId); } protected Stream.Listener notifyNewStream(Stream stream, HeadersFrame frame) @@ -1506,11 +1539,13 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio { private final int error; private final String reason; + private final Callback callback; - private CloseCallback(int error, String reason) + private CloseCallback(int error, String reason, Callback callback) { this.error = error; this.reason = reason; + this.callback = callback; } @Override @@ -1533,7 +1568,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio private void complete() { - close(error, reason, Callback.NOOP); + close(error, reason, callback); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java index 73f211d81c2..1b6e1735358 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java @@ -45,7 +45,7 @@ public class GoAwayGenerator extends FrameGenerator public int generateGoAway(ByteBufferPool.Lease lease, int lastStreamId, int error, byte[] payload) { if (lastStreamId < 0) - throw new IllegalArgumentException("Invalid last stream id: " + lastStreamId); + lastStreamId = 0; // The last streamId + the error code. int fixedLength = 4 + 4; diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java index dfe5c828039..37882e3650f 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java @@ -83,16 +83,39 @@ public class HTTP2ServerSession extends HTTP2Session implements ServerParser.Lis if (LOG.isDebugEnabled()) LOG.debug("Received {}", frame); + int streamId = frame.getStreamId(); + if ((streamId & 1) != 1) + { + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "invalid_stream_id"); + return; + } + + IStream stream = getStream(streamId); + MetaData metaData = frame.getMetaData(); if (metaData.isRequest()) { - IStream stream = createRemoteStream(frame.getStreamId()); - if (stream != null) + if (stream == null) { - onStreamOpened(stream); - stream.process(frame, Callback.NOOP); - Stream.Listener listener = notifyNewStream(stream, frame); - stream.setListener(listener); + if (isRemoteStreamClosed(streamId)) + { + onConnectionFailure(ErrorCode.STREAM_CLOSED_ERROR.code, "unexpected_headers_frame"); + } + else + { + stream = createRemoteStream(streamId); + if (stream != null) + { + onStreamOpened(stream); + stream.process(frame, Callback.NOOP); + Stream.Listener listener = notifyNewStream(stream, frame); + stream.setListener(listener); + } + } + } + else + { + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "duplicate_stream"); } } else if (metaData.isResponse()) @@ -102,8 +125,6 @@ public class HTTP2ServerSession extends HTTP2Session implements ServerParser.Lis else { // Trailers. - int streamId = frame.getStreamId(); - IStream stream = getStream(streamId); if (stream != null) { stream.process(frame, Callback.NOOP); @@ -112,7 +133,8 @@ public class HTTP2ServerSession extends HTTP2Session implements ServerParser.Lis else { if (LOG.isDebugEnabled()) - LOG.debug("Ignoring {}, stream #{} not found", frame, streamId); + LOG.debug("Stream #{} not found", streamId); + onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_headers_frame"); } } } From a9819ebb01f89a8d62b425152a72e8210bf6e8ae Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 12 Jul 2018 11:50:53 +0200 Subject: [PATCH 14/26] review fixes Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/HpackDecoder.java | 4 +-- .../jetty/http2/hpack/HpackException.java | 17 ++++++----- .../jetty/http2/hpack/MetaDataBuilder.java | 28 +++++++++---------- .../jetty/http2/hpack/HpackDecoderTest.java | 2 +- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 0f6ff926361..5526eb44772 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -73,7 +73,7 @@ public class HpackDecoder // If the buffer is big, don't even think about decoding it if (buffer.remaining()>_builder.getMaxSize()) - throw new HpackException.Session("431 Request Header Fields too large",6); + throw new HpackException.SessionException("431 Request Header Fields too large"); while(buffer.hasRemaining()) { @@ -92,7 +92,7 @@ public class HpackDecoder int index = NBitInteger.decode(buffer,7); Entry entry=_context.get(index); if (entry==null) - throw new HpackException.Session("Unknown index %d",index); + throw new HpackException.SessionException("Unknown index %d",index); if (entry.isStatic()) { diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java index 6e3655c2b70..287e02f2e13 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java @@ -27,26 +27,25 @@ public abstract class HpackException extends RuntimeException super(String.format(messageFormat, args)); } - public static class Stream extends HpackException + public static class StreamException extends HpackException { - Stream(String messageFormat, Object... args) + StreamException(String messageFormat, Object... args) { super(messageFormat,args); } } - - - public static class Session extends HpackException + + public static class SessionException extends HpackException { - Session(String messageFormat, Object... args) + SessionException(String messageFormat, Object... args) { super(messageFormat,args); } } - - public static class Compression extends Session + + public static class CompressionException extends SessionException { - public Compression(String messageFormat, Object... args) + public CompressionException(String messageFormat, Object... args) { super(messageFormat,args); } diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index dbfb698beba..de15f936716 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -27,7 +27,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; -import org.eclipse.jetty.http2.hpack.HpackException.Session; +import org.eclipse.jetty.http2.hpack.HpackException.SessionException; public class MetaDataBuilder { @@ -40,7 +40,7 @@ public class MetaDataBuilder private String _path; private long _contentLength=Long.MIN_VALUE; private HttpFields _fields = new HttpFields(10); - private HpackException.Stream _streamException; + private HpackException.StreamException _streamException; private boolean _request; private boolean _response; @@ -68,7 +68,7 @@ public class MetaDataBuilder return _size; } - public void emit(HttpField field) throws HpackException.Session + public void emit(HttpField field) throws HpackException.SessionException { HttpHeader header = field.getHeader(); String name = field.getName(); @@ -76,7 +76,7 @@ public class MetaDataBuilder int field_size = name.length() + (value == null ? 0 : value.length()); _size+=field_size+32; if (_size>_maxSize) - throw new HpackException.Session("Header Size %d > %d",_size,_maxSize); + throw new HpackException.SessionException("Header Size %d > %d",_size,_maxSize); if (field instanceof StaticTableHttpField) { @@ -162,10 +162,10 @@ public class MetaDataBuilder break; case TE: - if ("trailors".equalsIgnoreCase(value)) + if ("trailers".equalsIgnoreCase(value)) _fields.add(field); else - streamException("unsupported TE value %s", value); + streamException("Unsupported TE value %s", value); break; case CONNECTION: @@ -175,7 +175,7 @@ public class MetaDataBuilder default: if (name.charAt(0)==':') - streamException("Unknown psuodo header %s", name); + streamException("Unknown pseudo header %s", name); else _fields.add(field); break; @@ -184,7 +184,7 @@ public class MetaDataBuilder else { if (name.charAt(0)==':') - streamException("Unknown psuedo header %s",name); + streamException("Unknown pseudo header %s",name); else _fields.add(field); } @@ -192,7 +192,7 @@ public class MetaDataBuilder void streamException(String messageFormat, Object... args) { - HpackException.Stream stream = new HpackException.Stream(messageFormat, args); + HpackException.StreamException stream = new HpackException.StreamException(messageFormat, args); if (_streamException==null) _streamException = stream; else @@ -226,13 +226,13 @@ public class MetaDataBuilder } - public MetaData build() throws HpackException.Stream + public MetaData build() throws HpackException.StreamException { if (_streamException!=null) throw _streamException; if (_request && _response) - throw new HpackException.Stream("Request and Response headers"); + throw new HpackException.StreamException("Request and Response headers"); try { @@ -266,14 +266,14 @@ public class MetaDataBuilder * Check that the max size will not be exceeded. * @param length the length * @param huffman the huffman name - * @throws Session + * @throws SessionException */ - public void checkSize(int length, boolean huffman) throws Session + public void checkSize(int length, boolean huffman) throws SessionException { // Apply a huffman fudge factor if (huffman) length=(length*4)/3; if ((_size+length)>_maxSize) - throw new HpackException.Session("Header too large %d > %d", _size+length, _maxSize); + throw new HpackException.SessionException("Header too large %d > %d", _size+length, _maxSize); } } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 1fb9d4ced9c..6d98a68ed00 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -224,7 +224,7 @@ public class HpackDecoderTest decoder.decode(buffer); Assert.fail(); } - catch (HpackException.Session e) + catch (HpackException.SessionException e) { assertThat(e.getMessage(),Matchers.startsWith("Unknown index")); } From 0ad4b4483b986a8d1027eba599a633ea8667d889 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 15 Jul 2018 19:22:31 +0200 Subject: [PATCH 15/26] WIP Signed-off-by: Greg Wilkins --- .../java/org/eclipse/jetty/http/HttpURI.java | 3 +- .../java/org/eclipse/jetty/http/MetaData.java | 15 +- .../jetty/http2/hpack/MetaDataBuilder.java | 20 +- .../jetty/http2/hpack/HpackDecoderTest.java | 321 +++++++++++++++++- .../test/resources/jetty-logging.properties | 2 +- 5 files changed, 341 insertions(+), 20 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index b09accbb18e..af3e12e678e 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -164,7 +164,8 @@ public class HttpURI _host=host; _port=port; - parse(State.PATH,pathQuery,0,pathQuery.length()); + if (pathQuery!=null) + parse(State.PATH,pathQuery,0,pathQuery.length()); } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java index 75551f76ede..decab4d73bd 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java @@ -163,17 +163,26 @@ public class MetaData implements Iterable public Request(String method, HttpScheme scheme, HostPortHttpField hostPort, String uri, HttpVersion version, HttpFields fields) { - this(method, new HttpURI(scheme == null ? null : scheme.asString(), hostPort.getHost(), hostPort.getPort(), uri), version, fields); + this(method, new HttpURI(scheme == null ? null : scheme.asString(), + hostPort==null?null:hostPort.getHost(), + hostPort==null?-1:hostPort.getPort(), + uri), version, fields); } public Request(String method, HttpScheme scheme, HostPortHttpField hostPort, String uri, HttpVersion version, HttpFields fields, long contentLength) { - this(method, new HttpURI(scheme == null ? null : scheme.asString(), hostPort.getHost(), hostPort.getPort(), uri), version, fields, contentLength); + this(method, new HttpURI(scheme==null?null:scheme.asString(), + hostPort==null?null:hostPort.getHost(), + hostPort==null?-1:hostPort.getPort(), + uri), version, fields, contentLength); } public Request(String method, String scheme, HostPortHttpField hostPort, String uri, HttpVersion version, HttpFields fields, long contentLength) { - this(method, new HttpURI(scheme, hostPort.getHost(), hostPort.getPort(), uri), version, fields, contentLength); + this(method, new HttpURI(scheme, + hostPort==null?null:hostPort.getHost(), + hostPort==null?-1:hostPort.getPort(), + uri), version, fields, contentLength); } public Request(Request request) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index de15f936716..a53b70de97c 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -203,12 +203,12 @@ public class MetaDataBuilder { if (_fields.size()>0) { - streamException("Psuedo header %s after fields", header.asString()); + streamException("Pseudo header %s after fields", header.asString()); return false; } if (value==-1) return true; - streamException("Duplicate psuedo header %s", header.asString()); + streamException("Duplicate pseudo header %s", header.asString()); return false; } @@ -216,20 +216,22 @@ public class MetaDataBuilder { if (_fields.size()>0) { - streamException("Psuedo header %s after fields", header.asString()); + streamException("Pseudo header %s after fields", header.asString()); return false; } if (value==null) return true; - streamException("Duplicate psuedo header %s", header.asString()); + streamException("Duplicate pseudo header %s", header.asString()); return false; } - public MetaData build() throws HpackException.StreamException { if (_streamException!=null) + { + _streamException.addSuppressed(new Throwable()); throw _streamException; + } if (_request && _response) throw new HpackException.StreamException("Request and Response headers"); @@ -239,14 +241,10 @@ public class MetaDataBuilder HttpFields fields = _fields; _fields = new HttpFields(Math.max(10,fields.size()+5)); - if (_method!=null) + if (_method!=null || _path!=null || _authority!=null || _scheme!=null) return new MetaData.Request(_method,_scheme,_authority,_path,HttpVersion.HTTP_2,fields,_contentLength); - if (_status!=0) + if (_status>0) return new MetaData.Response(HttpVersion.HTTP_2,_status,fields,_contentLength); - if (_path!=null) - fields.put(HttpHeader.C_PATH,_path); - if (_authority!=null) - fields.put(HttpHeader.HOST,_authority.getValue()); return new MetaData(HttpVersion.HTTP_2,fields,_contentLength); } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 6d98a68ed00..dd861890d7e 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.http2.hpack; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -29,12 +28,11 @@ import static org.junit.Assert.assertTrue; import java.nio.ByteBuffer; import java.util.Iterator; -import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.hpack.HpackException.StreamException; import org.eclipse.jetty.util.TypeUtil; import org.hamcrest.Matchers; import org.junit.Assert; @@ -209,7 +207,7 @@ public class HpackDecoderTest MetaData metaData = decoder.decode(buffer); assertThat(decoder.getHpackContext().getDynamicTableSize(),is(0)); - assertThat(metaData.getFields().get(HttpHeader.C_PATH),Matchers.startsWith("This is a very large field")); + assertThat(((MetaData.Request)metaData).getURI().toString(),Matchers.startsWith("This is a very large field")); } @Test @@ -230,4 +228,319 @@ public class HpackDecoderTest } } + + /* 8.1.2.1. Pseudo-Header Fields */ + @Test() + public void test8_1_2_1_PsuedoHeaderFields() + { + // 1:Sends a HEADERS frame that contains a unknown pseudo-header field + MetaDataBuilder mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(":unknown","value")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Unknown pseudo header")); + } + + // 2: Sends a HEADERS frame that contains the pseudo-header field defined for response + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/path")); + mdb.emit(new HttpField(HttpHeader.C_STATUS,"100")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Request and Response headers")); + } + + // 3: Sends a HEADERS frame that contains a pseudo-header field as trailers + + // 4: Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/path")); + mdb.emit(new HttpField("Accept","No Compromise")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Pseudo header :authority after fields")); + } + } + + /* + * + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + ✔ 3: Sends a HEADERS frame that contains a pseudo-header field as trailers + + × 4: Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + + */ + + + + /* + 8.1.2.2. Connection-Specific Header Fields + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:33, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 1: Sends a HEADERS frame that contains the connection-specific header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:44, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 2: Sends a HEADERS frame that contains the TE header field with any value other than "trailers" + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + + 8.1.2.3. Request Pseudo-Header Fields + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:23, flags:0x04, stream_id:1) + [recv] DATA Frame (length:50, flags:0x01, stream_id:1) + [recv] RST_STREAM Frame (length:4, flags:0x00, stream_id:1) + [recv] Timeout + × 1: Sends a HEADERS frame with empty ":path" pseudo-header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:50, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:13, flags:0x05, stream_id:1) + [recv] Timeout + × 2: Sends a HEADERS frame that omits ":method" pseudo-header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: Timeout + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:14, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:100, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 3: Sends a HEADERS frame that omits ":scheme" pseudo-header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:14, flags:0x05, stream_id:1) + [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) + ✔ 4: Sends a HEADERS frame that omits ":path" pseudo-header field + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 5: Sends a HEADERS frame with duplicated ":method" pseudo-header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 6: Sends a HEADERS frame with duplicated ":scheme" pseudo-header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:18, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:79, flags:0x05, stream_id:1) + [recv] Timeout + × 7: Sends a HEADERS frame with duplicated ":method" pseudo-header field + -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: HEADERS Frame (length:79, flags:0x05, stream_id:1) + + 8.1.2.6. Malformed Requests and Responses + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:18, flags:0x04, stream_id:1) + [send] DATA Frame (length:4, flags:0x01, stream_id:1) + [recv] HEADERS Frame (length:100, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length + -> The endpoint MUST treat this as a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:18, flags:0x04, stream_id:1) + [send] DATA Frame (length:4, flags:0x00, stream_id:1) + [send] DATA Frame (length:4, flags:0x01, stream_id:1) + [recv] HEADERS Frame (length:100, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length + -> The endpoint MUST treat this as a stream error of type PROTOCOL_ERROR. + Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) + RST_STREAM Frame (Error Code: PROTOCOL_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + + 8.2. Server Push + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] PUSH_PROMISE Frame (length:19, flags:0x04, stream_id:1) + [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) + ✔ 1: Sends a PUSH_PROMISE frame + + HPACK: Header Compression for HTTP/2 + 2. Compression Process Overview + 2.3. Indexing Tables + 2.3.3. Index Address Space + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) + [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) + [recv] Connection closed + ✔ 1: Sends a header field representation with invalid index + + 4. Dynamic Table Management + 4.2. Maximum Table Size + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 1: Sends a dynamic table size update at the end of header block + -> The endpoint MUST treat this as a decoding error. + Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + + 5. Primitive Type Representations + 5.2. String Literal Representation + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:27, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits + -> The endpoint MUST treat this as a decoding error. + Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:26, flags:0x05, stream_id:1) + [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) + [recv] DATA Frame (length:687, flags:0x01, stream_id:1) + [recv] Timeout + × 2: Sends a Huffman-encoded string literal representation padded by zero + -> The endpoint MUST treat this as a decoding error. + Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) + Connection closed + Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) + [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) + [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) + [send] HEADERS Frame (length:28, flags:0x05, stream_id:1) + [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) + [recv] Connection closed + ✔ 3: Sends a Huffman-encoded string literal representation containing the EOS symbol + + */ + } diff --git a/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties b/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties index e40e8e43ce1..d33a7c32778 100644 --- a/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties +++ b/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties @@ -1,3 +1,3 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog org.eclipse.jetty.http2.LEVEL=INFO -org.eclipse.jetty.http2.hpack.LEVEL=INFO +org.eclipse.jetty.http2.hpack.LEVEL=DEBUG From 42844f2c5fd70d69645763b69c15db55785421f1 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 18 Jul 2018 11:08:05 +0200 Subject: [PATCH 16/26] WIP Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/MetaDataBuilder.java | 16 +++++++++------- .../org/eclipse/jetty/http2/hpack/HpackTest.java | 15 +++++++-------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index a53b70de97c..dc65c4fcc9e 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -236,27 +236,29 @@ public class MetaDataBuilder if (_request && _response) throw new HpackException.StreamException("Request and Response headers"); + + HttpFields fields = _fields; try { - HttpFields fields = _fields; - _fields = new HttpFields(Math.max(10,fields.size()+5)); - - if (_method!=null || _path!=null || _authority!=null || _scheme!=null) + if (_request) return new MetaData.Request(_method,_scheme,_authority,_path,HttpVersion.HTTP_2,fields,_contentLength); - if (_status>0) + if (_response) return new MetaData.Response(HttpVersion.HTTP_2,_status,fields,_contentLength); return new MetaData(HttpVersion.HTTP_2,fields,_contentLength); } finally { - _status=0; + _fields = new HttpFields(Math.max(10,fields.size()+5)); + _request=false; + _response=false; + _status=-1; _method=null; _scheme=null; _authority=null; _path=null; _size=0; - _contentLength=Long.MIN_VALUE; + _contentLength=Long.MIN_VALUE; } } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java index 9c7d2a24eb5..2f3f461de31 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java @@ -18,15 +18,18 @@ package org.eclipse.jetty.http2.hpack; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + import java.nio.ByteBuffer; import java.util.concurrent.TimeUnit; -import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.DateGenerator; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.MetaData.Response; @@ -35,10 +38,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.junit.Assert; import org.junit.Test; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; - public class HpackTest { final static HttpField ServerJetty = new PreEncodedHttpField(HttpHeader.SERVER,"jetty"); @@ -131,9 +130,9 @@ public class HpackTest decoder.decode(buffer); Assert.fail(); } - catch(BadMessageException e) + catch(HpackException.SessionException e) { - assertEquals(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431,e.getCode()); + assertThat(e.getMessage(),containsString("Header too large")); } } From 98ea112fd38155797acda7698be6bf5f26ac7dde Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 18 Jul 2018 12:26:55 +0200 Subject: [PATCH 17/26] wip Signed-off-by: Greg Wilkins --- .../jetty/http2/parser/HeaderBlockParser.java | 50 ++++++++++++++++--- .../jetty/http2/hpack/HpackDecoder.java | 2 +- .../jetty/http2/hpack/HpackException.java | 16 +++++- .../jetty/http2/hpack/MetaDataBuilder.java | 3 -- .../jetty/http2/hpack/HpackDecoderTest.java | 2 +- 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java index f9a048d8471..922388be2a1 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java @@ -20,8 +20,11 @@ package org.eclipse.jetty.http2.parser; import java.nio.ByteBuffer; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.hpack.HpackDecoder; +import org.eclipse.jetty.http2.hpack.HpackException.SessionException; +import org.eclipse.jetty.http2.hpack.HpackException.StreamException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.BufferUtil; @@ -72,17 +75,48 @@ public class HeaderBlockParser toDecode = buffer; } - MetaData result = hpackDecoder.decode(toDecode); - - buffer.limit(limit); - - if (blockBuffer != null) + try { - byteBufferPool.release(blockBuffer); - blockBuffer = null; + MetaData metadata = hpackDecoder.decode(toDecode); + + if (metadata instanceof MetaData.Request) + { + // TODO this must be an initial HEADERs frame received by the server OR + // TODO a push promise received by the client. + // TODO this must not be a trailers frame + } + else if (metadata instanceof MetaData.Response) + { + // TODO this must be an initial HEADERs frame received by the client + // TODO this must not be a trailers frame + } + else + { + // TODO this must be a trailers frame + } + + return metadata; } + catch(StreamException ex) + { + // TODO reset the stream + throw new BadMessageException("TODO"); + } + catch(SessionException ex) + { + // TODO reset the session + throw new BadMessageException("TODO"); + } + finally + { + buffer.limit(limit); - return result; + if (blockBuffer != null) + { + byteBufferPool.release(blockBuffer); + blockBuffer = null; + } + } } } } diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 5526eb44772..74b8b87deb9 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -66,7 +66,7 @@ public class HpackDecoder _localMaxDynamicTableSize=localMaxdynamciTableSize; } - public MetaData decode(ByteBuffer buffer) throws HpackException + public MetaData decode(ByteBuffer buffer) throws HpackException.SessionException, HpackException.StreamException { if (LOG.isDebugEnabled()) LOG.debug(String.format("CtxTbl[%x] decoding %d octets",_context.hashCode(),buffer.remaining())); diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java index 287e02f2e13..78516c73bd2 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackException.java @@ -18,15 +18,22 @@ package org.eclipse.jetty.http2.hpack; -import java.io.IOException; -public abstract class HpackException extends RuntimeException +@SuppressWarnings("serial") +public abstract class HpackException extends Exception { HpackException(String messageFormat, Object... args) { super(String.format(messageFormat, args)); } + /** + * A Stream HPACK exception. + *

Stream exceptions are not fatal to the connection and the + * hpack state is complete and able to continue handling other + * decoding/encoding for the session. + *

+ */ public static class StreamException extends HpackException { StreamException(String messageFormat, Object... args) @@ -35,6 +42,11 @@ public abstract class HpackException extends RuntimeException } } + /** + * A Session HPACK Exception. + *

Session exceptions are fatal for the stream and the HPACK + * state is unable to decode/encode further.

+ */ public static class SessionException extends HpackException { SessionException(String messageFormat, Object... args) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index dc65c4fcc9e..3a01e8a593a 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -228,10 +228,7 @@ public class MetaDataBuilder public MetaData build() throws HpackException.StreamException { if (_streamException!=null) - { - _streamException.addSuppressed(new Throwable()); throw _streamException; - } if (_request && _response) throw new HpackException.StreamException("Request and Response headers"); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index dd861890d7e..1e74d100fd7 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -231,7 +231,7 @@ public class HpackDecoderTest /* 8.1.2.1. Pseudo-Header Fields */ @Test() - public void test8_1_2_1_PsuedoHeaderFields() + public void test8_1_2_1_PsuedoHeaderFields() throws Exception { // 1:Sends a HEADERS frame that contains a unknown pseudo-header field MetaDataBuilder mdb = new MetaDataBuilder(4096); From fa46013cf71c1bf1ccfd2ae17eda38396539f723 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 19 Jul 2018 12:24:24 +0200 Subject: [PATCH 18/26] 8.1.2.2 Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/MetaDataBuilder.java | 6 +- .../jetty/http2/hpack/HpackDecoderTest.java | 84 +++++++++---------- 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index 3a01e8a593a..e526f78bebd 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -169,8 +169,10 @@ public class MetaDataBuilder break; case CONNECTION: - // TODO should other connection specific fields be listed here? - streamException("Connection specific field %s", header); + if ("TE".equalsIgnoreCase(value)) + _fields.add(field); + else + streamException("Connection specific field %s", header); break; default: diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 1e74d100fd7..f53490c0e4b 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -282,51 +282,47 @@ public class HpackDecoderTest } } - /* - * - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - ✔ 3: Sends a HEADERS frame that contains a pseudo-header field as trailers - - × 4: Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - - */ - - - - /* - 8.1.2.2. Connection-Specific Header Fields - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:33, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 1: Sends a HEADERS frame that contains the connection-specific header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:44, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 2: Sends a HEADERS frame that contains the TE header field with any value other than "trailers" - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + /* 8.1.2.2. Connection-Specific Header Fields */ + @Test() + public void test8_1_2_2_ConnectionSpecificHeaderFields() throws Exception + { + MetaDataBuilder mdb; + // 1: Sends a HEADERS frame that contains the connection-specific header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.CONNECTION,"value")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Connection specific field Connection")); + } + + // 2: Sends a HEADERS frame that contains the TE header field with any value other than "trailers" + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.TE,"not_trailers")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Unsupported TE value not_trailers")); + } + + + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.CONNECTION,"TE")); + mdb.emit(new HttpField(HttpHeader.TE,"trailers")); + Assert.assertNotNull(mdb.build()); + } + + + /* 8.1.2.3. Request Pseudo-Header Fields [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) From 971ca22367f9b7a182d61a1460f178de42c783e0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 19 Jul 2018 12:50:54 +0200 Subject: [PATCH 19/26] 8.1.2.3 Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/MetaDataBuilder.java | 20 +- .../jetty/http2/hpack/HpackDecoderTest.java | 212 ++++++++++-------- 2 files changed, 133 insertions(+), 99 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index e526f78bebd..74cc77da705 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -152,7 +152,12 @@ public class MetaDataBuilder case C_PATH: if(checkPseudoHeader(header, _path)) - _path = value; + { + if (value!=null && value.length()>0) + _path = value; + else + streamException("No Path"); + } _request = true; break; @@ -230,8 +235,11 @@ public class MetaDataBuilder public MetaData build() throws HpackException.StreamException { if (_streamException!=null) + { + _streamException.addSuppressed(new Throwable()); throw _streamException; - + } + if (_request && _response) throw new HpackException.StreamException("Request and Response headers"); @@ -240,7 +248,15 @@ public class MetaDataBuilder try { if (_request) + { + if (_method==null) + throw new HpackException.StreamException("No Method"); + if (_scheme==null) + throw new HpackException.StreamException("No Scheme"); + if (_path==null) + throw new HpackException.StreamException("No Path"); return new MetaData.Request(_method,_scheme,_authority,_path,HttpVersion.HTTP_2,fields,_contentLength); + } if (_response) return new MetaData.Response(HttpVersion.HTTP_2,_status,fields,_contentLength); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index f53490c0e4b..e6241d288d4 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -282,7 +282,6 @@ public class HpackDecoderTest } } - /* 8.1.2.2. Connection-Specific Header Fields */ @Test() public void test8_1_2_2_ConnectionSpecificHeaderFields() throws Exception { @@ -321,105 +320,124 @@ public class HpackDecoderTest Assert.assertNotNull(mdb.build()); } + + @Test() + public void test8_1_2_3_RequestPseudoHeaderFields() throws Exception + { + MetaDataBuilder mdb; + + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/")); + Assert.assertThat(mdb.build(),Matchers.instanceOf(MetaData.Request.class)); + + + // 1: Sends a HEADERS frame with empty ":path" pseudo-header field + mdb = new MetaDataBuilder(4096); + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("No Path")); + } + + // 2: Sends a HEADERS frame that omits ":method" pseudo-header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("No Method")); + } + + + // 3: Sends a HEADERS frame that omits ":scheme" pseudo-header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("No Scheme")); + } + + // 4: Sends a HEADERS frame that omits ":path" pseudo-header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("No Path")); + } + + // 5: Sends a HEADERS frame with duplicated ":method" pseudo-header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Duplicate")); + } + + // 6: Sends a HEADERS frame with duplicated ":scheme" pseudo-header field + mdb = new MetaDataBuilder(4096); + mdb.emit(new HttpField(HttpHeader.C_METHOD,"GET")); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_SCHEME,"http")); + mdb.emit(new HttpField(HttpHeader.C_AUTHORITY,"localhost:8080")); + mdb.emit(new HttpField(HttpHeader.C_PATH,"/")); + try + { + mdb.build(); + Assert.fail(); + } + catch(StreamException ex) + { + Assert.assertThat(ex.getMessage(),Matchers.containsString("Duplicate")); + } + + + + } + /* 8.1.2.3. Request Pseudo-Header Fields - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:23, flags:0x04, stream_id:1) - [recv] DATA Frame (length:50, flags:0x01, stream_id:1) - [recv] RST_STREAM Frame (length:4, flags:0x00, stream_id:1) - [recv] Timeout - × 1: Sends a HEADERS frame with empty ":path" pseudo-header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:50, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:13, flags:0x05, stream_id:1) - [recv] Timeout - × 2: Sends a HEADERS frame that omits ":method" pseudo-header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: Timeout - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:14, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:100, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 3: Sends a HEADERS frame that omits ":scheme" pseudo-header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:14, flags:0x05, stream_id:1) - [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) - ✔ 4: Sends a HEADERS frame that omits ":path" pseudo-header field - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 5: Sends a HEADERS frame with duplicated ":method" pseudo-header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 6: Sends a HEADERS frame with duplicated ":scheme" pseudo-header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:18, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:79, flags:0x05, stream_id:1) - [recv] Timeout - × 7: Sends a HEADERS frame with duplicated ":method" pseudo-header field - -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: HEADERS Frame (length:79, flags:0x05, stream_id:1) 8.1.2.6. Malformed Requests and Responses [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) From 0da922505612cde07c3c9583c7b5ae8d2cbff299 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 19 Jul 2018 13:49:40 +0200 Subject: [PATCH 20/26] dynamic table resize Signed-off-by: Greg Wilkins --- .../jetty/http2/hpack/HpackDecoder.java | 7 + .../jetty/http2/hpack/HpackDecoderTest.java | 152 +++++------------- .../jetty/http2/hpack/HpackEncoderTest.java | 24 +++ .../test/resources/jetty-logging.properties | 2 +- 4 files changed, 71 insertions(+), 114 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 74b8b87deb9..083f10b8bc5 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -74,6 +74,8 @@ public class HpackDecoder // If the buffer is big, don't even think about decoding it if (buffer.remaining()>_builder.getMaxSize()) throw new HpackException.SessionException("431 Request Header Fields too large"); + + boolean emitted = false; while(buffer.hasRemaining()) { @@ -99,6 +101,7 @@ public class HpackDecoder if (LOG.isDebugEnabled()) LOG.debug("decode IdxStatic {}",entry); // emit field + emitted = true; _builder.emit(entry.getHttpField()); // TODO copy and add to reference set if there is room @@ -109,6 +112,7 @@ public class HpackDecoder if (LOG.isDebugEnabled()) LOG.debug("decode Idx {}",entry); // emit + emitted = true; _builder.emit(entry.getHttpField()); } } @@ -133,6 +137,8 @@ public class HpackDecoder LOG.debug("decode resize="+size); if (size>_localMaxDynamicTableSize) throw new IllegalArgumentException(); + if (emitted) + throw new HpackException.CompressionException("Dynamic table resize after fields"); _context.resize(size); continue; @@ -240,6 +246,7 @@ public class HpackDecoder } // emit the field + emitted = true; _builder.emit(field); // if indexed add to dynamic table diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index e6241d288d4..05b21814c39 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -32,6 +32,7 @@ import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.hpack.HpackException.CompressionException; import org.eclipse.jetty.http2.hpack.HpackException.StreamException; import org.eclipse.jetty.util.TypeUtil; import org.hamcrest.Matchers; @@ -188,26 +189,55 @@ public class HpackDecoderTest @Test public void testResize() throws Exception { - String encoded = "3f6166871e33A13a47497f205f8841E92b043d492d49"; + String encoded = "203f136687A0E41d139d090760881c6490B2Cd39Ba7f"; ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); HpackDecoder decoder = new HpackDecoder(4096, 8192); MetaData metaData = decoder.decode(buffer); - assertThat(metaData.getFields().get(HttpHeader.HOST),is("aHostName")); - assertThat(metaData.getFields().get(HttpHeader.CONTENT_TYPE),is("some/content")); - assertThat(decoder.getHpackContext().getDynamicTableSize(),is(0)); + assertThat(metaData.getFields().get(HttpHeader.HOST),is( "localhost0")); + assertThat(metaData.getFields().get(HttpHeader.COOKIE),is("abcdefghij")); + assertThat(decoder.getHpackContext().getMaxDynamicTableSize(),is(50)); + assertThat(decoder.getHpackContext().size(),is(1)); + + + } + + @Test + public void testBadResize() throws Exception + { + /* + 4. Dynamic Table Management + 4.2. Maximum Table Size + × 1: Sends a dynamic table size update at the end of header block + -> The endpoint MUST treat this as a decoding error. + Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) + Connection closed + */ + + String encoded = "203f136687A0E41d139d090760881c6490B2Cd39Ba7f20"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + HpackDecoder decoder = new HpackDecoder(4096, 8192); + try + { + decoder.decode(buffer); + Assert.fail(); + } + catch(CompressionException e) + { + Assert.assertThat(e.getMessage(),Matchers.containsString("Dynamic table resize after fields")); + } } @Test public void testTooBigToIndex() throws Exception { - String encoded = "44FfEc02Df3990A190A0D4Ee5b3d2940Ec98Aa4a62D127D29e273a0aA20dEcAa190a503b262d8a2671D4A2672a927aA874988a2471D05510750c951139EdA2452a3a548cAa1aA90bE4B228342864A9E0D450A5474a92992a1aA513395448E3A0Aa17B96cFe3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f14E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F353F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F54f"; + String encoded = "3f610f17FfEc02Df3990A190A0D4Ee5b3d2940Ec98Aa4a62D127D29e273a0aA20dEcAa190a503b262d8a2671D4A2672a927aA874988a2471D05510750c951139EdA2452a3a548cAa1aA90bE4B228342864A9E0D450A5474a92992a1aA513395448E3A0Aa17B96cFe3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f14E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F3E7Cf9f3e7cF9F353F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F7F54f"; ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); HpackDecoder decoder = new HpackDecoder(128,8192); MetaData metaData = decoder.decode(buffer); assertThat(decoder.getHpackContext().getDynamicTableSize(),is(0)); - assertThat(((MetaData.Request)metaData).getURI().toString(),Matchers.startsWith("This is a very large field")); + assertThat(metaData.getFields().get("host"),Matchers.startsWith("This is a very large field")); } @Test @@ -430,131 +460,27 @@ public class HpackDecoderTest { Assert.assertThat(ex.getMessage(),Matchers.containsString("Duplicate")); } - - - } /* - 8.1.2.3. Request Pseudo-Header Fields - - 8.1.2.6. Malformed Requests and Responses - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:18, flags:0x04, stream_id:1) - [send] DATA Frame (length:4, flags:0x01, stream_id:1) - [recv] HEADERS Frame (length:100, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length - -> The endpoint MUST treat this as a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:18, flags:0x04, stream_id:1) - [send] DATA Frame (length:4, flags:0x00, stream_id:1) - [send] DATA Frame (length:4, flags:0x01, stream_id:1) - [recv] HEADERS Frame (length:100, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length - -> The endpoint MUST treat this as a stream error of type PROTOCOL_ERROR. - Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR) - RST_STREAM Frame (Error Code: PROTOCOL_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - - 8.2. Server Push - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] PUSH_PROMISE Frame (length:19, flags:0x04, stream_id:1) - [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) - ✔ 1: Sends a PUSH_PROMISE frame - - HPACK: Header Compression for HTTP/2 - 2. Compression Process Overview - 2.3. Indexing Tables - 2.3.3. Index Address Space - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) - [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) - [recv] Connection closed - ✔ 1: Sends a header field representation with invalid index - - 4. Dynamic Table Management - 4.2. Maximum Table Size - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:16, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout - × 1: Sends a dynamic table size update at the end of header block - -> The endpoint MUST treat this as a decoding error. - Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) + 8.1.2.6. Malformed Requests and Responses + × 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length + × 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length 5. Primitive Type Representations 5.2. String Literal Representation - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:27, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout × 1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits -> The endpoint MUST treat this as a decoding error. Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) Connection closed Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:26, flags:0x05, stream_id:1) - [recv] HEADERS Frame (length:101, flags:0x04, stream_id:1) - [recv] DATA Frame (length:687, flags:0x01, stream_id:1) - [recv] Timeout × 2: Sends a Huffman-encoded string literal representation padded by zero -> The endpoint MUST treat this as a decoding error. Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) Connection closed Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:24, flags:0x00, stream_id:0) - [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0) - [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0) - [send] HEADERS Frame (length:28, flags:0x05, stream_id:1) - [recv] GOAWAY Frame (length:20, flags:0x00, stream_id:0) - [recv] Connection closed ✔ 3: Sends a Huffman-encoded string literal representation containing the EOS symbol - */ } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java index 00349c46017..66381b69c35 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java @@ -249,4 +249,28 @@ public class HpackEncoderTest context.get(HpackContext.STATIC_SIZE+1).getSize()+context.get(HpackContext.STATIC_SIZE+2).getSize())); } + + @Test + public void testResize() + { + HttpFields fields = new HttpFields(); + fields.add("host", "localhost0"); + fields.add("cookie","abcdefghij"); + + HpackEncoder encoder = new HpackEncoder(4096); + + ByteBuffer buffer = BufferUtil.allocate(4096); + int pos = BufferUtil.flipToFill(buffer); + encoder.encodeMaxDynamicTableSize(buffer,0); + encoder.setRemoteMaxDynamicTableSize(50); + encoder.encode(buffer,new MetaData(HttpVersion.HTTP_2,fields)); + BufferUtil.flipToFlush(buffer,pos); + + HpackContext context = encoder.getHpackContext(); + + Assert.assertThat(context.getMaxDynamicTableSize(),Matchers.is(50)); + Assert.assertThat(context.size(),Matchers.is(1)); + + + } } diff --git a/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties b/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties index d33a7c32778..e40e8e43ce1 100644 --- a/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties +++ b/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties @@ -1,3 +1,3 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog org.eclipse.jetty.http2.LEVEL=INFO -org.eclipse.jetty.http2.hpack.LEVEL=DEBUG +org.eclipse.jetty.http2.hpack.LEVEL=INFO From a72fe7e3c77101e9336b3a0a7ffc6c5b17c79819 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 20 Jul 2018 22:40:05 +1000 Subject: [PATCH 21/26] Issue #2679 - HTTP/2 Spec Compliance Added tests for the problems with the Huffman encoding Signed-off-by: Lachlan Roberts --- .../jetty/http2/hpack/HpackDecoderTest.java | 99 +++++++++++++++---- 1 file changed, 79 insertions(+), 20 deletions(-) diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 05b21814c39..2d5cf49e9cf 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -462,25 +462,84 @@ public class HpackDecoderTest } } - - /* - 8.1.2.6. Malformed Requests and Responses - × 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length - × 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length - 5. Primitive Type Representations - 5.2. String Literal Representation - × 1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits - -> The endpoint MUST treat this as a decoding error. - Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - × 2: Sends a Huffman-encoded string literal representation padded by zero - -> The endpoint MUST treat this as a decoding error. - Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR) - Connection closed - Actual: DATA Frame (length:687, flags:0x01, stream_id:1) - ✔ 3: Sends a Huffman-encoded string literal representation containing the EOS symbol - */ - + @Test() + public void testHuffmanEncodedStandard() throws Exception + { + HpackDecoder decoder = new HpackDecoder(4096, 8192); + + String encoded = "82868441" + "83" + "49509F"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + + MetaData.Request request = (MetaData.Request)decoder.decode(buffer); + + assertEquals("GET", request.getMethod()); + assertEquals(HttpScheme.HTTP.asString(), request.getURI().getScheme()); + assertEquals("/", request.getURI().getPath()); + assertEquals("test", request.getURI().getHost()); + assertFalse(request.iterator().hasNext()); + } + + + /* 5.2.1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits */ + @Test() + public void testHuffmanEncodedExtraPadding() throws Exception + { + HpackDecoder decoder = new HpackDecoder(4096, 8192); + + String encoded = "82868441" + "83" + "49509FFF"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + + try + { + decoder.decode(buffer); + Assert.fail(); + } + catch (StreamException ex) + { + Assert.assertThat(ex.getMessage(), Matchers.containsString("Padding length exceeded")); + } + } + + + /* 5.2.2: Sends a Huffman-encoded string literal representation padded by zero */ + @Test() + public void testHuffmanEncodedZeroPadding() throws Exception + { + HpackDecoder decoder = new HpackDecoder(4096, 8192); + + String encoded = "82868441" + "83" + "495090"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + + try + { + decoder.decode(buffer); + Assert.fail(); + } + catch (StreamException ex) + { + Assert.assertThat(ex.getMessage(), Matchers.containsString("Zero padded")); + } + } + + + /* 5.2.3: Sends a Huffman-encoded string literal representation containing the EOS symbol */ + @Test() + public void testHuffmanEncodedWithEOS() throws Exception + { + HpackDecoder decoder = new HpackDecoder(4096, 8192); + + String encoded = "82868441" + "87" + "497FFFFFFF427F"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + + try + { + decoder.decode(buffer); + Assert.fail(); + } + catch (StreamException ex) + { + Assert.assertThat(ex.getMessage(), Matchers.containsString("EOS in content")); + } + } } From da213d58767e19a49069412d69e7c12af3a9cae2 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Sat, 21 Jul 2018 11:32:34 +1000 Subject: [PATCH 22/26] Issue #2679 - HTTP/2 Spec Compliance Now throwing exceptions for incorrect padding and EOS in content in Huffman.decode() Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/http2/hpack/Huffman.java | 23 +++++++++++++++---- .../jetty/http2/hpack/HpackContextTest.java | 2 +- .../jetty/http2/hpack/HpackDecoderTest.java | 4 ++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java index 67c58ee0a98..8a1dbb3c5ed 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java @@ -287,6 +287,7 @@ public class Huffman }; static final int[][] LCCODES = new int[CODES.length][]; + static final char EOS = 256; // Huffman decode tree stored in a flattened char array for good // locality of reference. @@ -344,12 +345,12 @@ public class Huffman } } - public static String decode(ByteBuffer buffer) + public static String decode(ByteBuffer buffer) throws HpackException.StreamException { return decode(buffer,buffer.remaining()); } - public static String decode(ByteBuffer buffer,int length) + public static String decode(ByteBuffer buffer,int length) throws HpackException.StreamException { StringBuilder out = new StringBuilder(length*2); int node = 0; @@ -373,6 +374,9 @@ public class Huffman node = tree[node*256+c]; if (rowbits[node]!=0) { + if(rowsym[node] == EOS) + throw new HpackException.StreamException("EOS in content"); + // terminal node out.append(rowsym[node]); bits -= rowbits[node]; @@ -390,9 +394,20 @@ public class Huffman { int c = (current << (8 - bits)) & 0xFF; node = tree[node*256+c]; - if (rowbits[node]==0 || rowbits[node] > bits) + + if (rowbits[node]==0 || rowbits[node] > bits) + { + int requiredPadding = 0; + for(int i=0; i>(8-bits)) != requiredPadding) + throw new HpackException.StreamException("Incorrect padding"); + break; - + } + + // TODO why is this even here if (rowbits[node]==0) throw new IllegalStateException(); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java index 8af7b0e11e5..d91caff99cf 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java @@ -411,7 +411,7 @@ public class HpackContextTest } @Test - public void testStaticHuffmanValues() + public void testStaticHuffmanValues() throws Exception { HpackContext ctx = new HpackContext(4096); for (int i=2;i<=14;i++) diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 2d5cf49e9cf..03982a01241 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -487,7 +487,7 @@ public class HpackDecoderTest { HpackDecoder decoder = new HpackDecoder(4096, 8192); - String encoded = "82868441" + "83" + "49509FFF"; + String encoded = "82868441" + "84" + "49509FFF"; ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); try @@ -518,7 +518,7 @@ public class HpackDecoderTest } catch (StreamException ex) { - Assert.assertThat(ex.getMessage(), Matchers.containsString("Zero padded")); + Assert.assertThat(ex.getMessage(), Matchers.containsString("Incorrect padding")); } } From 4ace2e4d8dd6b21527aa0561de3e5d38b136aca9 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 21 Jul 2018 11:42:31 +0200 Subject: [PATCH 23/26] Issue #2679 - h2spec compliance. Integrated HPACK modifications to comply with the specification. Signed-off-by: Simone Bordet --- .../http2/client/HTTP2ClientSession.java | 11 ++- .../http2/client/FlowControlStrategyTest.java | 29 +++++- .../eclipse/jetty/http2/client/HTTP2Test.java | 83 +---------------- .../jetty/http2/client/TrailersTest.java | 67 +++++++++++++- .../org/eclipse/jetty/http2/ErrorCode.java | 14 +++ .../eclipse/jetty/http2/HTTP2Connection.java | 89 +++++------------- .../org/eclipse/jetty/http2/HTTP2Session.java | 73 ++++++++++++--- .../org/eclipse/jetty/http2/HTTP2Stream.java | 72 ++++++++++++--- .../org/eclipse/jetty/http2/api/Stream.java | 5 + .../jetty/http2/frames/FailureFrame.java | 42 +++++++++ .../eclipse/jetty/http2/frames/FrameType.java | 3 +- .../jetty/http2/frames/GoAwayFrame.java | 3 +- .../jetty/http2/frames/ResetFrame.java | 6 +- .../jetty/http2/parser/BodyParser.java | 17 ++++ .../http2/parser/ContinuationBodyParser.java | 9 +- .../jetty/http2/parser/HeaderBlockParser.java | 71 +++++++++------ .../jetty/http2/parser/HeadersBodyParser.java | 5 +- .../eclipse/jetty/http2/parser/Parser.java | 91 ++++++++++++++++++- .../http2/parser/PushPromiseBodyParser.java | 5 +- .../jetty/http2/parser/ServerParser.java | 20 ++++ .../jetty/http2/hpack/HpackDecoder.java | 2 - .../jetty/http2/hpack/HpackEncoder.java | 46 ++++++++-- .../jetty/http2/hpack/MetaDataBuilder.java | 10 +- .../jetty/http2/hpack/HpackDecoderTest.java | 16 ++-- .../test/resources/jetty-logging.properties | 4 +- .../client/http/HttpReceiverOverHTTP2.java | 14 ++- .../HttpClientTransportOverHTTP2Test.java | 34 +++++++ .../server/HTTP2ServerConnectionFactory.java | 16 ++-- .../http2/server/HTTP2ServerSession.java | 2 +- 29 files changed, 589 insertions(+), 270 deletions(-) create mode 100644 jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java diff --git a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java index 0fbaf32fd81..3be563d093c 100644 --- a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java +++ b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientSession.java @@ -98,7 +98,7 @@ public class HTTP2ClientSession extends HTTP2Session { if (LOG.isDebugEnabled()) LOG.debug("Stream #{} not found", streamId); - if ((streamId & 1) == 1) + if (isClientStream(streamId)) { // Normal stream. // Headers or trailers arriving after @@ -134,9 +134,12 @@ public class HTTP2ClientSession extends HTTP2Session else { IStream pushStream = createRemoteStream(pushStreamId); - pushStream.process(frame, Callback.NOOP); - Stream.Listener listener = notifyPush(stream, pushStream, frame); - pushStream.setListener(listener); + if (pushStream != null) + { + pushStream.process(frame, Callback.NOOP); + Stream.Listener listener = notifyPush(stream, pushStream, frame); + pushStream.setListener(listener); + } } } diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java index 28862a21795..d601f2d3b17 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java @@ -734,7 +734,21 @@ public abstract class FlowControlStrategyTest public void testClientExceedingSessionWindow() throws Exception { // On server, we don't consume the data. - start(new ServerSessionListener.Adapter()); + start(new ServerSessionListener.Adapter() + { + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + return new Stream.Listener.Adapter() + { + @Override + public void onData(Stream stream, DataFrame frame, Callback callback) + { + // Do not succeed the callback. + } + }; + } + }); final CountDownLatch closeLatch = new CountDownLatch(1); Session session = newClient(new Session.Listener.Adapter() @@ -805,6 +819,19 @@ public abstract class FlowControlStrategyTest ((ISession)session).updateRecvWindow(FlowControlStrategy.DEFAULT_WINDOW_SIZE); return super.onPreface(session); } + + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + return new Stream.Listener.Adapter() + { + @Override + public void onData(Stream stream, DataFrame frame, Callback callback) + { + // Do not succeed the callback. + } + }; + } }); final CountDownLatch closeLatch = new CountDownLatch(1); diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java index 9961e799e80..1a3a315d095 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java @@ -46,12 +46,7 @@ import org.eclipse.jetty.http2.api.server.ServerSessionListener; import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.GoAwayFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; -import org.eclipse.jetty.http2.frames.PingFrame; -import org.eclipse.jetty.http2.frames.PriorityFrame; -import org.eclipse.jetty.http2.frames.PushPromiseFrame; -import org.eclipse.jetty.http2.frames.ResetFrame; import org.eclipse.jetty.http2.frames.SettingsFrame; -import org.eclipse.jetty.http2.frames.WindowUpdateFrame; import org.eclipse.jetty.http2.parser.ServerParser; import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory; import org.eclipse.jetty.server.Connector; @@ -752,7 +747,7 @@ public class HTTP2Test extends AbstractTest @Override protected ServerParser newServerParser(Connector connector, ServerParser.Listener listener) { - return super.newServerParser(connector, new ServerParserListenerWrapper(listener) + return super.newServerParser(connector, new ServerParser.Listener.Wrapper(listener) { @Override public void onGoAway(GoAwayFrame frame) @@ -806,80 +801,4 @@ public class HTTP2Test extends AbstractTest throw new RuntimeException(); } } - - private static class ServerParserListenerWrapper implements ServerParser.Listener - { - private final ServerParser.Listener listener; - - private ServerParserListenerWrapper(ServerParser.Listener listener) - { - this.listener = listener; - } - - @Override - public void onPreface() - { - listener.onPreface(); - } - - @Override - public void onData(DataFrame frame) - { - listener.onData(frame); - } - - @Override - public void onHeaders(HeadersFrame frame) - { - listener.onHeaders(frame); - } - - @Override - public void onPriority(PriorityFrame frame) - { - listener.onPriority(frame); - } - - @Override - public void onReset(ResetFrame frame) - { - listener.onReset(frame); - } - - @Override - public void onSettings(SettingsFrame frame) - { - listener.onSettings(frame); - } - - @Override - public void onPushPromise(PushPromiseFrame frame) - { - listener.onPushPromise(frame); - } - - @Override - public void onPing(PingFrame frame) - { - listener.onPing(frame); - } - - @Override - public void onGoAway(GoAwayFrame frame) - { - listener.onGoAway(frame); - } - - @Override - public void onWindowUpdate(WindowUpdateFrame frame) - { - listener.onWindowUpdate(frame); - } - - @Override - public void onConnectionFailure(int error, String reason) - { - listener.onConnectionFailure(error, reason); - } - } } diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java index 6a6ddc4d640..e51778a653c 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java @@ -26,13 +26,13 @@ import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import javax.servlet.ServletException; import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; @@ -42,11 +42,13 @@ import org.eclipse.jetty.http2.api.server.ServerSessionListener; import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.HeadersFrame; +import org.eclipse.jetty.http2.frames.ResetFrame; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.StringUtil; import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; @@ -106,7 +108,7 @@ public class TrailersTest extends AbstractTest start(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { Request jettyRequest = (Request)request; // No trailers yet. @@ -238,7 +240,7 @@ public class TrailersTest extends AbstractTest start(new EmptyHttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { Request jettyRequest = (Request)request; Response jettyResponse = jettyRequest.getResponse(); @@ -287,4 +289,63 @@ public class TrailersTest extends AbstractTest Assert.assertTrue(trailers.isEndStream()); Assert.assertThat(trailers.getMetaData().getFields().get(trailerName), Matchers.equalTo(trailerValue)); } + + @Test + public void testRequestTrailerInvalidHpack() throws Exception + { + CountDownLatch serverLatch = new CountDownLatch(1); + start(new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException + { + try + { + // Read the content to read the trailers + ServletInputStream input = request.getInputStream(); + while (true) + { + int read = input.read(); + if (read < 0) + break; + } + } + catch (IOException x) + { + serverLatch.countDown(); + throw x; + } + } + }); + + CountDownLatch clientLatch = new CountDownLatch(1); + Session session = newClient(new Session.Listener.Adapter()); + MetaData.Request request = newRequest("POST", new HttpFields()); + HeadersFrame requestFrame = new HeadersFrame(request, null, false); + FuturePromise promise = new FuturePromise<>(); + session.newStream(requestFrame, promise, new Stream.Listener.Adapter() + { + @Override + public void onReset(Stream stream, ResetFrame frame) + { + clientLatch.countDown(); + } + }); + Stream stream = promise.get(5, TimeUnit.SECONDS); + ByteBuffer data = ByteBuffer.wrap(StringUtil.getUtf8Bytes("hello")); + Callback.Completable completable = new Callback.Completable(); + stream.data(new DataFrame(stream.getId(), data, false), completable); + completable.thenRun(() -> + { + // Invalid trailer: cannot contain pseudo headers. + HttpFields trailerFields = new HttpFields(); + trailerFields.put(HttpHeader.C_METHOD, "GET"); + MetaData trailer = new MetaData(HttpVersion.HTTP_2, trailerFields); + HeadersFrame trailerFrame = new HeadersFrame(stream.getId(), trailer, null, true); + stream.headers(trailerFrame, Callback.NOOP); + }); + + Assert.assertTrue(serverLatch.await(5, TimeUnit.SECONDS)); + Assert.assertTrue(clientLatch.await(5, TimeUnit.SECONDS)); + } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/ErrorCode.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/ErrorCode.java index 8d38a0e852b..aae6c082ccd 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/ErrorCode.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/ErrorCode.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.http2; import java.util.HashMap; +import java.util.Locale; import java.util.Map; /** @@ -96,6 +97,19 @@ public enum ErrorCode return Codes.codes.get(error); } + public static String toString(int error, String dft) + { + ErrorCode errorCode = from(error); + String result; + if (errorCode != null) + result = errorCode.name().toLowerCase(Locale.ENGLISH); + else if (dft == null) + result = String.valueOf(error); + else + result = dft; + return result; + } + private static class Codes { private static final Map codes = new HashMap<>(); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java index 534359990f8..c13cbb62d80 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java @@ -27,14 +27,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jetty.http2.frames.DataFrame; -import org.eclipse.jetty.http2.frames.GoAwayFrame; -import org.eclipse.jetty.http2.frames.HeadersFrame; -import org.eclipse.jetty.http2.frames.PingFrame; -import org.eclipse.jetty.http2.frames.PriorityFrame; -import org.eclipse.jetty.http2.frames.PushPromiseFrame; -import org.eclipse.jetty.http2.frames.ResetFrame; -import org.eclipse.jetty.http2.frames.SettingsFrame; -import org.eclipse.jetty.http2.frames.WindowUpdateFrame; import org.eclipse.jetty.http2.parser.Parser; import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.ByteBufferPool; @@ -220,6 +212,7 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. private final Callback fillableCallback = new FillableCallback(); private NetworkBuffer buffer; private boolean shutdown; + private boolean failed; private void setInputBuffer(ByteBuffer byteBuffer) { @@ -237,7 +230,7 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. if (task != null) return task; - if (isFillInterested() || shutdown) + if (isFillInterested() || shutdown || failed) return null; if (buffer == null) @@ -248,11 +241,22 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. if (parse) { buffer.retain(); - - while (buffer.hasRemaining()) - parser.parse(buffer.buffer); - - boolean released = buffer.tryRelease(); + boolean released; + try + { + while (buffer.hasRemaining()) + { + parser.parse(buffer.buffer); + if (failed) + return null; + } + } + finally + { + released = buffer.tryRelease(); + if (failed && released) + releaseNetworkBuffer(); + } task = pollTask(); if (LOG.isDebugEnabled()) @@ -345,13 +349,11 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. } } - private class ParserListener implements Parser.Listener + private class ParserListener extends Parser.Listener.Wrapper { - private final Parser.Listener listener; - private ParserListener(Parser.Listener listener) { - this.listener = listener; + super(listener); } @Override @@ -363,58 +365,11 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. session.onData(frame, callback); } - @Override - public void onHeaders(HeadersFrame frame) - { - listener.onHeaders(frame); - } - - @Override - public void onPriority(PriorityFrame frame) - { - listener.onPriority(frame); - } - - @Override - public void onReset(ResetFrame frame) - { - listener.onReset(frame); - } - - @Override - public void onSettings(SettingsFrame frame) - { - listener.onSettings(frame); - } - - @Override - public void onPushPromise(PushPromiseFrame frame) - { - listener.onPushPromise(frame); - } - - @Override - public void onPing(PingFrame frame) - { - listener.onPing(frame); - } - - @Override - public void onGoAway(GoAwayFrame frame) - { - listener.onGoAway(frame); - } - - @Override - public void onWindowUpdate(WindowUpdateFrame frame) - { - listener.onWindowUpdate(frame); - } - @Override public void onConnectionFailure(int error, String reason) { - listener.onConnectionFailure(error, reason); + producer.failed = true; + super.onConnectionFailure(error, reason); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 53fdda5a4b7..04279864c5d 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -37,6 +37,7 @@ import org.eclipse.jetty.http2.api.Session; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.DisconnectFrame; +import org.eclipse.jetty.http2.frames.FailureFrame; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.GoAwayFrame; @@ -106,7 +107,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio this.maxLocalStreams = -1; this.maxRemoteStreams = -1; this.localStreamIds.set(initialStreamId); - this.lastRemoteStreamId.set((initialStreamId & 0x01) == 0x01 ? 0 : -1); + this.lastRemoteStreamId.set(isClientStream(initialStreamId) ? 0 : -1); this.streamIdleTimeout = endPoint.getIdleTimeout(); this.sendWindow.set(FlowControlStrategy.DEFAULT_WINDOW_SIZE); this.recvWindow.set(FlowControlStrategy.DEFAULT_WINDOW_SIZE); @@ -251,7 +252,9 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio // We must enlarge the session flow control window, // otherwise other requests will be stalled. flowControl.onDataConsumed(this, null, flowControlLength); - if (isRemoteStreamClosed(streamId)) + boolean local = (streamId & 1) == (localStreamIds.get() & 1); + boolean closed = local ? isLocalStreamClosed(streamId) : isRemoteStreamClosed(streamId); + if (closed) reset(new ResetFrame(streamId, ErrorCode.STREAM_CLOSED_ERROR.code), callback); else onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_data_frame", callback); @@ -288,7 +291,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio IStream stream = getStream(streamId); if (stream != null) { - stream.process(frame, new ResetCallback()); + stream.process(frame, new OnResetCallback()); } else { @@ -501,6 +504,17 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } } + @Override + public void onStreamFailure(int streamId, int error, String reason) + { + Callback callback = new ResetCallback(streamId, error, Callback.NOOP); + IStream stream = getStream(streamId); + if (stream != null) + stream.process(new FailureFrame(error, reason), callback); + else + callback.succeeded(); + } + private boolean sumOverflows(int a, int b) { try @@ -520,7 +534,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio onConnectionFailure(error, reason, Callback.NOOP); } - private void onConnectionFailure(int error, String reason, Callback callback) + protected void onConnectionFailure(int error, String reason, Callback callback) { notifyFailure(this, new IOException(String.format("%d/%s", error, reason)), new CloseCallback(error, reason, callback)); } @@ -1181,6 +1195,12 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } } + protected static boolean isClientStream(int streamId) + { + // Client-initiated stream ids are odd. + return (streamId & 1) == 1; + } + @Override public void dump(Appendable out, String indent) throws IOException { @@ -1509,7 +1529,37 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } } - private class ResetCallback implements Callback + private class ResetCallback extends Callback.Nested + { + private final int streamId; + private final int error; + + private ResetCallback(int streamId, int error, Callback callback) + { + super(callback); + this.streamId = streamId; + this.error = error; + } + + @Override + public void succeeded() + { + complete(); + } + + @Override + public void failed(Throwable x) + { + complete(); + } + + private void complete() + { + reset(new ResetFrame(streamId, error), getCallback()); + } + } + + private class OnResetCallback implements Callback { @Override public void succeeded() @@ -1535,17 +1585,16 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } } - private class CloseCallback implements Callback + private class CloseCallback extends Callback.Nested { private final int error; private final String reason; - private final Callback callback; private CloseCallback(int error, String reason, Callback callback) { + super(callback); this.error = error; this.reason = reason; - this.callback = callback; } @Override @@ -1560,15 +1609,9 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio complete(); } - @Override - public InvocationType getInvocationType() - { - return InvocationType.NON_BLOCKING; - } - private void complete() { - close(error, reason, callback); + close(error, reason, getCallback()); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java index 6320b9efab1..ce5c3e7ad46 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java @@ -30,6 +30,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.FailureFrame; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.HeadersFrame; import org.eclipse.jetty.http2.frames.PushPromiseFrame; @@ -255,6 +256,11 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa onWindowUpdate((WindowUpdateFrame)frame, callback); break; } + case FAILURE: + { + onFailure((FailureFrame)frame, callback); + break; + } default: { throw new UnsupportedOperationException(); @@ -321,6 +327,11 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa callback.succeeded(); } + private void onFailure(FailureFrame frame, Callback callback) + { + notifyFailure(this, frame, callback); + } + @Override public boolean updateClose(boolean update, CloseState.Event event) { @@ -498,31 +509,43 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private void notifyData(Stream stream, DataFrame frame, Callback callback) { - final Listener listener = this.listener; - if (listener == null) - return; - try + Listener listener = this.listener; + if (listener != null) { - listener.onData(stream, frame, callback); + try + { + listener.onData(stream, frame, callback); + } + catch (Throwable x) + { + LOG.info("Failure while notifying listener " + listener, x); + callback.failed(x); + } } - catch (Throwable x) + else { - LOG.info("Failure while notifying listener " + listener, x); + callback.succeeded(); } } private void notifyReset(Stream stream, ResetFrame frame, Callback callback) { - final Listener listener = this.listener; - if (listener == null) - return; - try + Listener listener = this.listener; + if (listener != null) { - listener.onReset(stream, frame, callback); + try + { + listener.onReset(stream, frame, callback); + } + catch (Throwable x) + { + LOG.info("Failure while notifying listener " + listener, x); + callback.failed(x); + } } - catch (Throwable x) + else { - LOG.info("Failure while notifying listener " + listener, x); + callback.succeeded(); } } @@ -542,6 +565,27 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa } } + private void notifyFailure(Stream stream, FailureFrame frame, Callback callback) + { + Listener listener = this.listener; + if (listener != null) + { + try + { + listener.onFailure(stream, frame.getError(), frame.getReason(), callback); + } + catch (Throwable x) + { + LOG.info("Failure while notifying listener " + listener, x); + callback.failed(x); + } + } + else + { + callback.succeeded(); + } + } + @Override public String dump() { diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java index 0d8f49f886b..f76afeaf445 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java @@ -214,6 +214,11 @@ public interface Stream return true; } + public default void onFailure(Stream stream, int error, String reason, Callback callback) + { + callback.succeeded(); + } + /** *

Empty implementation of {@link Listener}

*/ diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java new file mode 100644 index 00000000000..ea16526d15d --- /dev/null +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java @@ -0,0 +1,42 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.frames; + +public class FailureFrame extends Frame +{ + private final int error; + private final String reason; + + public FailureFrame(int error, String reason) + { + super(FrameType.FAILURE); + this.error = error; + this.reason = reason; + } + + public int getError() + { + return error; + } + + public String getReason() + { + return reason; + } +} diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FrameType.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FrameType.java index 917aa99cc4a..3353c85a5b2 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FrameType.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FrameType.java @@ -35,7 +35,8 @@ public enum FrameType CONTINUATION(9), // Synthetic frames only needed by the implementation. PREFACE(10), - DISCONNECT(11); + DISCONNECT(11), + FAILURE(12); public static FrameType from(int type) { diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java index e603771dee9..57a2dd5c5d0 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/GoAwayFrame.java @@ -76,11 +76,10 @@ public class GoAwayFrame extends Frame @Override public String toString() { - ErrorCode errorCode = ErrorCode.from(error); return String.format("%s,%d/%s/%s/%s", super.toString(), lastStreamId, - errorCode != null ? errorCode.toString() : String.valueOf(error), + ErrorCode.toString(error, null), tryConvertPayload(), closeState); } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/ResetFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/ResetFrame.java index d86c8ca9713..b05a6812ca6 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/ResetFrame.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/ResetFrame.java @@ -18,8 +18,6 @@ package org.eclipse.jetty.http2.frames; -import java.util.Locale; - import org.eclipse.jetty.http2.ErrorCode; public class ResetFrame extends Frame @@ -49,8 +47,6 @@ public class ResetFrame extends Frame @Override public String toString() { - ErrorCode errorCode = ErrorCode.from(error); - String reason = errorCode == null ? "error=" + error : errorCode.name().toLowerCase(Locale.ENGLISH); - return String.format("%s#%d{%s}", super.toString(), streamId, reason); + return String.format("%s#%d{%s}", super.toString(), streamId, ErrorCode.toString(error, null)); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java index 68b655ce694..369f312618e 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java @@ -222,4 +222,21 @@ public abstract class BodyParser LOG.info("Failure while notifying listener " + listener, x); } } + + protected void streamFailure(int streamId, int error, String reason) + { + notifyStreamFailure(streamId, error, reason); + } + + private void notifyStreamFailure(int streamId, int error, String reason) + { + try + { + listener.onStreamFailure(streamId, error, reason); + } + catch (Throwable x) + { + LOG.info("Failure while notifying listener " + listener, x); + } + } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ContinuationBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ContinuationBodyParser.java index ca997feba70..db52392be3b 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ContinuationBodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ContinuationBodyParser.java @@ -81,7 +81,7 @@ public class ContinuationBodyParser extends BodyParser headerBlockFragments.storeFragment(buffer, length, last); reset(); if (last) - onHeaders(); + return onHeaders(); return true; } } @@ -94,12 +94,17 @@ public class ContinuationBodyParser extends BodyParser return false; } - private void onHeaders() + private boolean onHeaders() { ByteBuffer headerBlock = headerBlockFragments.complete(); MetaData metaData = headerBlockParser.parse(headerBlock, headerBlock.remaining()); + if (metaData == HeaderBlockParser.SESSION_FAILURE) + return false; + if (metaData == null || metaData == HeaderBlockParser.STREAM_FAILURE) + return true; HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, headerBlockFragments.getPriorityFrame(), headerBlockFragments.isEndStream()); notifyHeaders(frame); + return true; } private void reset() diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java index 922388be2a1..7dc2e2c357c 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java @@ -20,26 +20,46 @@ package org.eclipse.jetty.http2.parser; import java.nio.ByteBuffer; -import org.eclipse.jetty.http.BadMessageException; +import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.hpack.HpackDecoder; -import org.eclipse.jetty.http2.hpack.HpackException.SessionException; -import org.eclipse.jetty.http2.hpack.HpackException.StreamException; +import org.eclipse.jetty.http2.hpack.HpackException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; public class HeaderBlockParser { + public static final MetaData STREAM_FAILURE = new MetaData(HttpVersion.HTTP_2, null); + public static final MetaData SESSION_FAILURE = new MetaData(HttpVersion.HTTP_2, null); + private static final Logger LOG = Log.getLogger(HeaderBlockParser.class); + + private final HeaderParser headerParser; private final ByteBufferPool byteBufferPool; private final HpackDecoder hpackDecoder; + private final BodyParser notifier; private ByteBuffer blockBuffer; - public HeaderBlockParser(ByteBufferPool byteBufferPool, HpackDecoder hpackDecoder) + public HeaderBlockParser(HeaderParser headerParser, ByteBufferPool byteBufferPool, HpackDecoder hpackDecoder, BodyParser notifier) { + this.headerParser = headerParser; this.byteBufferPool = byteBufferPool; this.hpackDecoder = hpackDecoder; + this.notifier = notifier; } + /** + * Parses @{code blockLength} HPACK bytes from the given {@code buffer}. + * + * @param buffer the buffer to parse + * @param blockLength the length of the HPACK block + * @return null, if the buffer contains less than {@code blockLength} bytes; + * {@link #STREAM_FAILURE} if parsing the HPACK block produced a stream failure; + * {@link #SESSION_FAILURE} if parsing the HPACK block produced a session failure; + * a valid MetaData object if the parsing was successful. + */ public MetaData parse(ByteBuffer buffer, int blockLength) { // We must wait for the all the bytes of the header block to arrive. @@ -77,35 +97,28 @@ public class HeaderBlockParser try { - MetaData metadata = hpackDecoder.decode(toDecode); - - if (metadata instanceof MetaData.Request) - { - // TODO this must be an initial HEADERs frame received by the server OR - // TODO a push promise received by the client. - // TODO this must not be a trailers frame - } - else if (metadata instanceof MetaData.Response) - { - // TODO this must be an initial HEADERs frame received by the client - // TODO this must not be a trailers frame - } - else - { - // TODO this must be a trailers frame - } - - return metadata; + return hpackDecoder.decode(toDecode); } - catch(StreamException ex) + catch (HpackException.StreamException x) { - // TODO reset the stream - throw new BadMessageException("TODO"); + if (LOG.isDebugEnabled()) + LOG.debug(x); + notifier.streamFailure(headerParser.getStreamId(), ErrorCode.PROTOCOL_ERROR.code, "invalid_hpack_block"); + return STREAM_FAILURE; } - catch(SessionException ex) + catch (HpackException.CompressionException x) { - // TODO reset the session - throw new BadMessageException("TODO"); + if (LOG.isDebugEnabled()) + LOG.debug(x); + notifier.connectionFailure(buffer, ErrorCode.COMPRESSION_ERROR.code, "invalid_hpack_block"); + return SESSION_FAILURE; + } + catch (HpackException.SessionException x) + { + if (LOG.isDebugEnabled()) + LOG.debug(x); + notifier.connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_hpack_block"); + return SESSION_FAILURE; } finally { diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java index f31c937b33b..2a1df70a4cd 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java @@ -170,13 +170,16 @@ public class HeadersBodyParser extends BodyParser if (hasFlag(Flags.END_HEADERS)) { MetaData metaData = headerBlockParser.parse(buffer, length); + if (metaData == HeaderBlockParser.SESSION_FAILURE) + return false; if (metaData != null) { if (LOG.isDebugEnabled()) LOG.debug("Parsed {} frame hpack from {}", FrameType.HEADERS, buffer); state = State.PADDING; loop = paddingLength == 0; - onHeaders(parentStreamId, weight, exclusive, metaData); + if (metaData != HeaderBlockParser.STREAM_FAILURE) + onHeaders(parentStreamId, weight, exclusive, metaData); } } else diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java index 71e49393399..4b45c7320fd 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java @@ -36,7 +36,6 @@ import org.eclipse.jetty.http2.frames.SettingsFrame; import org.eclipse.jetty.http2.frames.WindowUpdateFrame; import org.eclipse.jetty.http2.hpack.HpackDecoder; import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -63,8 +62,8 @@ public class Parser { this.listener = listener; this.headerParser = new HeaderParser(); - this.headerBlockParser = new HeaderBlockParser(byteBufferPool, new HpackDecoder(maxDynamicTableSize, maxHeaderSize)); this.unknownBodyParser = new UnknownBodyParser(headerParser, listener); + this.headerBlockParser = new HeaderBlockParser(headerParser, byteBufferPool, new HpackDecoder(maxDynamicTableSize, maxHeaderSize), unknownBodyParser); this.maxFrameLength = Frame.DEFAULT_MAX_LENGTH; this.bodyParsers = new BodyParser[FrameType.values().length]; } @@ -265,6 +264,8 @@ public class Parser public void onWindowUpdate(WindowUpdateFrame frame); + public void onStreamFailure(int streamId, int error, String reason); + public void onConnectionFailure(int error, String reason); public static class Adapter implements Listener @@ -314,12 +315,98 @@ public class Parser { } + @Override + public void onStreamFailure(int streamId, int error, String reason) + { + } + @Override public void onConnectionFailure(int error, String reason) { LOG.warn("Connection failure: {}/{}", error, reason); } } + + public static class Wrapper implements Listener + { + private final Parser.Listener listener; + + public Wrapper(Parser.Listener listener) + { + this.listener = listener; + } + + public Listener getParserListener() + { + return listener; + } + + @Override + public void onData(DataFrame frame) + { + listener.onData(frame); + } + + @Override + public void onHeaders(HeadersFrame frame) + { + listener.onHeaders(frame); + } + + @Override + public void onPriority(PriorityFrame frame) + { + listener.onPriority(frame); + } + + @Override + public void onReset(ResetFrame frame) + { + listener.onReset(frame); + } + + @Override + public void onSettings(SettingsFrame frame) + { + listener.onSettings(frame); + } + + @Override + public void onPushPromise(PushPromiseFrame frame) + { + listener.onPushPromise(frame); + } + + @Override + public void onPing(PingFrame frame) + { + listener.onPing(frame); + } + + @Override + public void onGoAway(GoAwayFrame frame) + { + listener.onGoAway(frame); + } + + @Override + public void onWindowUpdate(WindowUpdateFrame frame) + { + listener.onWindowUpdate(frame); + } + + @Override + public void onStreamFailure(int streamId, int error, String reason) + { + listener.onStreamFailure(streamId, error, reason); + } + + @Override + public void onConnectionFailure(int error, String reason) + { + listener.onConnectionFailure(error, reason); + } + } } private enum State diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java index 508936b2c02..c46da3dcb39 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java @@ -125,11 +125,14 @@ public class PushPromiseBodyParser extends BodyParser case HEADERS: { MetaData metaData = headerBlockParser.parse(buffer, length); + if (metaData == HeaderBlockParser.SESSION_FAILURE) + return false; if (metaData != null) { state = State.PADDING; loop = paddingLength == 0; - onPushPromise(streamId, metaData); + if (metaData != HeaderBlockParser.STREAM_FAILURE) + onPushPromise(streamId, metaData); } break; } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ServerParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ServerParser.java index 2b2c3a1dfd1..2f688b1161c 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ServerParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ServerParser.java @@ -158,6 +158,26 @@ public class ServerParser extends Parser { } } + + public static class Wrapper extends Parser.Listener.Wrapper implements Listener + { + public Wrapper(ServerParser.Listener listener) + { + super(listener); + } + + @Override + public ServerParser.Listener getParserListener() + { + return (Listener)super.getParserListener(); + } + + @Override + public void onPreface() + { + getParserListener().onPreface(); + } + } } private enum State diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 083f10b8bc5..df7b2a6e971 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java @@ -23,10 +23,8 @@ import java.nio.ByteBuffer; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.hpack.HpackContext.Entry; - import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java index 0bb5bb70fe3..9f9ee924f97 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java @@ -21,8 +21,11 @@ package org.eclipse.jetty.http2.hpack; import java.nio.ByteBuffer; import java.util.EnumSet; +import java.util.Set; +import java.util.stream.Collectors; import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; @@ -31,6 +34,9 @@ import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http2.hpack.HpackContext.Entry; import org.eclipse.jetty.http2.hpack.HpackContext.StaticEntry; +import org.eclipse.jetty.util.ArrayTrie; +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.Trie; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -38,17 +44,13 @@ import org.eclipse.jetty.util.log.Logger; public class HpackEncoder { public static final Logger LOG = Log.getLogger(HpackEncoder.class); - private final static HttpField[] __status= new HttpField[599]; - - final static EnumSet __DO_NOT_HUFFMAN = EnumSet.of( HttpHeader.AUTHORIZATION, HttpHeader.CONTENT_MD5, HttpHeader.PROXY_AUTHENTICATE, HttpHeader.PROXY_AUTHORIZATION); - final static EnumSet __DO_NOT_INDEX = EnumSet.of( // HttpHeader.C_PATH, // TODO more data needed @@ -69,18 +71,21 @@ public class HpackEncoder HttpHeader.LAST_MODIFIED, HttpHeader.SET_COOKIE, HttpHeader.SET_COOKIE2); - - final static EnumSet __NEVER_INDEX = EnumSet.of( HttpHeader.AUTHORIZATION, HttpHeader.SET_COOKIE, HttpHeader.SET_COOKIE2); + private static final PreEncodedHttpField CONNECTION_TE = new PreEncodedHttpField(HttpHeader.CONNECTION, "te"); + private static final PreEncodedHttpField TE_TRAILERS = new PreEncodedHttpField(HttpHeader.TE, "trailers"); + private static final Trie specialHopHeaders = new ArrayTrie<>(6); static { for (HttpStatus.Code code : HttpStatus.Code.values()) __status[code.getCode()]=new PreEncodedHttpField(HttpHeader.C_STATUS,Integer.toString(code.getCode())); + specialHopHeaders.put("close", true); + specialHopHeaders.put("te", true); } private final HpackContext _context; @@ -174,9 +179,30 @@ public class HpackEncoder encode(buffer,status); } - // Add all the other fields - for (HttpField field : metadata) - encode(buffer,field); + // Add all non-connection fields. + HttpFields fields = metadata.getFields(); + if (fields != null) + { + Set hopHeaders = fields.getCSV(HttpHeader.CONNECTION, false).stream() + .filter(v -> specialHopHeaders.get(v) == Boolean.TRUE) + .map(StringUtil::asciiToLowerCase) + .collect(Collectors.toSet()); + for (HttpField field : fields) + { + if (field.getHeader() == HttpHeader.CONNECTION) + continue; + if (!hopHeaders.isEmpty() && hopHeaders.contains(StringUtil.asciiToLowerCase(field.getName()))) + continue; + if (field.getHeader() == HttpHeader.TE) + { + if (!field.contains("trailers")) + continue; + encode(buffer, CONNECTION_TE); + encode(buffer, TE_TRAILERS); + } + encode(buffer,field); + } + } // Check size if (_maxHeaderListSize>0 && _headerListSize>_maxHeaderListSize) @@ -305,7 +331,7 @@ public class HpackEncoder encoding="Lit"+ ((name==null)?"HuffN":("IdxN"+(name.isStatic()?"S":"")+(1+NBitInteger.octectsNeeded(4,_context.index(name)))))+ (huffman?"HuffV":"LitV")+ - (indexed?"Idx":(never_index?"!!Idx":"!Idx")); + (never_index?"!!Idx":"!Idx"); } else if (field_size>=_context.getMaxDynamicTableSize() || header==HttpHeader.CONTENT_LENGTH && field.getValue().length()>2) { diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java index 74cc77da705..d891ab3dfe5 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java @@ -170,19 +170,19 @@ public class MetaDataBuilder if ("trailers".equalsIgnoreCase(value)) _fields.add(field); else - streamException("Unsupported TE value %s", value); + streamException("Unsupported TE value '%s'", value); break; case CONNECTION: if ("TE".equalsIgnoreCase(value)) _fields.add(field); else - streamException("Connection specific field %s", header); + streamException("Connection specific field '%s'", header); break; default: if (name.charAt(0)==':') - streamException("Unknown pseudo header %s", name); + streamException("Unknown pseudo header '%s'", name); else _fields.add(field); break; @@ -191,7 +191,7 @@ public class MetaDataBuilder else { if (name.charAt(0)==':') - streamException("Unknown pseudo header %s",name); + streamException("Unknown pseudo header '%s'",name); else _fields.add(field); } @@ -281,7 +281,7 @@ public class MetaDataBuilder * Check that the max size will not be exceeded. * @param length the length * @param huffman the huffman name - * @throws SessionException + * @throws SessionException in case of size errors */ public void checkSize(int length, boolean huffman) throws SessionException { diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 05b21814c39..37b65015f78 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -19,12 +19,6 @@ package org.eclipse.jetty.http2.hpack; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - import java.nio.ByteBuffer; import java.util.Iterator; @@ -39,6 +33,12 @@ import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + public class HpackDecoderTest { @Test @@ -327,7 +327,7 @@ public class HpackDecoderTest } catch(StreamException ex) { - Assert.assertThat(ex.getMessage(),Matchers.containsString("Connection specific field Connection")); + Assert.assertThat(ex.getMessage(),Matchers.containsString("Connection specific field 'Connection'")); } // 2: Sends a HEADERS frame that contains the TE header field with any value other than "trailers" @@ -340,7 +340,7 @@ public class HpackDecoderTest } catch(StreamException ex) { - Assert.assertThat(ex.getMessage(),Matchers.containsString("Unsupported TE value not_trailers")); + Assert.assertThat(ex.getMessage(),Matchers.containsString("Unsupported TE value 'not_trailers'")); } diff --git a/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties b/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties index e40e8e43ce1..f4e33644f4c 100644 --- a/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties +++ b/jetty-http2/http2-hpack/src/test/resources/jetty-logging.properties @@ -1,3 +1,3 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog -org.eclipse.jetty.http2.LEVEL=INFO -org.eclipse.jetty.http2.hpack.LEVEL=INFO +#org.eclipse.jetty.http2.LEVEL=DEBUG +#org.eclipse.jetty.http2.hpack.LEVEL=DEBUG diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index 354e19f5e45..a0f1817a6bb 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -23,7 +23,6 @@ import java.nio.ByteBuffer; import java.util.ArrayDeque; import java.util.Collections; import java.util.List; -import java.util.Locale; import java.util.Queue; import java.util.function.BiFunction; @@ -165,10 +164,8 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen HttpExchange exchange = getHttpExchange(); if (exchange == null) return; - - ErrorCode error = ErrorCode.from(frame.getError()); - String reason = error == null ? "reset" : error.name().toLowerCase(Locale.ENGLISH); - exchange.getRequest().abort(new IOException(reason)); + int error = frame.getError(); + exchange.getRequest().abort(new IOException(ErrorCode.toString(error, "reset_code_" + error))); } @Override @@ -178,6 +175,13 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen return true; } + @Override + public void onFailure(Stream stream, int error, String reason, Callback callback) + { + responseFailure(new IOException(String.format("%s/%s", ErrorCode.toString(error, null), reason))); + callback.succeeded(); + } + private void notifyContent(HttpExchange exchange, DataFrame frame, Callback callback) { contentNotifier.offer(new DataInfo(exchange, frame, callback)); diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java index e668fc20056..a6eb2acb7a8 100644 --- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java @@ -576,6 +576,40 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest Assert.assertArrayEquals(bytes, response.getContent()); } + @Test + public void testInvalidResponseHPack() throws Exception + { + start(new ServerSessionListener.Adapter() + { + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + // Produce an invalid HPACK block by adding a request pseudo-header to the response. + HttpFields fields = new HttpFields(); + fields.put(":method", "get"); + MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, fields, 0); + int streamId = stream.getId(); + HeadersFrame responseFrame = new HeadersFrame(streamId, response, null, false); + Callback.Completable callback = new Callback.Completable(); + stream.headers(responseFrame, callback); + byte[] bytes = "hello".getBytes(StandardCharsets.US_ASCII); + callback.thenRun(() -> stream.data(new DataFrame(streamId, ByteBuffer.wrap(bytes), true), Callback.NOOP)); + return null; + } + }); + + CountDownLatch latch = new CountDownLatch(1); + client.newRequest("localhost", connector.getLocalPort()) + .timeout(5, TimeUnit.SECONDS) + .send(result -> + { + if (result.isFailed()) + latch.countDown(); + }); + + Assert.assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + @Ignore @Test public void testExternalServer() throws Exception diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java index eee89bd9b48..3feb1f12fad 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java @@ -115,13 +115,10 @@ public class HTTP2ServerConnectionFactory extends AbstractHTTP2ServerConnectionF @Override public void onClose(Session session, GoAwayFrame frame, Callback callback) { - ErrorCode error = ErrorCode.from(frame.getError()); - if (error == null) - error = ErrorCode.STREAM_CLOSED_ERROR; String reason = frame.tryConvertPayload(); if (reason != null && !reason.isEmpty()) reason = " (" + reason + ")"; - getConnection().onSessionFailure(new EofException("HTTP/2 " + error + reason), callback); + getConnection().onSessionFailure(new EofException(String.format("Close %s/%s", ErrorCode.toString(frame.getError(), null), reason)), callback); } @Override @@ -156,10 +153,13 @@ public class HTTP2ServerConnectionFactory extends AbstractHTTP2ServerConnectionF @Override public void onReset(Stream stream, ResetFrame frame, Callback callback) { - ErrorCode error = ErrorCode.from(frame.getError()); - if (error == null) - error = ErrorCode.CANCEL_STREAM_ERROR; - getConnection().onStreamFailure((IStream)stream, new EofException("HTTP/2 " + error), callback); + getConnection().onStreamFailure((IStream)stream, new EofException("Reset " + ErrorCode.toString(frame.getError(), null)), callback); + } + + @Override + public void onFailure(Stream stream, int error, String reason, Callback callback) + { + getConnection().onStreamFailure((IStream)stream, new EofException(String.format("Failure %s/%s", ErrorCode.toString(error, null), reason)), callback); } @Override diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java index 37882e3650f..c89c72ed9ca 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerSession.java @@ -84,7 +84,7 @@ public class HTTP2ServerSession extends HTTP2Session implements ServerParser.Lis LOG.debug("Received {}", frame); int streamId = frame.getStreamId(); - if ((streamId & 1) != 1) + if (!isClientStream(streamId)) { onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "invalid_stream_id"); return; From 4b0becbbe04e929fe31ad2c2abd07ec83aad7c0f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 21 Jul 2018 12:52:44 +0200 Subject: [PATCH 24/26] Issue #2679 - h2spec compliance. Fixed content-length compliance. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http2/HTTP2Stream.java | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java index ce5c3e7ad46..b9609ad4903 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java @@ -28,6 +28,9 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.FailureFrame; @@ -59,9 +62,10 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private final ISession session; private final int streamId; private final boolean local; - private volatile Listener listener; - private volatile boolean localReset; - private volatile boolean remoteReset; + private boolean localReset; + private Listener listener; + private boolean remoteReset; + private long dataLength; public HTTP2Stream(Scheduler scheduler, ISession session, int streamId, boolean local) { @@ -69,6 +73,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa this.session = session; this.streamId = streamId; this.local = local; + this.dataLength = Long.MIN_VALUE; } @Override @@ -272,6 +277,15 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa { if (updateClose(frame.isEndStream(), CloseState.Event.RECEIVED)) session.removeStream(this); + MetaData metaData = frame.getMetaData(); + if (metaData.isRequest() || metaData.isResponse()) + { + HttpFields fields = metaData.getFields(); + long length = -1; + if (fields != null) + length = fields.getLongField(HttpHeader.CONTENT_LENGTH.asString()); + dataLength = length >= 0 ? length : Long.MIN_VALUE; + } callback.succeeded(); } @@ -301,8 +315,20 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa return; } + if (dataLength != Long.MIN_VALUE) + { + dataLength -= frame.remaining(); + if (frame.isEndStream() && dataLength != 0) + { + reset(new ResetFrame(streamId, ErrorCode.PROTOCOL_ERROR.code), Callback.NOOP); + callback.failed(new IOException("invalid_data_length")); + return; + } + } + if (updateClose(frame.isEndStream(), CloseState.Event.RECEIVED)) session.removeStream(this); + notifyData(this, frame, callback); } From b5e607068f435116bfe5b7ad6e703938471bec20 Mon Sep 17 00:00:00 2001 From: lachan-roberts Date: Sat, 21 Jul 2018 22:10:12 +1000 Subject: [PATCH 25/26] Issue #2679 - HTTP/2 Spec Compliance now handling padding over 7 bits with Exception throwing CompressionExceptions instead of StreamExceptions fixed issue with non terminal encoded strings Signed-off-by: lachan-roberts --- .../eclipse/jetty/http2/hpack/Huffman.java | 22 +++---- .../jetty/http2/hpack/HpackDecoderTest.java | 60 +++++++++++++++---- .../jetty/http2/hpack/HuffmanTest.java | 11 ---- 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java index 8a1dbb3c5ed..ddb2c4a97df 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java @@ -345,24 +345,25 @@ public class Huffman } } - public static String decode(ByteBuffer buffer) throws HpackException.StreamException + public static String decode(ByteBuffer buffer) throws HpackException.CompressionException { return decode(buffer,buffer.remaining()); } - public static String decode(ByteBuffer buffer,int length) throws HpackException.StreamException + public static String decode(ByteBuffer buffer,int length) throws HpackException.CompressionException { StringBuilder out = new StringBuilder(length*2); int node = 0; int current = 0; int bits = 0; - + byte[] array = buffer.array(); int position=buffer.position(); int start=buffer.arrayOffset()+position; int end=start+length; buffer.position(position+length); - + + for (int i=start; i 0) { int c = (current << (8 - bits)) & 0xFF; + int lastNode = node; node = tree[node*256+c]; if (rowbits[node]==0 || rowbits[node] > bits) @@ -402,20 +404,20 @@ public class Huffman requiredPadding = (requiredPadding << 1) | 1; if((c>>(8-bits)) != requiredPadding) - throw new HpackException.StreamException("Incorrect padding"); + throw new HpackException.CompressionException("Incorrect padding"); + node = lastNode; break; } - // TODO why is this even here - if (rowbits[node]==0) - throw new IllegalStateException(); - out.append(rowsym[node]); bits -= rowbits[node]; node = 0; } + if(node != 0) + throw new HpackException.CompressionException("Bad termination"); + return out.toString(); } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 03982a01241..60bca31b5a2 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java @@ -19,12 +19,6 @@ package org.eclipse.jetty.http2.hpack; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - import java.nio.ByteBuffer; import java.util.Iterator; @@ -39,6 +33,12 @@ import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + public class HpackDecoderTest { @Test @@ -495,9 +495,9 @@ public class HpackDecoderTest decoder.decode(buffer); Assert.fail(); } - catch (StreamException ex) + catch (CompressionException ex) { - Assert.assertThat(ex.getMessage(), Matchers.containsString("Padding length exceeded")); + Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination")); } } @@ -516,7 +516,7 @@ public class HpackDecoderTest decoder.decode(buffer); Assert.fail(); } - catch (StreamException ex) + catch (CompressionException ex) { Assert.assertThat(ex.getMessage(), Matchers.containsString("Incorrect padding")); } @@ -537,9 +537,49 @@ public class HpackDecoderTest decoder.decode(buffer); Assert.fail(); } - catch (StreamException ex) + catch (CompressionException ex) { Assert.assertThat(ex.getMessage(), Matchers.containsString("EOS in content")); } } + + + @Test() + public void testHuffmanEncodedOneIncompleteOctet() throws Exception + { + HpackDecoder decoder = new HpackDecoder(4096, 8192); + + String encoded = "82868441" + "81" + "FE"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + + try + { + decoder.decode(buffer); + Assert.fail(); + } + catch (CompressionException ex) + { + Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination")); + } + } + + + @Test() + public void testHuffmanEncodedTwoIncompleteOctet() throws Exception + { + HpackDecoder decoder = new HpackDecoder(4096, 8192); + + String encoded = "82868441" + "82" + "FFFE"; + ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded)); + + try + { + decoder.decode(buffer); + Assert.fail(); + } + catch (CompressionException ex) + { + Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination")); + } + } } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java index 61a6f42e184..3b697ac0fe5 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java @@ -50,17 +50,6 @@ public class HuffmanTest } } - @Test - public void testDecodeTrailingFF() throws Exception - { - for (String[] test:tests) - { - byte[] encoded=TypeUtil.fromHexString(test[1]+"FF"); - String decoded=Huffman.decode(ByteBuffer.wrap(encoded)); - Assert.assertEquals(test[0],test[2],decoded); - } - } - @Test public void testEncode() throws Exception { From 9a22dd820ab43c0068a09683acf6dba890626ab4 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 21 Jul 2018 15:55:43 +0200 Subject: [PATCH 26/26] Issue #2679 - h2spec compliance. Integrated h2spec execution in the Maven build. Signed-off-by: Simone Bordet --- jetty-http2/http2-server/pom.xml | 108 +++++++++++------- .../jetty/http2/server/H2SpecServer.java | 48 ++++++++ pom.xml | 5 + 3 files changed, 118 insertions(+), 43 deletions(-) create mode 100644 jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/H2SpecServer.java diff --git a/jetty-http2/http2-server/pom.xml b/jetty-http2/http2-server/pom.xml index 7313aa64f32..01532422e39 100644 --- a/jetty-http2/http2-server/pom.xml +++ b/jetty-http2/http2-server/pom.xml @@ -1,5 +1,6 @@ - + org.eclipse.jetty.http2 http2-parent @@ -10,55 +11,76 @@ http2-server Jetty :: HTTP2 :: Server - + ${project.groupId}.server + + com.github.madgnome + h2spec-maven-plugin + + org.eclipse.jetty.http2.server.H2SpecServer + ${skipTests} + + + 5.1 - closed: Sends a DATA frame + + + + + h2spec + test + + h2spec + + + + - - - org.eclipse.jetty.http2 - http2-common - ${project.version} - - - org.eclipse.jetty - jetty-server - ${project.version} - - - org.eclipse.jetty.alpn - alpn-api - ${alpn.api.version} - provided - - - org.eclipse.jetty - jetty-servlet - ${project.version} - test - - - org.eclipse.jetty - jetty-servlets - ${project.version} - test - - - org.eclipse.jetty - jetty-alpn-server - ${project.version} - test - - - org.eclipse.jetty.toolchain - jetty-test-helper - test - - + + + org.eclipse.jetty.http2 + http2-common + ${project.version} + + + org.eclipse.jetty + jetty-server + ${project.version} + + + org.eclipse.jetty.alpn + alpn-api + ${alpn.api.version} + provided + + + org.eclipse.jetty + jetty-servlet + ${project.version} + test + + + org.eclipse.jetty + jetty-servlets + ${project.version} + test + + + org.eclipse.jetty + jetty-alpn-server + ${project.version} + test + + + org.eclipse.jetty.toolchain + jetty-test-helper + test + + diff --git a/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/H2SpecServer.java b/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/H2SpecServer.java new file mode 100644 index 00000000000..de99425b90b --- /dev/null +++ b/jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/H2SpecServer.java @@ -0,0 +1,48 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.server; + +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; + +/** + * HTTP/2 server to run the 'h2spec' tool against. + */ +public class H2SpecServer +{ + public static void main(String[] args) throws Exception + { + int port = Integer.parseInt(args[0]); + + Server server = new Server(); + + HttpConfiguration http_config = new HttpConfiguration(); + http_config.setRequestHeaderSize(16 * 1024); + + HttpConnectionFactory http = new HttpConnectionFactory(http_config); + HTTP2CServerConnectionFactory h2c = new HTTP2CServerConnectionFactory(http_config); + ServerConnector connector = new ServerConnector(server, http, h2c); + connector.setPort(port); + server.addConnector(connector); + + server.start(); + } +} diff --git a/pom.xml b/pom.xml index 5e1d12d5c4c..8309d7aab73 100644 --- a/pom.xml +++ b/pom.xml @@ -887,6 +887,11 @@ buildnumber-maven-plugin 1.4 + + com.github.madgnome + h2spec-maven-plugin + 0.3 +