From 19700d07b16d3a3af72a0b90916e3a1408a78aeb Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 10 Sep 2024 09:01:30 +0100 Subject: [PATCH] Quote ETag if not quoted in HttpHeaders#setETag This aligns HttpHeaders with other places like ServletWebRequest and DefaultWebExchange where an ETag is accepted as input. It also allows us to remove quoting from places that delegate to HttpHeaders#setETag since it now does that internally. Closes gh-33412 --- .../java/org/springframework/http/HttpHeaders.java | 8 +++----- .../org/springframework/http/ResponseEntity.java | 5 ++--- .../web/server/adapter/DefaultServerWebExchange.java | 6 +++--- .../org/springframework/http/HttpHeadersTests.java | 12 ++++++------ .../server/DefaultEntityResponseBuilder.java | 5 ++--- .../server/DefaultServerResponseBuilder.java | 6 ++---- .../reactive/resource/VersionResourceResolver.java | 5 ++--- .../function/DefaultEntityResponseBuilder.java | 5 ++--- .../function/DefaultServerResponseBuilder.java | 6 ++---- 9 files changed, 24 insertions(+), 34 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java index 7ce36aad2e..0c428ff240 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java +++ b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java @@ -1063,11 +1063,9 @@ public class HttpHeaders implements MultiValueMap, Serializable /** * Set the (new) entity tag of the body, as specified by the {@code ETag} header. */ - public void setETag(@Nullable String eTag) { - if (eTag != null) { - Assert.isTrue(eTag.startsWith("\"") || eTag.startsWith("W/\""), "ETag does not start with W/\" or \""); - Assert.isTrue(eTag.endsWith("\""), "ETag does not end with \""); - set(ETAG, eTag); + public void setETag(@Nullable String tag) { + if (tag != null) { + set(ETAG, ETag.quoteETagIfNecessary(tag)); } else { remove(ETAG); diff --git a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java b/spring-web/src/main/java/org/springframework/http/ResponseEntity.java index ea62bc565a..2a55dfcbb7 100644 --- a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java +++ b/spring-web/src/main/java/org/springframework/http/ResponseEntity.java @@ -568,9 +568,8 @@ public class ResponseEntity extends HttpEntity { } @Override - public BodyBuilder eTag(@Nullable String eTag) { - eTag = (eTag != null ? ETag.quoteETagIfNecessary(eTag) : eTag); - this.headers.setETag(eTag); + public BodyBuilder eTag(@Nullable String tag) { + this.headers.setETag(tag); return this; } diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java b/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java index 09c833ce47..49f64e6223 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java @@ -417,13 +417,13 @@ public class DefaultServerWebExchange implements ServerWebExchange { addCachingResponseHeaders(eTag, lastModified); } - private void addCachingResponseHeaders(@Nullable String eTag, Instant lastModified) { + private void addCachingResponseHeaders(@Nullable String tag, Instant lastModified) { if (SAFE_METHODS.contains(getRequest().getMethod())) { if (lastModified.isAfter(Instant.EPOCH) && getResponseHeaders().getLastModified() == -1) { getResponseHeaders().setLastModified(lastModified.toEpochMilli()); } - if (StringUtils.hasLength(eTag) && getResponseHeaders().getETag() == null) { - getResponseHeaders().setETag(ETag.quoteETagIfNecessary(eTag)); + if (StringUtils.hasLength(tag) && getResponseHeaders().getETag() == null) { + getResponseHeaders().setETag(tag); } } } diff --git a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java index c0057254bc..7767b4ea41 100644 --- a/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java +++ b/spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java @@ -214,15 +214,15 @@ class HttpHeadersTests { } @Test - void illegalETagWithoutQuotes() { - String eTag = "v2.6"; - assertThatIllegalArgumentException().isThrownBy(() -> headers.setETag(eTag)); + void eTagWithoutQuotes() { + headers.setETag("v2.6"); + assertThat(headers.getETag()).isEqualTo("\"v2.6\""); } @Test - void illegalWeakETagWithoutLeadingQuote() { - String etag = "W/v2.6\""; - assertThatIllegalArgumentException().isThrownBy(() -> headers.setETag(etag)); + void weakETagWithoutLeadingQuote() { + headers.setETag("W/v2.6\""); + assertThat(headers.getETag()).isEqualTo("\"W/v2.6\"\""); } @Test diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilder.java index 75c0126085..d31d0ff8a9 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilder.java @@ -148,9 +148,8 @@ class DefaultEntityResponseBuilder implements EntityResponse.Builder { } @Override - public EntityResponse.Builder eTag(String eTag) { - eTag = ETag.quoteETagIfNecessary(eTag); - this.headers.setETag(eTag); + public EntityResponse.Builder eTag(String tag) { + this.headers.setETag(tag); return this; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java index 00443c9d48..1b2f372b7c 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java @@ -147,10 +147,8 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { } @Override - public ServerResponse.BodyBuilder eTag(String eTag) { - Assert.notNull(eTag, "etag must not be null"); - eTag = ETag.quoteETagIfNecessary(eTag); - this.headers.setETag(eTag); + public ServerResponse.BodyBuilder eTag(String tag) { + this.headers.setETag(tag); return this; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java index 674d6d2567..941308e137 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -332,8 +332,7 @@ public class VersionResourceResolver extends AbstractResourceResolver { @Override public HttpHeaders getResponseHeaders() { - HttpHeaders headers = (this.original instanceof HttpResource httpResource ? - httpResource.getResponseHeaders() : new HttpHeaders()); + HttpHeaders headers = (this.original instanceof HttpResource hr ? hr.getResponseHeaders() : new HttpHeaders()); headers.setETag("W/\"" + this.version + "\""); return headers; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultEntityResponseBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultEntityResponseBuilder.java index bed228e264..d4947e0746 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultEntityResponseBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultEntityResponseBuilder.java @@ -166,9 +166,8 @@ final class DefaultEntityResponseBuilder implements EntityResponse.Builder } @Override - public EntityResponse.Builder eTag(String eTag) { - eTag = ETag.quoteETagIfNecessary(eTag); - this.headers.setETag(eTag); + public EntityResponse.Builder eTag(String tag) { + this.headers.setETag(tag); return this; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java index 46ea10a80f..a6117d4caf 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java @@ -127,10 +127,8 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { } @Override - public ServerResponse.BodyBuilder eTag(String eTag) { - Assert.notNull(eTag, "etag must not be null"); - eTag = ETag.quoteETagIfNecessary(eTag); - this.headers.setETag(eTag); + public ServerResponse.BodyBuilder eTag(String tag) { + this.headers.setETag(tag); return this; }