Skip to content

Separate out ETag validation and parsing logic #33370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions spring-web/src/main/java/org/springframework/http/ETag.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.http;

import org.springframework.util.Assert;

/**
* ETag header value holder.
*
* @author Riley Park
* @since TODO
* @param value value that uniquely represents the resource
* @param weak if weak validation should be used
* @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag">ETag Header</a>
*/
public record ETag(
String value,
boolean weak
) {

/**
* ETag prefix.
*
* @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag#directives">ETag Header Directives</a>
*/
public static final String PREFIX = "\"";

/**
* ETag prefix, with a weak validator.
*
* @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag#directives">ETag Header Directives</a>
* @see <a href=https://developer.mozilla.org/en-US/docs/Web/HTTP/Conditional_requests#weak_validation">Weak Validation</a>
*/
public static final String PREFIX_WEAK = "W/\"";

/**
* ETag suffix.
*
* @see <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag#directives">ETag Header Directives</a>
*/
public static final String SUFFIX = "\"";

/**
* Parses an {@code ETag} header value as defined in RFC 7232.
* @param etag the {@literal ETag} header value
* @return the parsed content disposition
* @see #toString()
*/
public static ETag parse(String etag) {
boolean weak = etag.startsWith(PREFIX_WEAK);
Assert.isTrue(etag.startsWith(PREFIX) || weak,
"Invalid ETag: does not start with " + PREFIX + " or " + PREFIX_WEAK);
Assert.isTrue(etag.endsWith(SUFFIX), "Invalid ETag: does not end with " + SUFFIX);
int start = (weak ? PREFIX_WEAK.length() : PREFIX.length());
String value = etag.substring(start, etag.length() - SUFFIX.length());
return new ETag(value, weak);
}

public String toHeaderValue() {
return (weak ? PREFIX_WEAK : PREFIX) + value + SUFFIX;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1072,11 +1072,16 @@ public long getDate() {
* Set the (new) entity tag of the body, as specified by the {@code ETag} header.
*/
public void setETag(@Nullable String etag) {
setETag((etag != null) ? ETag.parse(etag) : null);
}

/**
* Set the (new) entity tag of the body, as specified by the {@code ETag} header.
* @since TODO
*/
public void setETag(@Nullable ETag etag) {
if (etag != null) {
Assert.isTrue(etag.startsWith("\"") || etag.startsWith("W/"),
"Invalid ETag: does not start with W/ or \"");
Assert.isTrue(etag.endsWith("\""), "Invalid ETag: does not end with \"");
set(ETAG, etag);
set(ETAG, etag.toHeaderValue());
}
else {
remove(ETAG);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,15 @@ public interface HeadersBuilder<B extends HeadersBuilder<B>> {
*/
B eTag(@Nullable String etag);

/**
* Set the entity tag of the body, as specified by the {@code ETag} header.
* @since TODO
* @param etag the new entity tag
* @return this builder
* @see HttpHeaders#setETag(ETag)
*/
B eTag(@Nullable ETag etag);

/**
* Set the time the resource was last changed, as specified by the
* {@code Last-Modified} header.
Expand Down Expand Up @@ -569,14 +578,12 @@ public BodyBuilder contentType(MediaType contentType) {

@Override
public BodyBuilder eTag(@Nullable String etag) {
if (etag != null) {
if (!etag.startsWith("\"") && !etag.startsWith("W/\"")) {
etag = "\"" + etag;
}
if (!etag.endsWith("\"")) {
etag = etag + "\"";
}
}
this.headers.setETag(etag);
return this;
}

@Override
public BodyBuilder eTag(@Nullable ETag etag) {
this.headers.setETag(etag);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.springframework.context.i18n.LocaleContext;
import org.springframework.core.ResolvableType;
import org.springframework.core.codec.Hints;
import org.springframework.http.ETag;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -368,10 +369,10 @@ private String padEtagIfNecessary(@Nullable String etag) {
if (!StringUtils.hasLength(etag)) {
return etag;
}
if ((etag.startsWith("\"") || etag.startsWith("W/\"")) && etag.endsWith("\"")) {
if ((etag.startsWith(ETag.PREFIX) || etag.startsWith(ETag.PREFIX_WEAK)) && etag.endsWith(ETag.SUFFIX)) {
return etag;
}
return "\"" + etag + "\"";
return ETag.PREFIX + etag + ETag.SUFFIX;
}

private boolean eTagStrongMatch(@Nullable String first, @Nullable String second) {
Expand Down
54 changes: 54 additions & 0 deletions spring-web/src/test/java/org/springframework/http/ETagTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.http;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;

/**
* Tests for {@link ETag}.
* @author Riley Park
*/
class ETagTests {

@Test
void parse() {
String raw = "\"v2.6\"";
ETag etag = ETag.parse(raw);
assertThat(etag.value()).isEqualTo("v2.6");
assertThat(etag.weak()).isFalse();
assertThat(etag.toHeaderValue()).isEqualTo(raw);
}

@Test
void parseWeak() {
String raw = "W/\"v2.6\"";
ETag etag = ETag.parse(raw);
assertThat(etag.value()).isEqualTo("v2.6");
assertThat(etag.weak()).isTrue();
assertThat(etag.toHeaderValue()).isEqualTo(raw);
}

@Test
void illegalETagWithoutQuoteAfterWSlash() {
String raw = "W/v2.6\"";
assertThatIllegalArgumentException().isThrownBy(() -> ETag.parse(raw));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,14 @@ void eTag() {
assertThat(headers.getFirst("ETag")).as("Invalid ETag header").isEqualTo("\"v2.6\"");
}

@Test
void eTagW() {
String eTag = "W\"v2.6\"";
headers.setETag(eTag);
assertThat(headers.getETag()).as("Invalid ETag header").isEqualTo(eTag);
assertThat(headers.getFirst("ETag")).as("Invalid ETag header").isEqualTo("W\"v2.6\"");
}

@Test
void host() {
InetSocketAddress host = InetSocketAddress.createUnresolved("localhost", 8080);
Expand Down Expand Up @@ -219,6 +227,12 @@ void illegalETag() {
assertThatIllegalArgumentException().isThrownBy(() -> headers.setETag(eTag));
}

@Test
void illegalETagWithoutQuoteAfterWSlash() {
String eTag = "W/v2.6\"";
assertThatIllegalArgumentException().isThrownBy(() -> headers.setETag(eTag));
}

@Test
void ifMatch() {
String ifMatch = "\"v2.6\"";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ void Etagheader() {
responseEntity = ResponseEntity.ok().eTag("W/\"foo\"").build();
assertThat(responseEntity.getHeaders().getETag()).isEqualTo("W/\"foo\"");

responseEntity = ResponseEntity.ok().eTag(null).build();
responseEntity = ResponseEntity.ok().eTag((String) null).build();
assertThat(responseEntity.getHeaders().getETag()).isNull();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

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;
Expand Down Expand Up @@ -148,12 +149,12 @@ public EntityResponse.Builder<T> contentType(MediaType contentType) {

@Override
public EntityResponse.Builder<T> eTag(String etag) {
if (!etag.startsWith("\"") && !etag.startsWith("W/\"")) {
etag = "\"" + etag;
}
if (!etag.endsWith("\"")) {
etag = etag + "\"";
}
this.headers.setETag(etag);
return this;
}

@Override
public EntityResponse.Builder<T> eTag(ETag etag) {
this.headers.setETag(etag);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -148,12 +149,13 @@ public ServerResponse.BodyBuilder contentType(MediaType contentType) {
@Override
public ServerResponse.BodyBuilder eTag(String etag) {
Assert.notNull(etag, "etag must not be null");
if (!etag.startsWith("\"") && !etag.startsWith("W/\"")) {
etag = "\"" + etag;
}
if (!etag.endsWith("\"")) {
etag = etag + "\"";
}
this.headers.setETag(etag);
return this;
}

@Override
public ServerResponse.BodyBuilder eTag(ETag etag) {
Assert.notNull(etag, "etag must not be null");
this.headers.setETag(etag);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import org.springframework.core.ParameterizedTypeReference;
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;
Expand Down Expand Up @@ -220,6 +221,15 @@ interface Builder<T> {
*/
Builder<T> eTag(String etag);

/**
* Set the entity tag of the body, as specified by the {@code ETag} header.
* @since TODO
* @param etag the new entity tag
* @return this builder
* @see HttpHeaders#setETag(ETag)
*/
Builder<T> eTag(ETag etag);

/**
* Set the time the resource was last changed, as specified by the
* {@code Last-Modified} header.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.core.ReactiveAdapterRegistry;
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;
Expand Down Expand Up @@ -300,6 +301,15 @@ interface HeadersBuilder<B extends HeadersBuilder<B>> {
*/
B eTag(String eTag);

/**
* Set the entity tag of the body, as specified by the {@code ETag} header.
* @since TODO
* @param eTag the new entity tag
* @return this builder
* @see HttpHeaders#setETag(ETag)
*/
B eTag(ETag eTag);

/**
* Set the time the resource was last changed, as specified by the
* {@code Last-Modified} header.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import org.springframework.core.io.AbstractResource;
import org.springframework.core.io.Resource;
import org.springframework.http.ETag;
import org.springframework.http.HttpHeaders;
import org.springframework.lang.Nullable;
import org.springframework.util.AntPathMatcher;
Expand Down Expand Up @@ -334,7 +335,7 @@ public String getDescription() {
public HttpHeaders getResponseHeaders() {
HttpHeaders headers = (this.original instanceof HttpResource httpResource ?
httpResource.getResponseHeaders() : new HttpHeaders());
headers.setETag("W/\"" + this.version + "\"");
headers.setETag(new ETag(this.version, true));
return headers;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.springframework.core.io.Resource;
import org.springframework.core.io.support.ResourceRegion;
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;
Expand Down Expand Up @@ -166,12 +167,12 @@ public EntityResponse.Builder<T> contentType(MediaType contentType) {

@Override
public EntityResponse.Builder<T> eTag(String etag) {
if (!etag.startsWith("\"") && !etag.startsWith("W/\"")) {
etag = "\"" + etag;
}
if (!etag.endsWith("\"")) {
etag = etag + "\"";
}
this.headers.setETag(etag);
return this;
}

@Override
public EntityResponse.Builder<T> eTag(ETag etag) {
this.headers.setETag(etag);
return this;
}
Expand Down
Loading
Loading