PR feedback

Author: shirhatti

src/Servers/HttpSys/src/DelegationRule.cs

@@ -28,7 +28,7 @@ internal DelegationRule(string queueName, string urlPrefix, ILogger logger)
             _logger = logger;
             QueueName = queueName;
             UrlPrefix = urlPrefix;
-            Queue = new RequestQueue(null, queueName, UrlPrefix, _logger, receiver: true);
+            Queue = new RequestQueue(queueName, UrlPrefix, _logger, receiver: true);
         }
 
         public void Dispose()

src/Servers/HttpSys/src/IHttpSysRequestTransferFeature.cs

@@ -11,8 +11,8 @@ public interface IHttpSysRequestTransferFeature
         bool IsTransferable { get; }
 
         /// <summary>
-        /// Attempt to transfer the request to another HttpSys request queue. The request body
-        /// must not be read before this is invoked. Check <see cref="IsTransferable"/>
+        /// Attempt to transfer the request to another Http.Sys request queue. The request body
+        /// must not be read nor the response started before this is invoked. Check <see cref="IsTransferable"/>
         /// before invoking.
         /// </summary>
         void TransferRequest(DelegationRule destination);

src/Servers/HttpSys/src/NativeInterop/HttpApi.cs

@@ -160,10 +160,7 @@ internal static bool IsFeatureSupported(HTTP_FEATURE_ID feature)
         {
             try
             {
-                if (HttpIsFeatureSupported(feature))
-                {
-                    return true;
-                }
+                return HttpIsFeatureSupported(feature);
             }
             catch (EntryPointNotFoundException) { }
 

src/Servers/HttpSys/src/NativeInterop/RequestQueue.cs

@@ -19,10 +19,18 @@ internal class RequestQueue
         private readonly ILogger _logger;
         private bool _disposed;
 
-        internal RequestQueue(UrlGroup urlGroup, string requestQueueName, string urlPrefix, ILogger logger, bool receiver)
-            : this(urlGroup, requestQueueName, mode: default, logger, receiver)
+        internal RequestQueue(string requestQueueName, string urlPrefix, ILogger logger, bool receiver)
+            : this(urlGroup: null, requestQueueName, RequestQueueMode.Attach, logger, receiver)
         {
-            UrlGroup = new UrlGroup(this, UrlPrefix.Create(urlPrefix));
+            try
+            {
+                UrlGroup = new UrlGroup(this, UrlPrefix.Create(urlPrefix));
+            }
+            catch
+            {
+                Dispose();
+                throw;
+            }
         }
 
         internal RequestQueue(UrlGroup urlGroup, string requestQueueName, RequestQueueMode mode, ILogger logger)
@@ -42,12 +50,10 @@ private RequestQueue(UrlGroup urlGroup, string requestQueueName, RequestQueueMod
             {
                 flags = HttpApiTypes.HTTP_CREATE_REQUEST_QUEUE_FLAG.OpenExisting;
                 Created = false;
-            }
-
-            if (receiver)
-            {
-                flags = HttpApiTypes.HTTP_CREATE_REQUEST_QUEUE_FLAG.OpenExisting | HttpApiTypes.HTTP_CREATE_REQUEST_QUEUE_FLAG.Delegation;
-                Created = false;
+                if (receiver)
+                {
+                    flags |= HttpApiTypes.HTTP_CREATE_REQUEST_QUEUE_FLAG.Delegation;
+                }
             }
 
             var statusCode = HttpApi.HttpCreateRequestQueue(

src/Servers/HttpSys/src/RequestProcessing/Request.cs

@@ -347,7 +347,7 @@ public X509Certificate2 ClientCertificate
             }
         }
 
-        public bool IsTransferable => (HasRequestBodyStarted || RequestContext.Response.HasStarted);
+        public bool IsTransferable => !(HasRequestBodyStarted || RequestContext.Response.HasStarted);
 
         // Populates the client certificate.  The result may be null if there is no client cert.
         // TODO: Does it make sense for this to be invoked multiple times (e.g. renegotiate)? Client and server code appear to

src/Servers/HttpSys/src/RequestProcessing/RequestContext.cs

@@ -352,7 +352,7 @@ internal unsafe void Transfer(DelegationRule destination)
                                                                destination.Queue.Handle,
                                                                Request.RequestId,
                                                                destination.Queue.UrlGroup.Id,
-                                                               1,
+                                                               propertyInfoSetSize: 1,
                                                                &property);
             }
 

src/Servers/HttpSys/src/ServerDelegationPropertyFeature.cs

@@ -21,9 +21,9 @@ public ServerDelegationPropertyFeature(RequestQueue queue, ILogger logger)
 
         public DelegationRule CreateDelegationRule(string queueName, string uri)
         {
-            var wrapper = new DelegationRule(queueName, uri, _logger);
-            _queue.UrlGroup.SetDelegationProperty(wrapper.Queue);
-            return wrapper;
+            var rule = new DelegationRule(queueName, uri, _logger);
+            _queue.UrlGroup.SetDelegationProperty(rule.Queue);
+            return rule;
         }
     }
 }

src/Servers/HttpSys/test/FunctionalTests/DelegateTests.cs

@@ -66,6 +66,7 @@ public async Task DelegateAfterWriteToResponseBodyShouldThrowTest()
             {
                 await httpContext.Response.WriteAsync(_expectedResponseString);
                 var transferFeature = httpContext.Features.Get<IHttpSysRequestTransferFeature>();
+                Assert.False(transferFeature.IsTransferable);
                 Assert.Throws<InvalidOperationException>(() => transferFeature.TransferRequest(wrapper));
             });
 
@@ -97,6 +98,7 @@ public async Task WriteToBodyAfterDelegateShouldThrowTest()
             {
                 var transferFeature = httpContext.Features.Get<IHttpSysRequestTransferFeature>();
                 transferFeature.TransferRequest(wrapper);
+                Assert.False(transferFeature.IsTransferable);
                 await Assert.ThrowsAsync<InvalidOperationException>(async () =>
                 {
                     await httpContext.Response.WriteAsync(_expectedResponseString);
@@ -146,21 +148,11 @@ public async Task DelegateAfterRequestBodyReadShouldThrow()
         [DelegateSupportedCondition(false)]
         public async Task DelegationFeaturesAreNull()
         {
-            var queueName = Guid.NewGuid().ToString();
-            using var receiver = Utilities.CreateHttpServer(out var receiverAddress, async httpContext =>
-            {
-                await httpContext.Response.WriteAsync(_expectedResponseString);
-            },
-           options =>
-           {
-               options.RequestQueueName = queueName;
-           });
-
-            using var delegator = Utilities.CreateHttpServer(out var delegatorAddress, async httpContext =>
+            using var delegator = Utilities.CreateHttpServer(out var delegatorAddress, httpContext =>
             {
                 var transferFeature = httpContext.Features.Get<IHttpSysRequestTransferFeature>();
                 Assert.Null(transferFeature);
-                await httpContext.Response.WriteAsync(_expectedResponseString);
+                return Task.CompletedTask;
             });
 
             var delegationProperty = delegator.Features.Get<IServerDelegationFeature>();

src/Shared/HttpSys/NativeInterop/HttpApiTypes.cs

@@ -32,7 +32,6 @@ internal enum HTTP_SERVER_PROPERTY
             HttpServerListenEndpointProperty,
             HttpServerChannelBindProperty,
             HttpServerProtectionLevelProperty,
-            // Internal
             HttpServerDelegationProperty = 16
         }