diff --git a/src/ServiceControl.AcceptanceTests/WebApi/When_a_request_is_repeated_with_its_etag.cs b/src/ServiceControl.AcceptanceTests/WebApi/When_a_request_is_repeated_with_its_etag.cs new file mode 100644 index 0000000000..dbc86721c0 --- /dev/null +++ b/src/ServiceControl.AcceptanceTests/WebApi/When_a_request_is_repeated_with_its_etag.cs @@ -0,0 +1,95 @@ +namespace ServiceControl.AcceptanceTests.WebApi +{ + using System.Net; + using System.Net.Http; + using System.Threading.Tasks; + using AcceptanceTesting; + using NServiceBus.AcceptanceTesting; + using NUnit.Framework; + using Recoverability.MessageRedirects; + + class When_a_request_is_repeated_with_its_etag : AcceptanceTest + { + [TestCase("/api/customchecks", "GET", false)] + [TestCase("/api/redirects", "GET", true)] + [TestCase("/api/redirect", "HEAD", true)] + public async Task Should_answer_not_modified(string url, string method, bool seedARedirect) + { + Answer issued = null; + Answer repeated = null; + + await Define() + .Done(async ctx => + { + if (seedARedirect) + { + await this.Post("/api/redirects", new RedirectRequest + { + fromphysicaladdress = "endpointA@machine1", + tophysicaladdress = "endpointB@machine2" + }, status => status is not HttpStatusCode.Created); + } + + // Internal custom checks re-report on a timer, so the validator can move between + // the two requests. + for (var attempt = 0; attempt < 5; attempt++) + { + issued = await Ask(method, url, ifNoneMatch: null); + + if (issued.Etag == null) + { + continue; + } + + repeated = await Ask(method, url, issued.Etag); + + if (repeated.Status == HttpStatusCode.NotModified) + { + break; + } + } + + return true; + }) + .Run(); + + Assert.That(issued.Etag, Is.Not.Null, $"{method} {url} issued no ETag, so there is nothing for a client to revalidate against"); + Assert.That(repeated.Status, Is.EqualTo(HttpStatusCode.NotModified), $"{method} {url} sent the full payload again for a client that already held {issued.Etag}"); + + // ServicePulse drives pagination from Total-Count, and a revalidating client takes it from + // the 304 rather than from the body it already holds. + Assert.That(repeated.TotalCount, Is.Not.Null.And.EqualTo(issued.TotalCount), $"{method} {url} did not carry its Total-Count through to the 304"); + } + + async Task Ask(string method, string url, string ifNoneMatch) + { + using var response = await Send(method, url, ifNoneMatch); + + return new Answer( + response.StatusCode, + Header(response, "ETag"), + Header(response, "Total-Count")); + } + + static string Header(HttpResponseMessage response, string name) => + response.Headers.TryGetValues(name, out var values) ? string.Join(string.Empty, values) : null; + + Task Send(string method, string url, string ifNoneMatch) + { + var request = new HttpRequestMessage(new HttpMethod(method), url); + + if (ifNoneMatch != null) + { + // Unvalidated, so a malformed validator reaches the server and shows up as a 200 + // rather than throwing here. + request.Headers.TryAddWithoutValidation("If-None-Match", ifNoneMatch); + } + + return HttpClient.SendAsync(request); + } + + record Answer(HttpStatusCode Status, string Etag, string TotalCount); + + class Context : ScenarioContext; + } +} diff --git a/src/ServiceControl.Audit.AcceptanceTests/WebApi/When_a_message_body_is_requested_twice.cs b/src/ServiceControl.Audit.AcceptanceTests/WebApi/When_a_message_body_is_requested_twice.cs new file mode 100644 index 0000000000..50fb1e4bfd --- /dev/null +++ b/src/ServiceControl.Audit.AcceptanceTests/WebApi/When_a_message_body_is_requested_twice.cs @@ -0,0 +1,92 @@ +namespace ServiceControl.Audit.AcceptanceTests.WebApi +{ + using System.Net; + using System.Net.Http; + using System.Threading.Tasks; + using AcceptanceTesting; + using AcceptanceTesting.EndpointTemplates; + using Audit.Auditing.MessagesView; + using NServiceBus; + using NServiceBus.AcceptanceTesting; + using NServiceBus.Settings; + using NUnit.Framework; + + class When_a_message_body_is_requested_twice : AcceptanceTest + { + [Test] + public async Task Should_answer_not_modified() + { + string issued = null; + HttpStatusCode? repeated = null; + + await Define() + .WithEndpoint(b => b.When(bus => bus.SendLocal(new MyMessage { Payload = "PAYLOAD" }))) + .Done(async c => + { + if (c.MessageId == null) + { + return false; + } + + MessagesView audited = await this.TryGetSingle("/api/messages?include_system_messages=false&sort=id", m => m.MessageId == c.MessageId); + + if (audited == null) + { + return false; + } + + var url = $"/api{audited.BodyUrl}"; + + using var first = await this.GetRaw(url); + + if (!first.Headers.TryGetValues("ETag", out var values)) + { + return false; + } + + issued = string.Join(string.Empty, values); + + var request = new HttpRequestMessage(HttpMethod.Get, url); + // Unvalidated, so a malformed validator reaches the server and shows up as a 200. + request.Headers.TryAddWithoutValidation("If-None-Match", issued); + + using var second = await HttpClient.SendAsync(request); + repeated = second.StatusCode; + + return true; + }) + .Run(); + + Assert.That(issued, Is.Not.Null, "the body response carried no validator, so a client can never revalidate it"); + Assert.That(repeated, Is.EqualTo(HttpStatusCode.NotModified), $"the body was sent again to a client that already held {issued}"); + } + + public class Receiver : EndpointConfigurationBuilder + { + public Receiver() => EndpointSetup(); + + [Handler] + public class MyMessageHandler(MyContext testContext, IReadOnlySettings settings) : IHandleMessages + { + public Task Handle(MyMessage message, IMessageHandlerContext context) + { + testContext.EndpointNameOfReceivingEndpoint = settings.EndpointName(); + testContext.MessageId = context.MessageId; + return Task.CompletedTask; + } + } + } + + public class MyMessage : ICommand + { + public string Payload { get; set; } + } + + public class MyContext : ScenarioContext + { + public string MessageId { get; set; } + + public string EndpointNameOfReceivingEndpoint { get; set; } + } + } +} diff --git a/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAttachmentsBodyStorage.cs b/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAttachmentsBodyStorage.cs index d2193c57fe..2408cb4bf6 100644 --- a/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAttachmentsBodyStorage.cs +++ b/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAttachmentsBodyStorage.cs @@ -1,6 +1,5 @@ namespace ServiceControl.Audit.Persistence.InMemory { - using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -61,7 +60,8 @@ public async Task TryFetch(string bodyId, CancellationToken cancel Stream = new MemoryStream(messageBody.Content), ContentType = messageBody.ContentType, BodySize = messageBody.BodySize, - Etag = Guid.NewGuid().ToString() + // Bodies are immutable per message, so the id is a stable validator + Etag = bodyId }); } diff --git a/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAuditDataStore.cs b/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAuditDataStore.cs index f06b0681b7..d147f5afca 100644 --- a/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAuditDataStore.cs +++ b/src/ServiceControl.Audit.Persistence.InMemory/InMemoryAuditDataStore.cs @@ -168,7 +168,7 @@ Task GetMessageBodyFromMetadata(string messageId) return Task.FromResult(MessageBodyView.NotFound()); } - return Task.FromResult(MessageBodyView.FromString(body, contentType, bodySize, string.Empty)); + return Task.FromResult(MessageBodyView.FromString(body, contentType, bodySize, messageId)); } public Task>> QueryAuditCounts(string endpointName, CancellationToken cancellationToken) diff --git a/src/ServiceControl.Audit.Persistence.Tests/AuditTests.cs b/src/ServiceControl.Audit.Persistence.Tests/AuditTests.cs index 89e2249fac..0aa2983656 100644 --- a/src/ServiceControl.Audit.Persistence.Tests/AuditTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests/AuditTests.cs @@ -115,6 +115,28 @@ public async Task Can_roundtrip_message_body() } } + [Test] + public async Task Message_body_validator_is_stable_across_reads() + { + var unitOfWork = await StartAuditUnitOfWork(1); + + var body = new byte[100]; + Random.Shared.NextBytes(body); + var processedMessage = MakeMessage(); + + await unitOfWork.RecordProcessedMessage(processedMessage, body); + + await unitOfWork.DisposeAsync(); + + var bodyId = GetBodyId(processedMessage); + + var first = await DataStore.GetMessageBody(bodyId, TestContext.CurrentContext.CancellationToken); + var second = await DataStore.GetMessageBody(bodyId, TestContext.CurrentContext.CancellationToken); + + Assert.That(first.ETag, Is.Not.Null.And.Not.Empty, "a body with no validator cannot be revalidated, so conditional GET is dead on it"); + Assert.That(second.ETag, Is.EqualTo(first.ETag), "the body did not change, so neither may its validator"); + } + [Test] public async Task Does_respect_max_message_body() { diff --git a/src/ServiceControl.Audit.UnitTests/Infrastructure/WebApi/ConditionalGetTests.cs b/src/ServiceControl.Audit.UnitTests/Infrastructure/WebApi/ConditionalGetTests.cs new file mode 100644 index 0000000000..621bfd9426 --- /dev/null +++ b/src/ServiceControl.Audit.UnitTests/Infrastructure/WebApi/ConditionalGetTests.cs @@ -0,0 +1,97 @@ +namespace ServiceControl.Audit.UnitTests.Infrastructure.WebApi +{ + using System.Net; + using Audit.Infrastructure.WebApi; + using Microsoft.AspNetCore.Http; + using Microsoft.AspNetCore.Mvc; + using Microsoft.AspNetCore.Mvc.Abstractions; + using Microsoft.AspNetCore.Mvc.Filters; + using Microsoft.AspNetCore.Routing; + using NUnit.Framework; + + [TestFixture] + public class ConditionalGetTests + { + [Test] + public void Repeating_a_request_with_the_etag_just_issued_is_not_modified() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + httpContext.Request.Headers.IfNoneMatch = httpContext.Response.Headers.ETag; + + var context = ResultExecuting(httpContext); + + new NotModifiedStatusHttpHandler().OnResultExecuting(context); + + Assert.That(context.Result, Is.InstanceOf(), + "the client already holds this version, so the response must be a 304 rather than the full payload"); + Assert.That(((StatusCodeResult)context.Result).StatusCode, Is.EqualTo((int)HttpStatusCode.NotModified)); + } + + [Test] + public void A_different_etag_still_returns_the_payload() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + httpContext.Request.Headers.IfNoneMatch = "\"something-else\""; + + var context = ResultExecuting(httpContext); + + new NotModifiedStatusHttpHandler().OnResultExecuting(context); + + Assert.That(context.Result, Is.InstanceOf()); + } + + [Test] + public void The_emitted_etag_is_a_well_formed_entity_tag() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + + Assert.That(httpContext.Response.GetTypedHeaders().ETag, Is.Not.Null, + "an ETag that cannot be parsed as an entity-tag disables conditional GET without any error"); + } + + [Test] + public void A_deterministic_etag_is_a_well_formed_entity_tag() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithDeterministicEtag("any-non-empty-payload-signature"); + + Assert.That(httpContext.Response.GetTypedHeaders().ETag, Is.Not.Null); + } + + [Test] + public void The_emitted_etag_quotes_the_value_without_altering_it() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + + Assert.That(httpContext.Response.Headers.ETag.ToString(), Is.EqualTo("\"4611686018427387904\"")); + } + + [TestCase(null)] + [TestCase("")] + public void A_call_site_with_nothing_to_validate_emits_no_etag_header(string value) + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag(value); + + Assert.That(httpContext.Response.Headers.ContainsKey("ETag"), Is.False, + "an empty entity-tag is well formed, so it would match itself and answer 304 for unrelated payloads"); + } + + static ResultExecutingContext ResultExecuting(HttpContext httpContext) => + new( + new ActionContext(httpContext, new RouteData(), new ActionDescriptor()), + [], + new OkObjectResult(new object()), + controller: null); + } +} diff --git a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs index e88d1fbb6f..14357f0b7f 100644 --- a/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs +++ b/src/ServiceControl.Audit/Auditing/MessagesView/GetMessagesController.cs @@ -68,7 +68,7 @@ public async Task Get(string id, CancellationToken cancellationTo throw new Exception($"Metadata for message '{id}' indicated that a body was present but no content could be found in storage"); } - Response.Headers.ETag = result.ETag; + Response.WithEtag(result.ETag); var contentType = result.ContentType ?? "text/*"; return result.StringContent != null ? Content(result.StringContent, contentType) : File(result.StreamContent, contentType); } diff --git a/src/ServiceControl.Audit/Infrastructure/WebApi/HttpResponseExtensions.cs b/src/ServiceControl.Audit/Infrastructure/WebApi/HttpResponseExtensions.cs index f4af3b765d..532273710a 100644 --- a/src/ServiceControl.Audit/Infrastructure/WebApi/HttpResponseExtensions.cs +++ b/src/ServiceControl.Audit/Infrastructure/WebApi/HttpResponseExtensions.cs @@ -14,7 +14,19 @@ static class HttpResponseExtensions { public static void WithTotalCount(this HttpResponse response, long totalCount) => response.WithHeader("Total-Count", totalCount.ToString(CultureInfo.InvariantCulture)); - public static void WithEtag(this HttpResponse response, StringValues value) => response.Headers.ETag = value; + public static void WithEtag(this HttpResponse response, StringValues value) + { + var validator = value.ToString(); + + if (string.IsNullOrEmpty(validator)) + { + return; + } + + // RFC 9110 requires an entity-tag to be a quoted string. Unquoted, EntityTagHeaderValue + // cannot parse it and NotModifiedStatusHttpHandler never matches a client's If-None-Match. + response.Headers.ETag = $"\"{validator}\""; + } public static void WithQueryStatsInfo(this HttpResponse response, QueryStatsInfo queryStatsInfo) { diff --git a/src/ServiceControl.MultiInstance.AcceptanceTests/Auditing/When_requesting_a_message_body.cs b/src/ServiceControl.MultiInstance.AcceptanceTests/Auditing/When_requesting_a_message_body.cs index cfe75262c2..7188e391a5 100644 --- a/src/ServiceControl.MultiInstance.AcceptanceTests/Auditing/When_requesting_a_message_body.cs +++ b/src/ServiceControl.MultiInstance.AcceptanceTests/Auditing/When_requesting_a_message_body.cs @@ -72,7 +72,7 @@ public async Task Should_be_forwarded_to_audit_instance(CancellationToken cancel using (Assert.EnterMultipleScope()) { Assert.That(body, Is.EqualTo(context.MessageBody), "Body bytes mismatch"); - Assert.That(response.Headers.GetValues("ETag").SingleOrDefault(), Is.Not.Null, "Etag not set"); + Assert.That(response.Headers.ETag, Is.Not.Null, "Etag not set, or not a well formed entity-tag"); } } diff --git a/src/ServiceControl.UnitTests/Infrastructure/WebApi/ConditionalGetTests.cs b/src/ServiceControl.UnitTests/Infrastructure/WebApi/ConditionalGetTests.cs new file mode 100644 index 0000000000..8a2c1e945a --- /dev/null +++ b/src/ServiceControl.UnitTests/Infrastructure/WebApi/ConditionalGetTests.cs @@ -0,0 +1,100 @@ +namespace ServiceControl.UnitTests.Infrastructure.WebApi; + +using System.Collections.Generic; +using System.Net; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using NUnit.Framework; +using ServiceControl.Infrastructure.WebApi; + +[TestFixture] +public class ConditionalGetTests +{ + [Test] + public void Repeating_a_request_with_the_etag_just_issued_is_not_modified() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + + httpContext.Request.Headers.IfNoneMatch = httpContext.Response.Headers.ETag; + + var context = ResultExecuting(httpContext); + + new NotModifiedStatusHttpHandler().OnResultExecuting(context); + + Assert.That(context.Result, Is.InstanceOf(), + "the client already holds this version, so the response must be a 304 rather than the full payload"); + Assert.That(((StatusCodeResult)context.Result).StatusCode, Is.EqualTo((int)HttpStatusCode.NotModified)); + } + + [Test] + public void A_different_etag_still_returns_the_payload() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + httpContext.Request.Headers.IfNoneMatch = "\"something-else\""; + + var context = ResultExecuting(httpContext); + + new NotModifiedStatusHttpHandler().OnResultExecuting(context); + + Assert.That(context.Result, Is.InstanceOf()); + } + + [Test] + public void The_emitted_etag_is_a_well_formed_entity_tag() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + + // RFC 9110 requires an entity-tag to be a quoted string. GetTypedHeaders parses through + // EntityTagHeaderValue and yields null for anything else. + Assert.That(httpContext.Response.GetTypedHeaders().ETag, Is.Not.Null, + "an ETag that cannot be parsed as an entity-tag disables conditional GET without any error"); + } + + [Test] + public void A_deterministic_etag_is_a_well_formed_entity_tag() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithDeterministicEtag("any-non-empty-payload-signature"); + + Assert.That(httpContext.Response.GetTypedHeaders().ETag, Is.Not.Null); + } + + [Test] + public void The_emitted_etag_quotes_the_value_without_altering_it() + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag("4611686018427387904"); + + Assert.That(httpContext.Response.Headers.ETag.ToString(), Is.EqualTo("\"4611686018427387904\"")); + } + + [TestCase(null)] + [TestCase("")] + public void A_call_site_with_nothing_to_validate_emits_no_etag_header(string value) + { + var httpContext = new DefaultHttpContext(); + + httpContext.Response.WithEtag(value); + + Assert.That(httpContext.Response.Headers.ContainsKey("ETag"), Is.False, + "an empty entity-tag is well formed, so it would match itself and answer 304 for unrelated payloads"); + } + + static ResultExecutingContext ResultExecuting(HttpContext httpContext) => + new( + new ActionContext(httpContext, new RouteData(), new ActionDescriptor()), + [], + new OkObjectResult(new object()), + controller: null); +} diff --git a/src/ServiceControl.UnitTests/ScatterGather/RemoteInstanceEtagTests.cs b/src/ServiceControl.UnitTests/ScatterGather/RemoteInstanceEtagTests.cs new file mode 100644 index 0000000000..e66b2e551e --- /dev/null +++ b/src/ServiceControl.UnitTests/ScatterGather/RemoteInstanceEtagTests.cs @@ -0,0 +1,29 @@ +namespace ServiceControl.UnitTests.ScatterGather +{ + using System.Net.Http; + using CompositeViews.Messages; + using NUnit.Framework; + + [TestFixture] + public class RemoteInstanceEtagTests + { + [TestCase("\"4611686018427387904\"", TestName = "A_remote_etag_is_read_when_the_instance_quotes_it")] + [TestCase("4611686018427387904", TestName = "A_remote_etag_is_read_when_the_instance_predates_the_conditional_get_fix")] + public void A_remote_etag_is_read(string asSentByTheRemoteInstance) + { + var response = new HttpResponseMessage(); + response.Headers.TryAddWithoutValidation("ETag", asSentByTheRemoteInstance); + + Assert.That(ScatterGatherApiBase.ReadEtag(response.Headers), Is.EqualTo("4611686018427387904"), + "a rolling upgrade runs both shapes side by side, so both have to be understood"); + } + + [Test] + public void An_instance_that_sends_no_etag_contributes_nothing() + { + var response = new HttpResponseMessage(); + + Assert.That(ScatterGatherApiBase.ReadEtag(response.Headers), Is.Null); + } + } +} diff --git a/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs b/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs index d586fe34cd..5ee7542b6d 100644 --- a/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs +++ b/src/ServiceControl/CompositeViews/Messages/GetMessagesController.cs @@ -99,7 +99,7 @@ public async Task Get(string id, [FromQuery(Name = "instance_id") return NoContent(); } - Response.Headers.ETag = result.Etag; + Response.WithEtag(result.Etag); return File(result.Stream, result.ContentType ?? "text/*"); } diff --git a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs index c77921ae57..9058cb9be6 100644 --- a/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs +++ b/src/ServiceControl/CompositeViews/Messages/ScatterGatherApi.cs @@ -5,6 +5,7 @@ namespace ServiceControl.CompositeViews.Messages using System.Linq; using System.Net; using System.Net.Http; + using System.Net.Http.Headers; using System.Threading.Tasks; using Infrastructure.WebApi; using Microsoft.AspNetCore.Http; @@ -15,8 +16,23 @@ namespace ServiceControl.CompositeViews.Messages interface IApi; - // used to hoist the static jsonSerializer field across the generic instances - public abstract class ScatterGatherApiBase; + // Non-generic, so statics live once rather than once per closed generic instantiation. + public abstract class ScatterGatherApiBase + { + internal static string ReadEtag(HttpResponseHeaders headers) + { + // Read raw rather than through headers.ETag. An instance predating the quoted validator + // sends a bare token, which EntityTagHeaderValue fails to parse and discards silently. + if (!headers.TryGetValues("ETag", out var values)) + { + return null; + } + + var etag = values.FirstOrDefault(); + + return etag?.Length > 1 && etag[0] == '"' && etag[^1] == '"' ? etag[1..^1] : etag; + } + } public record ScatterGatherContext(PagingInfo PagingInfo); @@ -169,16 +185,9 @@ static async Task> ParseResult(HttpResponseMessage responseMes totalCount = int.Parse(totalCounts.ElementAt(0)); } - string etag = responseMessage.Headers.ETag?.Tag; - if (etag != null) - { - // Strip quotes from Etag, checking for " which isn't really needed as Etag always has quotes but not 100% certain. - // Later the value is joined into a new Etag when the results are aggregated and returned - if (etag.StartsWith("\"")) - { - etag = etag.Substring(1, etag.Length - 2); - } - } + // Unquoted, because AggregateStats concatenates it with the other instances' values and + // the result is re-tagged before it goes back on the wire. + var etag = ReadEtag(responseMessage.Headers); return new QueryResult(remoteResults, new QueryStatsInfo(etag, totalCount, isStale: false)); } diff --git a/src/ServiceControl/Infrastructure/WebApi/HttpResponseExtensions.cs b/src/ServiceControl/Infrastructure/WebApi/HttpResponseExtensions.cs index 6d33e4394e..226148016a 100644 --- a/src/ServiceControl/Infrastructure/WebApi/HttpResponseExtensions.cs +++ b/src/ServiceControl/Infrastructure/WebApi/HttpResponseExtensions.cs @@ -13,7 +13,19 @@ static class HttpResponseExtensions { public static void WithTotalCount(this HttpResponse response, long totalCount) => response.WithHeader("Total-Count", totalCount.ToString(CultureInfo.InvariantCulture)); - public static void WithEtag(this HttpResponse response, StringValues value) => response.Headers.ETag = value; + public static void WithEtag(this HttpResponse response, StringValues value) + { + var validator = value.ToString(); + + if (string.IsNullOrEmpty(validator)) + { + return; + } + + // RFC 9110 requires an entity-tag to be a quoted string. Unquoted, EntityTagHeaderValue + // cannot parse it and NotModifiedStatusHttpHandler never matches a client's If-None-Match. + response.Headers.ETag = $"\"{validator}\""; + } public static void WithQueryStatsInfo(this HttpResponse response, QueryStatsInfo queryStatsInfo) {