From 92f21dbda0d9ef4780385d408a4e5d0c92e06029 Mon Sep 17 00:00:00 2001
From: John Luo <johluo@microsoft.com>
Date: Thu, 7 Feb 2019 12:50:53 -0800
Subject: [PATCH] Unset reserved bit when setting 31 bit uint values (#7339)
* Unset reserved bit when setting 31 bit uint values
* Add FrameWriter tests
* Add overload
---
.../Core/src/Internal/Http2/Bitshifter.cs | 17 ++-
.../src/Internal/Http2/Http2FrameWriter.cs | 6 +-
.../Kestrel/Core/test/BitShifterTests.cs | 48 ++++++++
.../Core/test/Http2FrameWriterTests.cs | 108 ++++++++++++++++++
.../Http2/Http2TestBase.cs | 2 +-
5 files changed, 173 insertions(+), 8 deletions(-)
create mode 100644 src/Servers/Kestrel/Core/test/BitShifterTests.cs
create mode 100644 src/Servers/Kestrel/Core/test/Http2FrameWriterTests.cs
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Bitshifter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Bitshifter.cs
index 6dea8682911..8aa5f59e352 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http2/Bitshifter.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Bitshifter.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
@@ -36,11 +36,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
// Does not overwrite the highest order bit
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void WriteUInt31BigEndian(Span<byte> destination, uint value)
+ => WriteUInt31BigEndian(destination, value, true);
+
+ [MethodImpl(MethodImplOptions.AggressiveInlining)]
+ public static void WriteUInt31BigEndian(Span<byte> destination, uint value, bool preserveHighestBit)
{
Debug.Assert(value <= 0x7F_FF_FF_FF, value.ToString());
- // Keep the highest bit
- var reserved = (destination[0] & 0x80u) << 24;
- BinaryPrimitives.WriteUInt32BigEndian(destination, value | reserved);
+
+ if (preserveHighestBit)
+ {
+ // Keep the highest bit
+ value |= (destination[0] & 0x80u) << 24;
+ }
+
+ BinaryPrimitives.WriteUInt32BigEndian(destination, value);
}
}
}
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs
index 2f67445b272..73be2ed64ad 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs
@@ -402,7 +402,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
_outgoingFrame.PrepareWindowUpdate(streamId, sizeIncrement);
WriteHeaderUnsynchronized();
var buffer = _outputWriter.GetSpan(4);
- Bitshifter.WriteUInt31BigEndian(buffer, (uint)sizeIncrement);
+ Bitshifter.WriteUInt31BigEndian(buffer, (uint)sizeIncrement, preserveHighestBit: false);
_outputWriter.Advance(4);
return TimeFlushUnsynchronizedAsync();
}
@@ -538,7 +538,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
WriteHeaderUnsynchronized();
var buffer = _outputWriter.GetSpan(8);
- Bitshifter.WriteUInt31BigEndian(buffer, (uint)lastStreamId);
+ Bitshifter.WriteUInt31BigEndian(buffer, (uint)lastStreamId, preserveHighestBit: false);
buffer = buffer.Slice(4);
BinaryPrimitives.WriteUInt32BigEndian(buffer, (uint)errorCode);
_outputWriter.Advance(8);
@@ -578,7 +578,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
buffer[1] = frame.Flags;
buffer = buffer.Slice(2);
- Bitshifter.WriteUInt31BigEndian(buffer, (uint)frame.StreamId);
+ Bitshifter.WriteUInt31BigEndian(buffer, (uint)frame.StreamId, preserveHighestBit: false);
output.Advance(Http2FrameReader.HeaderLength);
}
diff --git a/src/Servers/Kestrel/Core/test/BitShifterTests.cs b/src/Servers/Kestrel/Core/test/BitShifterTests.cs
new file mode 100644
index 00000000000..5ba9b1747a9
--- /dev/null
+++ b/src/Servers/Kestrel/Core/test/BitShifterTests.cs
@@ -0,0 +1,48 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
+{
+ public class BitShifterTests
+ {
+ [Fact]
+ public void WriteUInt31BigEndian_PreservesHighestBit()
+ {
+ // Arrange
+ Span<byte> dirtySpan = new byte[] { 0xff, 0xff, 0xff, 0xff };
+
+ // Act
+ Bitshifter.WriteUInt31BigEndian(dirtySpan, 1);
+
+ Assert.Equal(new byte[] { 0x80, 0x00, 0x00, 0x01 }, dirtySpan.ToArray());
+ }
+
+ [Fact]
+ public void WriteUInt31BigEndian_True_PreservesHighestBit()
+ {
+ // Arrange
+ Span<byte> dirtySpan = new byte[] { 0xff, 0xff, 0xff, 0xff };
+
+ // Act
+ Bitshifter.WriteUInt31BigEndian(dirtySpan, 1, true);
+
+ Assert.Equal(new byte[] { 0x80, 0x00, 0x00, 0x01 }, dirtySpan.ToArray());
+ }
+
+ [Fact]
+ public void WriteUInt31BigEndian_False_OverwritesHighestBit()
+ {
+ // Arrange
+ Span<byte> dirtySpan = new byte[] { 0xff, 0xff, 0xff, 0xff };
+
+ // Act
+ Bitshifter.WriteUInt31BigEndian(dirtySpan, 1, false);
+
+ Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x01 }, dirtySpan.ToArray());
+ }
+ }
+}
diff --git a/src/Servers/Kestrel/Core/test/Http2FrameWriterTests.cs b/src/Servers/Kestrel/Core/test/Http2FrameWriterTests.cs
new file mode 100644
index 00000000000..dd5f993ea25
--- /dev/null
+++ b/src/Servers/Kestrel/Core/test/Http2FrameWriterTests.cs
@@ -0,0 +1,108 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Buffers;
+using System.IO.Pipelines;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
+using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
+using Moq;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
+{
+ public class Http2FrameWriterTests
+ {
+ private MemoryPool<byte> _dirtyMemoryPool;
+
+ public Http2FrameWriterTests()
+ {
+ var memoryBlock = new Mock<IMemoryOwner<byte>>();
+ memoryBlock.Setup(block => block.Memory).Returns(() =>
+ {
+ var blockArray = new byte[4096];
+ for (int i = 0; i < 4096; i++)
+ {
+ blockArray[i] = 0xff;
+ }
+ return new Memory<byte>(blockArray);
+ });
+
+ var dirtyMemoryPool = new Mock<MemoryPool<byte>>();
+ dirtyMemoryPool.Setup(pool => pool.Rent(It.IsAny<int>())).Returns(memoryBlock.Object);
+ _dirtyMemoryPool = dirtyMemoryPool.Object;
+ }
+
+ [Fact]
+ public async Task WriteWindowUpdate_UnsetsReservedBit()
+ {
+ // Arrange
+ var pipe = new Pipe(new PipeOptions(_dirtyMemoryPool, PipeScheduler.Inline, PipeScheduler.Inline));
+ var frameWriter = new Http2FrameWriter(pipe.Writer, null, null, null, null, null, null, new Mock<IKestrelTrace>().Object);
+
+ // Act
+ await frameWriter.WriteWindowUpdateAsync(1, 1);
+
+ // Assert
+ var payload = await pipe.Reader.ReadForLengthAsync(Http2FrameReader.HeaderLength + 4);
+
+ Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x01 }, payload.Skip(Http2FrameReader.HeaderLength).Take(4).ToArray());
+ }
+
+ [Fact]
+ public async Task WriteGoAway_UnsetsReservedBit()
+ {
+ // Arrange
+ var pipe = new Pipe(new PipeOptions(_dirtyMemoryPool, PipeScheduler.Inline, PipeScheduler.Inline));
+ var frameWriter = new Http2FrameWriter(pipe.Writer, null, null, null, null, null, null, new Mock<IKestrelTrace>().Object);
+
+ // Act
+ await frameWriter.WriteGoAwayAsync(1, Http2ErrorCode.NO_ERROR);
+
+ // Assert
+ var payload = await pipe.Reader.ReadForLengthAsync(Http2FrameReader.HeaderLength + 4);
+
+ Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x01 }, payload.Skip(Http2FrameReader.HeaderLength).Take(4).ToArray());
+ }
+
+ [Fact]
+ public async Task WriteHeader_UnsetsReservedBit()
+ {
+ // Arrange
+ var pipe = new Pipe(new PipeOptions(_dirtyMemoryPool, PipeScheduler.Inline, PipeScheduler.Inline));
+ var frame = new Http2Frame();
+ frame.PreparePing(Http2PingFrameFlags.NONE);
+
+ // Act
+ Http2FrameWriter.WriteHeader(frame, pipe.Writer);
+ await pipe.Writer.FlushAsync();
+
+ // Assert
+ var payload = await pipe.Reader.ReadForLengthAsync(Http2FrameReader.HeaderLength);
+
+ Assert.Equal(new byte[] { 0x00, 0x00, 0x00, 0x00 }, payload.Skip(5).Take(4).ToArray());
+ }
+ }
+
+ public static class PipeReaderExtensions
+ {
+ public static async Task<byte[]> ReadForLengthAsync(this PipeReader pipeReader, int length)
+ {
+ while (true)
+ {
+ var result = await pipeReader.ReadAsync();
+ var buffer = result.Buffer;
+
+ if (!buffer.IsEmpty && buffer.Length >= length)
+ {
+ return buffer.Slice(0, length).ToArray();
+ }
+
+ pipeReader.AdvanceTo(buffer.Start, buffer.End);
+ }
+ }
+ }
+}
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs
index 41a451eb765..05b54a8d12f 100644
--- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs
+++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs
@@ -1055,7 +1055,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
frame.PrepareWindowUpdate(streamId, sizeIncrement);
Http2FrameWriter.WriteHeader(frame, outputWriter);
var buffer = outputWriter.GetSpan(4);
- Bitshifter.WriteUInt31BigEndian(buffer, (uint)sizeIncrement);
+ BinaryPrimitives.WriteUInt32BigEndian(buffer, (uint)sizeIncrement);
outputWriter.Advance(4);
return FlushAsync(outputWriter);
}
--
GitLab