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
spring-projects · Sep 10, 2024 · 1b26122 · 1b26122
1 parent 80b264b
commit 1b26122
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 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
@@ -134,16 +134,27 @@ public static List<ETag> parse(String source) {
 		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
@@ -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 BodyBuilder contentType(MediaType contentType) {
 		}
 
 		@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/...n/java/org/springframework/web/reactive/function/server/DefaultEntityResponseBuilder.java b/...n/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.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 @@ public EntityResponse.Builder<T> contentType(MediaType contentType) {
 	}
 
 	@Override
-	public EntityResponse.Builder<T> eTag(String etag) {
-		etag = ETag.format(etag);
-		this.headers.setETag(etag);
+	public EntityResponse.Builder<T> eTag(String eTag) {
+		eTag = ETag.quoteETagIfNecessary(eTag);
+		this.headers.setETag(eTag);
 		return this;
 	}
 

diff --git a/...n/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java b/...n/java/org/springframework/web/reactive/function/server/DefaultServerResponseBuilder.java
@@ -32,11 +32,18 @@
 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 @@ public ServerResponse.BodyBuilder contentType(MediaType contentType) {
 	}
 
 	@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/.../src/main/java/org/springframework/web/servlet/function/DefaultEntityResponseBuilder.java b/.../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.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 @@ public EntityResponse.Builder<T> contentType(MediaType contentType) {
 	}
 
 	@Override
-	public EntityResponse.Builder<T> eTag(String etag) {
-		etag = ETag.format(etag);
-		this.headers.setETag(etag);
+	public EntityResponse.Builder<T> eTag(String eTag) {
+		eTag = ETag.quoteETagIfNecessary(eTag);
+		this.headers.setETag(eTag);
 		return this;
 	}
 

diff --git a/.../src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java b/.../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.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 @@ public ServerResponse.BodyBuilder contentType(MediaType contentType) {
 	}
 
 	@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;
 	}