From 1b26122e649781a40e2c6c609d69a67b81149af3 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 9 Sep 2024 18:19:59 +0100 Subject: [PATCH] Polishing and minor refactoring Update checks whether quoting is needed to be more complete than what we've used so far, making sure the there is both opening and closing quotes independent of each other. See gh-33412 --- .../java/org/springframework/http/ETag.java | 23 ++++++++++++++----- .../springframework/http/ResponseEntity.java | 10 ++++---- .../server/DefaultEntityResponseBuilder.java | 17 ++++++++++---- .../server/DefaultServerResponseBuilder.java | 17 ++++++++++---- .../DefaultEntityResponseBuilder.java | 18 +++++++++++---- .../DefaultServerResponseBuilder.java | 17 +++++++++----- 6 files changed, 69 insertions(+), 33 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/ETag.java b/spring-web/src/main/java/org/springframework/http/ETag.java index be2ded9891..0acfaefc04 100644 --- a/spring-web/src/main/java/org/springframework/http/ETag.java +++ b/spring-web/src/main/java/org/springframework/http/ETag.java @@ -134,16 +134,27 @@ public record ETag(String tag, boolean weak) { return result; } - public static String format(String etag) { - if (!etag.startsWith("\"") && !etag.startsWith("W/\"")) { - etag = "\"" + etag; + /** + * Add quotes around the ETag value if not present already. + * @param tag the ETag value + * @return the resulting, quoted value + * @since 6.2 + */ + public static String quoteETagIfNecessary(String tag) { + if (tag.startsWith("W/\"")) { + if (tag.length() > 3 && tag.endsWith("\"")) { + return tag; + } } - if (!etag.endsWith("\"")) { - etag = etag + "\""; + else if (tag.startsWith("\"")) { + if (tag.length() > 1 && tag.endsWith("\"")) { + return tag; + } } - return etag; + return ("\"" + tag + "\""); } + private enum State { BEFORE_QUOTES, IN_QUOTES, AFTER_QUOTES 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 8ab005e293..ea62bc565a 100644 --- a/spring-web/src/main/java/org/springframework/http/ResponseEntity.java +++ b/spring-web/src/main/java/org/springframework/http/ResponseEntity.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. @@ -568,11 +568,9 @@ public class ResponseEntity extends HttpEntity { } @Override - public BodyBuilder eTag(@Nullable String etag) { - if (etag != null) { - etag = ETag.format(etag); - } - this.headers.setETag(etag); + public BodyBuilder eTag(@Nullable String eTag) { + eTag = (eTag != null ? ETag.quoteETagIfNecessary(eTag) : eTag); + this.headers.setETag(eTag); return this; } 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 7722c53eb1..75c0126085 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 @@ -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. @@ -28,10 +28,17 @@ import java.util.Optional; import java.util.Set; import java.util.function.Consumer; -import org.springframework.http.*; import reactor.core.publisher.Mono; import org.springframework.core.codec.Hints; +import org.springframework.http.CacheControl; +import org.springframework.http.ETag; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseCookie; import org.springframework.http.codec.HttpMessageWriter; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; @@ -141,9 +148,9 @@ class DefaultEntityResponseBuilder implements EntityResponse.Builder { } @Override - public EntityResponse.Builder eTag(String etag) { - etag = ETag.format(etag); - this.headers.setETag(etag); + public EntityResponse.Builder eTag(String eTag) { + eTag = ETag.quoteETagIfNecessary(eTag); + this.headers.setETag(eTag); 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 7c6e2ee990..00443c9d48 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 @@ -32,11 +32,18 @@ import java.util.function.Consumer; import java.util.function.Function; import org.reactivestreams.Publisher; -import org.springframework.http.*; import reactor.core.publisher.Mono; import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.codec.Hints; +import org.springframework.http.CacheControl; +import org.springframework.http.ETag; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.MediaType; +import org.springframework.http.ReactiveHttpOutputMessage; +import org.springframework.http.ResponseCookie; import org.springframework.http.codec.HttpMessageWriter; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; @@ -140,10 +147,10 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { } @Override - public ServerResponse.BodyBuilder eTag(String etag) { - Assert.notNull(etag, "etag must not be null"); - etag = ETag.format(etag); - this.headers.setETag(etag); + public ServerResponse.BodyBuilder eTag(String eTag) { + Assert.notNull(eTag, "etag must not be null"); + eTag = ETag.quoteETagIfNecessary(eTag); + this.headers.setETag(eTag); return this; } 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 e44cd3131a..bed228e264 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 @@ -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. @@ -45,7 +45,15 @@ import org.springframework.core.ResolvableType; import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourceRegion; -import org.springframework.http.*; +import org.springframework.http.CacheControl; +import org.springframework.http.ETag; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRange; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.InvalidMediaTypeException; +import org.springframework.http.MediaType; import org.springframework.http.converter.GenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.SmartHttpMessageConverter; @@ -158,9 +166,9 @@ final class DefaultEntityResponseBuilder implements EntityResponse.Builder } @Override - public EntityResponse.Builder eTag(String etag) { - etag = ETag.format(etag); - this.headers.setETag(etag); + public EntityResponse.Builder eTag(String eTag) { + eTag = ETag.quoteETagIfNecessary(eTag); + this.headers.setETag(eTag); 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 d5611d19c5..46ea10a80f 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 @@ -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. @@ -30,7 +30,12 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.springframework.core.ParameterizedTypeReference; -import org.springframework.http.*; +import org.springframework.http.CacheControl; +import org.springframework.http.ETag; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -122,10 +127,10 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { } @Override - public ServerResponse.BodyBuilder eTag(String etag) { - Assert.notNull(etag, "etag must not be null"); - etag = ETag.format(etag); - this.headers.setETag(etag); + public ServerResponse.BodyBuilder eTag(String eTag) { + Assert.notNull(eTag, "etag must not be null"); + eTag = ETag.quoteETagIfNecessary(eTag); + this.headers.setETag(eTag); return this; }