From e05004465c15b16e56e07aa465489507f9b8c41d Mon Sep 17 00:00:00 2001 From: justcoding121 Date: Thu, 10 Sep 2026 12:46:26 -0500 Subject: [PATCH] Fix Chrome HTTP/2 PROTOCOL_ERROR when MITM body-write hooks emit DATA before HEADERS. Inspector always subscribes OnResponseBodyWrite; a direct SendData write could overtake queued HEADERS, which Chrome treats as DATA on an idle stream. --- .../Http2/Http2Helper.Copy.cs | 11 +- .../Http2/Http2Helper.Send.cs | 60 ++++- .../Http2BodyWriteHookFrameOrderTests.cs | 224 ++++++++++++++++++ 3 files changed, 282 insertions(+), 13 deletions(-) create mode 100644 tests/Titanium.Web.Proxy.IntegrationTests/Http2BodyWriteHookFrameOrderTests.cs diff --git a/src/Titanium.Web.Proxy/Http2/Http2Helper.Copy.cs b/src/Titanium.Web.Proxy/Http2/Http2Helper.Copy.cs index 46942a70..b459f678 100644 --- a/src/Titanium.Web.Proxy/Http2/Http2Helper.Copy.cs +++ b/src/Titanium.Web.Proxy/Http2/Http2Helper.Copy.cs @@ -1293,10 +1293,13 @@ await lockedOwnLegWrite(() => SendRstStreamAsync( var outBytes = bodyWriteArgs.BodyBytes ?? Array.Empty(); - // Reserve outside outputWriteLock — same ordering as the default DATA relay above. - await SendData(frameHeader, frameHeaderBuffer, streamId, outBytes, - endStreamFlag, remoteSettings.MaxFrameSize, outboundFlow, output, cancellationToken, - outputWriteLock); + // Queue on the same FIFO as QueueSendHeader. A direct SendData write can + // overtake MITM-re-encoded HEADERS still sitting on ClientFrameWriter / + // ServerFrameWriter (Inspector always subscribes OnResponseBodyWrite), + // which Chrome treats as DATA on an idle stream (PROTOCOL_ERROR). + await QueueSendData(connectionState, towardServer: isClient, outputWriteLock, + streamId, outBytes, endStreamFlag, remoteSettings.MaxFrameSize, outboundFlow, + output, cancellationToken); // we have emitted our own (possibly re-sized) DATA frame(s); suppress the default relay sendPacket = false; diff --git a/src/Titanium.Web.Proxy/Http2/Http2Helper.Send.cs b/src/Titanium.Web.Proxy/Http2/Http2Helper.Send.cs index 68e6f2dc..4b9e95bc 100644 --- a/src/Titanium.Web.Proxy/Http2/Http2Helper.Send.cs +++ b/src/Titanium.Web.Proxy/Http2/Http2Helper.Send.cs @@ -71,14 +71,13 @@ private static void QueueSendHeaderTowardServer(Http2ConnectionState connectionS frameHeaderBuffer, rr, endStream, output, pushPromise); /// - /// Frames as one client-bound DATA frame into a rented buffer and - /// queues it on the dedicated client frame writer. The caller must already hold the - /// flow-control reservation for . Used by the synthetic/bridge - /// response paths so responses from many concurrent streams coalesce into few socket writes - /// instead of each taking per frame. + /// Frames as one DATA frame into a rented buffer and queues it on the + /// same dedicated writer FIFO as so DATA cannot overtake HEADERS + /// on that direction. The caller must already hold the flow-control reservation for + /// when the payload is non-empty. /// - private static void QueueDataFrame(Http2ConnectionState connectionState, Stream clientStream, - int streamId, ReadOnlyMemory payload, bool endStream) + private static void QueueDataFrame(Http2ConnectionState connectionState, bool towardServer, + SemaphoreSlim writeLock, Stream output, int streamId, ReadOnlyMemory payload, bool endStream) { var total = 9 + payload.Length; var rented = ArrayPool.Shared.Rent(total); @@ -91,8 +90,51 @@ private static void QueueDataFrame(Http2ConnectionState connectionState, Stream }; dataFrameHeader.CopyToBuffer(rented); payload.Span.CopyTo(rented.AsSpan(9)); - connectionState.EnqueueWriteRented(towardServer: false, connectionState.ClientWriteLock, - clientStream, rented, total); + connectionState.EnqueueWriteRented(towardServer, writeLock, output, rented, total); + } + + /// + /// Frames as one client-bound DATA frame into a rented buffer and + /// queues it on the dedicated client frame writer. The caller must already hold the + /// flow-control reservation for . Used by the synthetic/bridge + /// response paths so responses from many concurrent streams coalesce into few socket writes + /// instead of each taking per frame. + /// + private static void QueueDataFrame(Http2ConnectionState connectionState, Stream clientStream, + int streamId, ReadOnlyMemory payload, bool endStream) => + QueueDataFrame(connectionState, towardServer: false, connectionState.ClientWriteLock, + clientStream, streamId, payload, endStream); + + /// + /// Same framing and flow-control reservation as , but the frames are + /// queued on the dedicated writer FIFO used by . The per-chunk + /// OnRequestBodyWrite/OnResponseBodyWrite path previously called + /// (direct locked write). That raced the MITM HEADERS enqueue: DATA could hit the peer socket + /// first, which Chrome treats as DATA on an idle stream (ERR_HTTP2_PROTOCOL_ERROR). + /// + private static async ValueTask QueueSendData(Http2ConnectionState connectionState, bool towardServer, // NOSONAR S107 -- Frame-writing state is kept explicit for this low-level helper. + SemaphoreSlim writeLock, int streamId, ReadOnlyMemory data, bool endStream, int maxFrameSize, + Http2FlowController flow, Stream output, CancellationToken cancellationToken) + { + if (maxFrameSize <= 0) maxFrameSize = 16384; + + if (data.Length == 0) + { + QueueDataFrame(connectionState, towardServer, writeLock, output, streamId, + ReadOnlyMemory.Empty, endStream); + return; + } + + var pos = 0; + while (pos < data.Length) + { + var frameLength = Math.Min(maxFrameSize, data.Length - pos); + var isLastFrame = pos + frameLength >= data.Length; + await flow.ReserveAsync(streamId, frameLength, cancellationToken).ConfigureAwait(false); + QueueDataFrame(connectionState, towardServer, writeLock, output, streamId, + data.Slice(pos, frameLength), isLastFrame && endStream); + pos += frameLength; + } } /// diff --git a/tests/Titanium.Web.Proxy.IntegrationTests/Http2BodyWriteHookFrameOrderTests.cs b/tests/Titanium.Web.Proxy.IntegrationTests/Http2BodyWriteHookFrameOrderTests.cs new file mode 100644 index 00000000..c2e8704e --- /dev/null +++ b/tests/Titanium.Web.Proxy.IntegrationTests/Http2BodyWriteHookFrameOrderTests.cs @@ -0,0 +1,224 @@ +using System; +using System.IO; +using System.Security.Cryptography.X509Certificates; +using System.Text; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Titanium.Web.Proxy; +using Titanium.Web.Proxy.Http2; +using Titanium.Web.Proxy.IntegrationTests.Helpers; +using Titanium.Web.Proxy.IntegrationTests.Setup; + +namespace Titanium.Web.Proxy.IntegrationTests; + +/// +/// Regression for Chrome ERR_HTTP2_PROTOCOL_ERROR (RST_STREAM error code 1) when Inspector +/// (or any MITM subscriber) attaches OnResponseBodyWrite/OnRequestBodyWrite without +/// buffering the whole body. MITM re-encodes HEADERS onto the dedicated frame-writer FIFO while the +/// body-write hook used to emit DATA with a direct locked socket write. DATA could overtake HEADERS +/// on the wire — RFC 7540 DATA on an idle stream is PROTOCOL_ERROR. Release builds made the race +/// easier to hit because the frame loop admits HEADERS+DATA back-to-back before the writer drain +/// runs. Leftover root CAs from a previous install produce certificate errors, not this symptom. +/// +[TestClass] +public class Http2BodyWriteHookFrameOrderTests +{ + private static X509Certificate2 CreateOriginCertificate() + { + return TestCertificateAuthority.ServerCertificate; + } + + [TestMethod] + [Timeout(30 * 1000)] + public async Task Http2_Mitm_OnResponseBodyWrite_DoesNot_Send_Data_Before_Headers() + { + using var rawServer = new Http2RawOriginServer(CreateOriginCertificate()); + rawServer.HandleConnection(async connection => + { + await connection.SendInitialSettingsAsync(); + var (streamId, _, _) = await connection.ReadRequestAsync(); + + var responseHeaders = connection.EncodeHeaders( + new[] { (":status", "200") }, + new[] { ("content-type", "text/plain") }); + await connection.WriteHeaderBlockAsync(streamId, responseHeaders, endStream: false); + await connection.WriteFrameAsync(Http2FrameType.Data, streamId, Http2FrameFlag.EndStream, + Encoding.ASCII.GetBytes("ok")); + }); + + using var testSuite = new TestSuite(); + var proxy = SubscribeInspectorLikeHooks(testSuite.GetProxy()); + + var uri = new Uri(rawServer.Url); + using var rawClient = await Http2RawClient.ConnectAsync(proxy.ProxyEndPoints[0].Port, uri.Host, uri.Port); + await SendGetAsync(rawClient, uri, 1); + + var (firstStreamFrame, body) = await ReadStreamFramesUntilEndAsync(rawClient, 1); + Assert.AreEqual(Http2FrameType.Headers, firstStreamFrame.Type, + "DATA must not precede HEADERS on the client socket (idle-stream PROTOCOL_ERROR)."); + Assert.AreEqual("ok", Encoding.ASCII.GetString(body)); + } + + [TestMethod] + [Timeout(30 * 1000)] + public async Task Http2_Mitm_OnResponseBodyWrite_204_EmptyData_DoesNot_Precede_Headers() + { + // Google ads/collect often answers 204. Inspector does not call GetResponseBody (HasBody is + // false) but still has OnResponseBodyWrite subscribed, so an origin that sends HEADERS then + // an empty DATA END_STREAM used to be able to emit DATA first. + using var rawServer = new Http2RawOriginServer(CreateOriginCertificate()); + rawServer.HandleConnection(async connection => + { + await connection.SendInitialSettingsAsync(); + var (streamId, _, _) = await connection.ReadRequestAsync(); + + var responseHeaders = connection.EncodeHeaders( + new[] { (":status", "204") }, + Array.Empty<(string, string)>()); + await connection.WriteHeaderBlockAsync(streamId, responseHeaders, endStream: false); + await connection.WriteFrameAsync(Http2FrameType.Data, streamId, Http2FrameFlag.EndStream, + Array.Empty()); + }); + + using var testSuite = new TestSuite(); + var proxy = SubscribeInspectorLikeHooks(testSuite.GetProxy()); + + var uri = new Uri(rawServer.Url); + using var rawClient = await Http2RawClient.ConnectAsync(proxy.ProxyEndPoints[0].Port, uri.Host, uri.Port); + await SendGetAsync(rawClient, uri, 1); + + var (firstStreamFrame, _) = await ReadStreamFramesUntilEndAsync(rawClient, 1); + Assert.AreEqual(Http2FrameType.Headers, firstStreamFrame.Type, + "Empty DATA for a 204 must not precede HEADERS (Chrome ERR_HTTP2_PROTOCOL_ERROR)."); + } + + [TestMethod] + [Timeout(30 * 1000)] + public async Task Http2_Mitm_OnRequestBodyWrite_DoesNot_Send_Data_Before_Headers() + { + var originSawHeadersFirst = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + + using var rawServer = new Http2RawOriginServer(CreateOriginCertificate()); + rawServer.HandleConnection(async connection => + { + await connection.SendInitialSettingsAsync(); + + Http2FrameType? firstStreamFrame = null; + int streamId = -1; + while (true) + { + var frame = await connection.ReadFrameAsync(); + if (frame.StreamId == 0) + continue; + + firstStreamFrame ??= frame.Type; + if (streamId < 0 && frame.Type == Http2FrameType.Headers) + streamId = frame.StreamId; + + if (frame.Type is Http2FrameType.Data or Http2FrameType.Headers + && (frame.Flags & Http2FrameFlag.EndStream) != 0) + break; + } + + originSawHeadersFirst.TrySetResult(firstStreamFrame == Http2FrameType.Headers); + + if (streamId > 0) + { + var responseHeaders = connection.EncodeHeaders( + new[] { (":status", "204") }, + Array.Empty<(string, string)>()); + await connection.WriteHeaderBlockAsync(streamId, responseHeaders, endStream: true); + } + }); + + using var testSuite = new TestSuite(); + var proxy = SubscribeInspectorLikeHooks(testSuite.GetProxy()); + + var uri = new Uri(rawServer.Url); + using var rawClient = await Http2RawClient.ConnectAsync(proxy.ProxyEndPoints[0].Port, uri.Host, uri.Port); + var requestHeaders = rawClient.Connection.EncodeHeaders( + new[] + { + (":method", "POST"), (":scheme", "https"), (":authority", $"{uri.Host}:{uri.Port}"), + (":path", "/") + }, + new[] { ("content-type", "text/plain") }); + await rawClient.Connection.WriteHeaderBlockAsync(1, requestHeaders, endStream: false); + await rawClient.Connection.WriteFrameAsync(Http2FrameType.Data, 1, Http2FrameFlag.EndStream, + Encoding.ASCII.GetBytes("body")); + + var completed = await Task.WhenAny(originSawHeadersFirst.Task, Task.Delay(5000)); + Assert.AreSame(originSawHeadersFirst.Task, completed, "Origin never finished reading the POST."); + Assert.IsTrue(await originSawHeadersFirst.Task, + "Request DATA must not precede HEADERS toward the origin (idle-stream PROTOCOL_ERROR)."); + } + + private static ProxyServer SubscribeInspectorLikeHooks(ProxyServer proxy) + { + proxy.EnableHttp2 = true; + proxy.EnableHttpInterception = true; + // Inspector always attaches these, including when the throttle profile is None. + proxy.BeforeRequest += (_, _) => Task.CompletedTask; + proxy.BeforeResponse += (_, _) => Task.CompletedTask; + proxy.OnRequestBodyWrite += (_, _) => Task.CompletedTask; + proxy.OnResponseBodyWrite += (_, _) => Task.CompletedTask; + return proxy; + } + + private static Task SendGetAsync(Http2RawClient rawClient, Uri uri, int streamId) + { + var requestHeaders = rawClient.Connection.EncodeHeaders( + new[] + { + (":method", "GET"), (":scheme", "https"), (":authority", $"{uri.Host}:{uri.Port}"), + (":path", "/") + }, + Array.Empty<(string, string)>()); + return rawClient.Connection.WriteHeaderBlockAsync(streamId, requestHeaders, true); + } + + private static async Task<(Http2RawFrame.Frame FirstStreamFrame, byte[] Body)> ReadStreamFramesUntilEndAsync( + Http2RawClient rawClient, int streamId) + { + Http2RawFrame.Frame? first = null; + var body = new MemoryStream(); + for (var i = 0; i < 64; i++) + { + var frame = await rawClient.Connection.ReadFrameAsync(); + if (frame.Type == Http2FrameType.RstStream && frame.StreamId == streamId) + { + var code = (frame.Payload[0] << 24) | (frame.Payload[1] << 16) | + (frame.Payload[2] << 8) | (frame.Payload[3]); + Assert.Fail($"Stream {streamId} was reset with {(Http2ErrorCode)code} before completing."); + } + + if (frame.Type == Http2FrameType.GoAway) + { + var code = (frame.Payload[4] << 24) | (frame.Payload[5] << 16) | + (frame.Payload[6] << 8) | (frame.Payload[7]); + Assert.Fail($"Connection GOAWAY with {(Http2ErrorCode)code} before stream {streamId} completed."); + } + + if (frame.StreamId != streamId) + continue; + if (frame.Type is Http2FrameType.WindowUpdate or Http2FrameType.Priority) + continue; + + first ??= frame; + if (frame.Type == Http2FrameType.Data && first.Value.Type != Http2FrameType.Headers) + { + Assert.Fail("DATA arrived on the stream before HEADERS (idle-stream PROTOCOL_ERROR)."); + } + + if (frame.Type == Http2FrameType.Data) + body.Write(frame.Payload, 0, frame.Payload.Length); + + if ((frame.Flags & Http2FrameFlag.EndStream) != 0) + return (first.Value, body.ToArray()); + } + + Assert.Fail($"Stream {streamId} never received END_STREAM."); + return default; + } +}