Release: merge develop into beta (v7.0.5-beta)

src/Titanium.Web.Proxy/Http2/Http2Helper.Copy.cs

@@ -46,7 +46,7 @@
             "transfer-encoding", "content-length", "host", "trailer"
         };
 
         private static async Task CopyHttp2FrameAsync(Stream input, Stream output, // NOSONAR S3776, CA1068 -- Protocol flow and established token position are retained.
             Http2ConnectionState connectionState,
             Func<SessionEventArgs> sessionFactory,
             Func<SessionEventArgs, Http2StreamContext, Task> onBeforeRequestResponse,
@@ -1293,10 +1293,13 @@
 
                             var outBytes = bodyWriteArgs.BodyBytes ?? Array.Empty<byte>();
 
-                            // 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;

src/Titanium.Web.Proxy/Http2/Http2Helper.Send.cs

@@ -48,7 +48,7 @@
         ///     DATA frames for the same direction must also go through <see cref="Http2ConnectionState.EnqueueWriteRented"/>
         ///     so they cannot overtake this HEADERS on the wire.
         /// </summary>
         private static void QueueSendHeader(Http2ConnectionState connectionState, bool towardServer, // NOSONAR S107 -- Parameters kept explicit to avoid allocating options bags on hot bridge/pool paths.
             SemaphoreSlim writeLock, Http2Settings settings, Http2FrameHeader frameHeader,
             byte[] frameHeaderBuffer, RequestResponseBase rr, bool endStream, Stream output, bool pushPromise)
         {
@@ -64,21 +64,20 @@
             }
         }
 
         private static void QueueSendHeaderTowardServer(Http2ConnectionState connectionState, // NOSONAR S107 -- Parameters kept explicit to avoid allocating options bags on hot bridge/pool paths.
             SemaphoreSlim serverWriteLock, Http2Settings settings, Http2FrameHeader frameHeader,
             byte[] frameHeaderBuffer, RequestResponseBase rr, bool endStream, Stream output, bool pushPromise) =>
             QueueSendHeader(connectionState, towardServer: true, serverWriteLock, settings, frameHeader,
                 frameHeaderBuffer, rr, endStream, output, pushPromise);
 
         /// <summary>
-        ///     Frames <paramref name="payload"/> 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 <paramref name="payload"/>. Used by the synthetic/bridge
-        ///     response paths so responses from many concurrent streams coalesce into few socket writes
-        ///     instead of each taking <see cref="Http2ConnectionState.ClientWriteLock"/> per frame.
+        ///     Frames <paramref name="payload"/> as one DATA frame into a rented buffer and queues it on the
+        ///     same dedicated writer FIFO as <see cref="QueueSendHeader"/> so DATA cannot overtake HEADERS
+        ///     on that direction. The caller must already hold the flow-control reservation for
+        ///     <paramref name="payload"/> when the payload is non-empty.
         /// </summary>
-        private static void QueueDataFrame(Http2ConnectionState connectionState, Stream clientStream,
-            int streamId, ReadOnlyMemory<byte> payload, bool endStream)
+        private static void QueueDataFrame(Http2ConnectionState connectionState, bool towardServer,
+            SemaphoreSlim writeLock, Stream output, int streamId, ReadOnlyMemory<byte> payload, bool endStream)
         {
             var total = 9 + payload.Length;
             var rented = ArrayPool<byte>.Shared.Rent(total);
@@ -91,8 +90,51 @@
             };
             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);
+        }
+
+        /// <summary>
+        ///     Frames <paramref name="payload"/> 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 <paramref name="payload"/>. Used by the synthetic/bridge
+        ///     response paths so responses from many concurrent streams coalesce into few socket writes
+        ///     instead of each taking <see cref="Http2ConnectionState.ClientWriteLock"/> per frame.
+        /// </summary>
+        private static void QueueDataFrame(Http2ConnectionState connectionState, Stream clientStream,
+            int streamId, ReadOnlyMemory<byte> payload, bool endStream) =>
+            QueueDataFrame(connectionState, towardServer: false, connectionState.ClientWriteLock,
+                clientStream, streamId, payload, endStream);
+
+        /// <summary>
+        ///     Same framing and flow-control reservation as <see cref="SendData"/>, but the frames are
+        ///     queued on the dedicated writer FIFO used by <see cref="QueueSendHeader"/>. The per-chunk
+        ///     <c>OnRequestBodyWrite</c>/<c>OnResponseBodyWrite</c> path previously called <see cref="SendData"/>
+        ///     (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 (<c>ERR_HTTP2_PROTOCOL_ERROR</c>).
+        /// </summary>
+        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<byte> 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<byte>.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;
+            }
         }
 
         /// <summary>
@@ -176,13 +218,13 @@
         /// <summary>
         ///     Builds HEADERS/CONTINUATION wire bytes into an ArrayPool buffer (caller owns the rent).
         /// </summary>
         private static ArraySegment<byte> RentFramedHeaderBlock(Http2FrameHeader frameHeader, // NOSONAR S107 -- Frame fields stay explicit.
             byte[] frameHeaderBuffer, int streamId, Http2FrameType type, bool endStream, bool hasPriority,
             ReadOnlyMemory<byte> data, int maxFrameSize) =>
             RentFramedHeaderBlock(frameHeader, frameHeaderBuffer, streamId, type, endStream, hasPriority, data,
                 ReadOnlyMemory<byte>.Empty, maxFrameSize);
 
         private static ArraySegment<byte> RentFramedHeaderBlock(Http2FrameHeader frameHeader, // NOSONAR S107, S1172 -- Frame fields stay explicit; frameHeaderBuffer retained for call-site IL match.
             byte[] frameHeaderBuffer, // NOSONAR S1172 -- retained for call-site IL match.
             int streamId, Http2FrameType type, bool endStream, bool hasPriority,
             ReadOnlyMemory<byte> data, ReadOnlyMemory<byte> append, int maxFrameSize)
@@ -261,7 +303,7 @@
         ///     on the first, matching the semantics of the frame types they belong to. HEADERS/CONTINUATION
         ///     frames are not subject to flow control (RFC 7540 ?6.9), so no reservation is made here.
         /// </summary>
         private static async Task WriteHeaderBlockAsync(Http2FrameHeader frameHeader, byte[] frameHeaderBuffer, // NOSONAR S107 -- Frame fields are kept explicit in this low-level encoder helper.
             int streamId, Http2FrameType type, bool endStream, bool hasPriority, ReadOnlyMemory<byte> data,
             int maxFrameSize, Stream output)
         {
@@ -302,7 +344,7 @@
             } while (pos < data.Length);
         }
 
         internal static async Task SendBody(Http2Settings settings, RequestResponseBase rr, Http2FrameHeader frameHeader, // NOSONAR S107 -- Frame-writing state is kept explicit for this low-level helper.
             byte[] frameHeaderBuffer, byte[] buffer, Http2FlowController flow, Stream output,
             CancellationToken cancellationToken)
         {
@@ -906,7 +948,7 @@
         ///     Bounded by <paramref name="length" /> and a short timeout so a peer that declares a huge length and
         ///     then stalls cannot use this to hang the relay.
         /// </summary>
         private static async Task DiscardRejectedFramePayloadAsync(Stream input, int length, // NOSONAR S1144 -- reflection test seam
             CancellationToken cancellationToken)
         {
             try
@@ -1143,7 +1185,7 @@
         /// </summary>
         private sealed class NoOpHeaderListener : IHeaderListener
         {
             public static readonly NoOpHeaderListener Instance = new(); // NOSONAR S1144 -- reserved singleton for compressed-relay decode
 
             public void AddHeader(ByteString name, ByteString value, bool sensitive)
             {

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;
+
+/// <summary>
+///     Regression for Chrome <c>ERR_HTTP2_PROTOCOL_ERROR</c> (RST_STREAM error code 1) when Inspector
+///     (or any MITM subscriber) attaches <c>OnResponseBodyWrite</c>/<c>OnRequestBodyWrite</c> 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.
+/// </summary>
+[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<byte>());
+        });
+
+        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<bool>(
+            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;
+    }
+}