From 494cfe7dcaa8ee603902b26af5e1d8177cf8fc41 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 13 Aug 2020 12:34:47 +1200 Subject: [PATCH] Sync HPackDecoder with ASP.NET Core changes (#40689) * Sync HPackDecoder with ASP.NET Core changes --- .../Http/aspnetcore/Http2/Hpack/HPackDecoder.cs | 65 +++++++++++++++------- .../Net/aspnetcore/Http2/HPackDecoderTest.cs | 30 ++++++++-- .../Net/Http/SocketsHttpHandler/Http2Stream.cs | 6 +- .../tests/UnitTests/HPack/HPackRoundtripTests.cs | 5 +- 4 files changed, 78 insertions(+), 28 deletions(-) diff --git a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackDecoder.cs b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackDecoder.cs index bb9b988..8899104 100644 --- a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackDecoder.cs +++ b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackDecoder.cs @@ -92,6 +92,7 @@ namespace System.Net.Http.HPack private State _state = State.Ready; private byte[]? _headerName; + private int _headerStaticIndex; private int _stringIndex; private int _stringLength; private int _headerNameLength; @@ -492,23 +493,36 @@ namespace System.Net.Http.HPack private void ProcessHeaderValue(ReadOnlySpan data, IHttpHeadersHandler handler) { - ReadOnlySpan headerNameSpan = _headerNameRange == null - ? new Span(_headerName, 0, _headerNameLength) - : data.Slice(_headerNameRange.GetValueOrDefault().start, _headerNameRange.GetValueOrDefault().length); - ReadOnlySpan headerValueSpan = _headerValueRange == null - ? new Span(_headerValueOctets, 0, _headerValueLength) + ? _headerValueOctets.AsSpan(0, _headerValueLength) : data.Slice(_headerValueRange.GetValueOrDefault().start, _headerValueRange.GetValueOrDefault().length); - handler.OnHeader(headerNameSpan, headerValueSpan); - - _headerNameRange = null; - _headerValueRange = null; + if (_headerStaticIndex > 0) + { + handler.OnStaticIndexedHeader(_headerStaticIndex, headerValueSpan); - if (_index) + if (_index) + { + _dynamicTable.Insert(H2StaticTable.Get(_headerStaticIndex - 1).Name, headerValueSpan); + } + } + else { - _dynamicTable.Insert(headerNameSpan, headerValueSpan); + ReadOnlySpan headerNameSpan = _headerNameRange == null + ? _headerName.AsSpan(0, _headerNameLength) + : data.Slice(_headerNameRange.GetValueOrDefault().start, _headerNameRange.GetValueOrDefault().length); + + handler.OnHeader(headerNameSpan, headerValueSpan); + + if (_index) + { + _dynamicTable.Insert(headerNameSpan, headerValueSpan); + } } + + _headerStaticIndex = 0; + _headerNameRange = null; + _headerValueRange = null; } public void CompleteDecode() @@ -522,15 +536,30 @@ namespace System.Net.Http.HPack private void OnIndexedHeaderField(int index, IHttpHeadersHandler handler) { - ref readonly HeaderField header = ref GetHeader(index); - handler.OnHeader(header.Name, header.Value); + if (index <= H2StaticTable.Count) + { + handler.OnStaticIndexedHeader(index); + } + else + { + ref readonly HeaderField header = ref GetDynamicHeader(index); + handler.OnHeader(header.Name, header.Value); + } + _state = State.Ready; } private void OnIndexedHeaderName(int index) { - _headerName = GetHeader(index).Name; - _headerNameLength = _headerName.Length; + if (index <= H2StaticTable.Count) + { + _headerStaticIndex = index; + } + else + { + _headerName = GetDynamicHeader(index).Name; + _headerNameLength = _headerName.Length; + } _state = State.HeaderValueLength; } @@ -615,13 +644,11 @@ namespace System.Net.Http.HPack return (b & HuffmanMask) != 0; } - private ref readonly HeaderField GetHeader(int index) + private ref readonly HeaderField GetDynamicHeader(int index) { try { - return ref index <= H2StaticTable.Count - ? ref H2StaticTable.Get(index - 1) - : ref _dynamicTable[index - H2StaticTable.Count - 1]; + return ref _dynamicTable[index - H2StaticTable.Count - 1]; } catch (IndexOutOfRangeException) { diff --git a/src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http2/HPackDecoderTest.cs b/src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http2/HPackDecoderTest.cs index 40221b8..6c84993 100644 --- a/src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http2/HPackDecoderTest.cs +++ b/src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http2/HPackDecoderTest.cs @@ -104,10 +104,27 @@ namespace System.Net.Http.Unit.Tests.HPack } [Fact] - public void DecodesIndexedHeaderField_StaticTable() + public void DecodesIndexedHeaderField_StaticTableWithValue() { _decoder.Decode(_indexedHeaderStatic, endHeaders: true, handler: _handler); Assert.Equal("GET", _handler.DecodedHeaders[":method"]); + + Assert.Equal(":method", _handler.DecodedStaticHeaders[H2StaticTable.MethodGet].Key); + Assert.Equal("GET", _handler.DecodedStaticHeaders[H2StaticTable.MethodGet].Value); + } + + [Fact] + public void DecodesIndexedHeaderField_StaticTableWithoutValue() + { + byte[] encoded = _literalHeaderFieldWithIndexingIndexedName + .Concat(_headerValue) + .ToArray(); + + _decoder.Decode(encoded, endHeaders: true, handler: _handler); + Assert.Equal(_headerValueString, _handler.DecodedHeaders[_userAgentString]); + + Assert.Equal(_userAgentString, _handler.DecodedStaticHeaders[H2StaticTable.UserAgent].Key); + Assert.Equal(_headerValueString, _handler.DecodedStaticHeaders[H2StaticTable.UserAgent].Value); } [Fact] @@ -707,6 +724,7 @@ namespace System.Net.Http.Unit.Tests.HPack public class TestHttpHeadersHandler : IHttpHeadersHandler { public Dictionary DecodedHeaders { get; } = new Dictionary(); + public Dictionary> DecodedStaticHeaders { get; } = new Dictionary>(); void IHttpHeadersHandler.OnHeader(ReadOnlySpan name, ReadOnlySpan value) { @@ -718,14 +736,16 @@ namespace System.Net.Http.Unit.Tests.HPack void IHttpHeadersHandler.OnStaticIndexedHeader(int index) { - // Not yet implemented for HPACK. - throw new NotImplementedException(); + ref readonly HeaderField entry = ref H2StaticTable.Get(index - 1); + ((IHttpHeadersHandler)this).OnHeader(entry.Name, entry.Value); + DecodedStaticHeaders[index] = new KeyValuePair(Encoding.ASCII.GetString(entry.Name), Encoding.ASCII.GetString(entry.Value)); } void IHttpHeadersHandler.OnStaticIndexedHeader(int index, ReadOnlySpan value) { - // Not yet implemented for HPACK. - throw new NotImplementedException(); + byte[] name = H2StaticTable.Get(index - 1).Name; + ((IHttpHeadersHandler)this).OnHeader(name, value); + DecodedStaticHeaders[index] = new KeyValuePair(Encoding.ASCII.GetString(name), Encoding.ASCII.GetString(value)); } void IHttpHeadersHandler.OnHeadersComplete(bool endStream) { } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 8671947..c019cd4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Net.Http.Headers; +using System.Net.Http.HPack; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Text; @@ -440,13 +441,14 @@ namespace System.Net.Http void IHttpHeadersHandler.OnStaticIndexedHeader(int index) { // TODO: https://github.com/dotnet/runtime/issues/1505 - Debug.Fail("Currently unused by HPACK, this should never be called."); + ref readonly HeaderField entry = ref H2StaticTable.Get(index - 1); + OnHeader(entry.Name, entry.Value); } void IHttpHeadersHandler.OnStaticIndexedHeader(int index, ReadOnlySpan value) { // TODO: https://github.com/dotnet/runtime/issues/1505 - Debug.Fail("Currently unused by HPACK, this should never be called."); + OnHeader(H2StaticTable.Get(index - 1).Name, value); } public void OnHeader(ReadOnlySpan name, ReadOnlySpan value) diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs index 3fd84d6..03f801e 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs @@ -177,12 +177,13 @@ namespace System.Net.Http.Unit.Tests.HPack public void OnStaticIndexedHeader(int index) { - throw new NotImplementedException(); + ref readonly HeaderField entry = ref H2StaticTable.Get(index - 1); + OnHeader(entry.Name, entry.Value); } public void OnStaticIndexedHeader(int index, ReadOnlySpan value) { - throw new NotImplementedException(); + OnHeader(H2StaticTable.Get(index - 1).Name, value); } } } -- 2.7.4