From c12d93c5d1696fc030b7b2fc0fb999b99795c59c Mon Sep 17 00:00:00 2001 From: divcon Date: Thu, 10 Nov 2022 22:33:01 +0900 Subject: [PATCH] Polish ServletWebRequest and DefaultServerWebExchange - The return values of ServletWebRequest.validateIfUnmodifiedSince and DefaultServerWebExchange.validateIfUnmodifiedSince are not used. So I think that it is better to remove the return statements. - Add missing @Nullable declarations to eTag method parameters. - Simplify if statements Closes gh-29460 --- .../context/request/ServletWebRequest.java | 23 ++++++++----------- .../adapter/DefaultServerWebExchange.java | 14 +++++------ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java index 7da825ff30..ba27169d02 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java @@ -232,10 +232,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ private boolean validateIfMatch(@Nullable String eTag) { Enumeration ifMatchHeaders = getRequest().getHeaders(HttpHeaders.IF_MATCH); - if (SAFE_METHODS.contains(getRequest().getMethod())) { - return false; - } - if (!ifMatchHeaders.hasMoreElements()) { + if (SAFE_METHODS.contains(getRequest().getMethod()) || !ifMatchHeaders.hasMoreElements()) { return false; } this.notModified = matchRequestedETags(ifMatchHeaders, eTag, false); @@ -308,7 +305,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ return first.equals(second); } - private void updateResponseStateChanging(String eTag, long lastModifiedTimestamp) { + private void updateResponseStateChanging(@Nullable String eTag, long lastModifiedTimestamp) { if (this.notModified && getResponse() != null) { getResponse().setStatus(HttpStatus.PRECONDITION_FAILED.value()); } @@ -329,20 +326,18 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ return true; } - private boolean validateIfModifiedSince(long lastModifiedTimestamp) { + private void validateIfModifiedSince(long lastModifiedTimestamp) { if (lastModifiedTimestamp < 0) { - return false; + return; } long ifModifiedSince = parseDateHeader(HttpHeaders.IF_MODIFIED_SINCE); - if (ifModifiedSince == -1) { - return false; + if (ifModifiedSince != -1) { + // We will perform this validation... + this.notModified = ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000); } - // We will perform this validation... - this.notModified = ifModifiedSince >= (lastModifiedTimestamp / 1000 * 1000); - return true; } - private void updateResponseIdempotent(String eTag, long lastModifiedTimestamp) { + private void updateResponseIdempotent(@Nullable String eTag, long lastModifiedTimestamp) { if (getResponse() != null) { boolean isHttpGetOrHead = SAFE_METHODS.contains(getRequest().getMethod()); if (this.notModified) { @@ -353,7 +348,7 @@ public class ServletWebRequest extends ServletRequestAttributes implements Nativ } } - private void addCachingResponseHeaders(String eTag, long lastModifiedTimestamp) { + private void addCachingResponseHeaders(@Nullable String eTag, long lastModifiedTimestamp) { if (SAFE_METHODS.contains(getRequest().getMethod())) { if (lastModifiedTimestamp > 0 && parseDateValue(getResponse().getHeader(HttpHeaders.LAST_MODIFIED)) == -1) { getResponse().setDateHeader(HttpHeaders.LAST_MODIFIED, lastModifiedTimestamp); 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 221b5c6efb..dc3537a0f8 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 @@ -346,7 +346,7 @@ public class DefaultServerWebExchange implements ServerWebExchange { return first.equals(second); } - private void updateResponseStateChanging(String eTag, Instant lastModified) { + private void updateResponseStateChanging(@Nullable String eTag, Instant lastModified) { if (this.notModified) { getResponse().setStatusCode(HttpStatus.PRECONDITION_FAILED); } @@ -401,17 +401,15 @@ public class DefaultServerWebExchange implements ServerWebExchange { return true; } - private boolean validateIfModifiedSince(Instant lastModified) { + private void validateIfModifiedSince(Instant lastModified) { if (lastModified.isBefore(Instant.EPOCH)) { - return false; + return; } long ifModifiedSince = getRequestHeaders().getIfModifiedSince(); - if (ifModifiedSince == -1) { - return false; + if (ifModifiedSince != -1) { + // We will perform this validation... + this.notModified = ChronoUnit.SECONDS.between(lastModified, Instant.ofEpochMilli(ifModifiedSince)) >= 0; } - // We will perform this validation... - this.notModified = ChronoUnit.SECONDS.between(lastModified, Instant.ofEpochMilli(ifModifiedSince)) >= 0; - return true; } @Override