Merge branch '6.1.x'

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java

@@ -21,6 +21,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Consumer;
 
@@ -212,6 +213,38 @@ public void onComplete(AsyncEvent event) throws IOException {
 	}
 
 
+	/**
+	 * Return 0 when there is no need to obtain a lock (no async handling in
+	 * progress), 1 if lock was acquired, and -1 if lock is not acquired because
+	 * request is no longer usable.
+	 */
+	private int tryObtainLock() {
+
+		if (this.state == State.NEW) {
+			return 0;
+		}
+
+		// Do not wait indefinitely, stop if we moved on from ASYNC state (e.g. to ERROR),
+		// helps to avoid ABBA deadlock with onError callback
+
+		while (this.state == State.ASYNC) {
+			try {
+				if (this.stateLock.tryLock(500, TimeUnit.MILLISECONDS)) {
+					if (this.state == State.ASYNC) {
+						return 1;
+					}
+					this.stateLock.unlock();
+					break;
+				}
+			}
+			catch (InterruptedException ex) {
+				// ignore
+			}
+		}
+
+		return -1;
+	}
+
 	/**
 	 * Package private access for testing only.
 	 */
@@ -246,7 +279,7 @@ public void setAsyncWebRequest(StandardServletAsyncWebRequest asyncWebRequest) {
 		@Override
 		@SuppressWarnings("NullAway")
 		public ServletOutputStream getOutputStream() throws IOException {
-			int level = obtainLockAndCheckState();
+			int level = obtainLockOrRaiseException();
 			try {
 				if (this.outputStream == null) {
 					Assert.notNull(this.asyncWebRequest, "Not initialized");
@@ -266,7 +299,7 @@ public ServletOutputStream getOutputStream() throws IOException {
 		@Override
 		@SuppressWarnings("NullAway")
 		public PrintWriter getWriter() throws IOException {
-			int level = obtainLockAndCheckState();
+			int level = obtainLockOrRaiseException();
 			try {
 				if (this.writer == null) {
 					Assert.notNull(this.asyncWebRequest, "Not initialized");
@@ -284,7 +317,7 @@ public PrintWriter getWriter() throws IOException {
 
 		@Override
 		public void flushBuffer() throws IOException {
-			int level = obtainLockAndCheckState();
+			int level = obtainLockOrRaiseException();
 			try {
 				getResponse().flushBuffer();
 			}
@@ -296,25 +329,15 @@ public void flushBuffer() throws IOException {
 			}
 		}
 
-		/**
-		 * Return 0 if checks passed and lock is not needed, 1 if checks passed
-		 * and lock is held, or raise AsyncRequestNotUsableException.
-		 */
-		private int obtainLockAndCheckState() throws AsyncRequestNotUsableException {
+		private int obtainLockOrRaiseException() throws AsyncRequestNotUsableException {
 			Assert.notNull(this.asyncWebRequest, "Not initialized");
-			if (this.asyncWebRequest.state == State.NEW) {
-				return 0;
-			}
-
-			this.asyncWebRequest.stateLock.lock();
-			if (this.asyncWebRequest.state == State.ASYNC) {
-				return 1;
+			int result = this.asyncWebRequest.tryObtainLock();
+			if (result == -1) {
+				throw new AsyncRequestNotUsableException("Response not usable after " +
+						(this.asyncWebRequest.state == State.COMPLETED ?
+								"async request completion" : "response errors") + ".");
 			}
-
-			this.asyncWebRequest.stateLock.unlock();
-			throw new AsyncRequestNotUsableException("Response not usable after " +
-					(this.asyncWebRequest.state == State.COMPLETED ?
-							"async request completion" : "response errors") + ".");
+			return result;
 		}
 
 		void handleIOException(IOException ex, String msg) throws AsyncRequestNotUsableException {
@@ -360,7 +383,7 @@ public void setWriteListener(WriteListener writeListener) {
 
 		@Override
 		public void write(int b) throws IOException {
-			int level = this.response.obtainLockAndCheckState();
+			int level = this.response.obtainLockOrRaiseException();
 			try {
 				this.delegate.write(b);
 			}
@@ -373,7 +396,7 @@ public void write(int b) throws IOException {
 		}
 
 		public void write(byte[] buf, int offset, int len) throws IOException {
-			int level = this.response.obtainLockAndCheckState();
+			int level = this.response.obtainLockOrRaiseException();
 			try {
 				this.delegate.write(buf, offset, len);
 			}
@@ -387,7 +410,7 @@ public void write(byte[] buf, int offset, int len) throws IOException {
 
 		@Override
 		public void flush() throws IOException {
-			int level = this.response.obtainLockAndCheckState();
+			int level = this.response.obtainLockOrRaiseException();
 			try {
 				this.delegate.flush();
 			}
@@ -401,7 +424,7 @@ public void flush() throws IOException {
 
 		@Override
 		public void close() throws IOException {
-			int level = this.response.obtainLockAndCheckState();
+			int level = this.response.obtainLockOrRaiseException();
 			try {
 				this.delegate.close();
 			}
@@ -435,7 +458,7 @@ private LifecyclePrintWriter(PrintWriter delegate, StandardServletAsyncWebReques
 
 		@Override
 		public void flush() {
-			int level = tryObtainLockAndCheckState();
+			int level = this.asyncWebRequest.tryObtainLock();
 			if (level > -1) {
 				try {
 					this.delegate.flush();
@@ -448,7 +471,7 @@ public void flush() {
 
 		@Override
 		public void close() {
-			int level = tryObtainLockAndCheckState();
+			int level = this.asyncWebRequest.tryObtainLock();
 			if (level > -1) {
 				try {
 					this.delegate.close();
@@ -466,7 +489,7 @@ public boolean checkError() {
 
 		@Override
 		public void write(int c) {
-			int level = tryObtainLockAndCheckState();
+			int level = this.asyncWebRequest.tryObtainLock();
 			if (level > -1) {
 				try {
 					this.delegate.write(c);
@@ -479,7 +502,7 @@ public void write(int c) {
 
 		@Override
 		public void write(char[] buf, int off, int len) {
-			int level = tryObtainLockAndCheckState();
+			int level = this.asyncWebRequest.tryObtainLock();
 			if (level > -1) {
 				try {
 					this.delegate.write(buf, off, len);
@@ -497,7 +520,7 @@ public void write(char[] buf) {
 
 		@Override
 		public void write(String s, int off, int len) {
-			int level = tryObtainLockAndCheckState();
+			int level = this.asyncWebRequest.tryObtainLock();
 			if (level > -1) {
 				try {
 					this.delegate.write(s, off, len);
@@ -513,22 +536,6 @@ public void write(String s) {
 			this.delegate.write(s);
 		}
 
-		/**
-		 * Return 0 if checks passed and lock is not needed, 1 if checks passed
-		 * and lock is held, and -1 if checks did not pass.
-		 */
-		private int tryObtainLockAndCheckState() {
-			if (this.asyncWebRequest.state == State.NEW) {
-				return 0;
-			}
-			this.asyncWebRequest.stateLock.lock();
-			if (this.asyncWebRequest.state == State.ASYNC) {
-				return 1;
-			}
-			this.asyncWebRequest.stateLock.unlock();
-			return -1;
-		}
-
 		private void releaseLock(int level) {
 			if (level > 0) {
 				this.asyncWebRequest.stateLock.unlock();

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/DefaultRenderingBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -84,7 +84,12 @@ private Model initModel() {
 
 	@Override
 	public DefaultRenderingBuilder status(HttpStatusCode status) {
-		this.status = status;
+		if (this.view instanceof RedirectView) {
+			((RedirectView) this.view).setStatusCode(status);
+		}
+		else {
+			this.status = status;
+		}
 		return this;
 	}
 

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/DefaultRenderingBuilderTests.java

@@ -23,6 +23,7 @@
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -126,6 +127,16 @@ void redirectWithPropagateQuery() {
 		assertThat(((RedirectView) view).isPropagateQuery()).isTrue();
 	}
 
+	@Test // gh-33498
+	void redirectWithCustomStatus() {
+		HttpStatus status = HttpStatus.MOVED_PERMANENTLY;
+		Rendering rendering = Rendering.redirectTo("foo").status(status).build();
+
+		Object view = rendering.view();
+		assertThat(view.getClass()).isEqualTo(RedirectView.class);
+		assertThat(((RedirectView) view).getStatusCode()).isEqualTo(status);
+	}
+
 
 	private static class Foo {}
 

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.web.reactive.result.view;
 
+import java.net.URI;
 import java.nio.ByteBuffer;
 import java.time.Duration;
 import java.util.Arrays;
@@ -202,6 +203,20 @@ void handleReturnValueTypes() {
 		assertThat(exchange.getResponse().getHeaders().getFirst("h")).isEqualTo("h1");
 	}
 
+	@Test // gh-33498
+	void handleRedirect() {
+		HttpStatus status = HttpStatus.MOVED_PERMANENTLY;
+		Rendering returnValue = Rendering.redirectTo("foo").status(status).build();
+		MethodParameter returnType = on(Handler.class).resolveReturnType(Rendering.class);
+		HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.bindingContext);
+
+		MockServerWebExchange exchange = MockServerWebExchange.from(get("/path"));
+		resultHandler(new UrlBasedViewResolver()).handleResult(exchange, result).block(Duration.ofSeconds(5));
+
+		assertThat(exchange.getResponse().getStatusCode()).isEqualTo(status);
+		assertThat(exchange.getResponse().getHeaders().getLocation()).isEqualTo(URI.create("foo"));
+	}
+
 	@Test
 	void handleWithMultipleResolvers() {
 		testHandle("/account",