Merge branch '6.1.x'

This commit is contained in:
rstoyanchev 2024-09-09 11:56:21 +01:00
commit d2b25c0378
4 changed files with 85 additions and 47 deletions

View File

@ -21,6 +21,7 @@ import java.io.PrintWriter;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer; import java.util.function.Consumer;
@ -212,6 +213,38 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
} }
/**
* 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. * Package private access for testing only.
*/ */
@ -246,7 +279,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
@SuppressWarnings("NullAway") @SuppressWarnings("NullAway")
public ServletOutputStream getOutputStream() throws IOException { public ServletOutputStream getOutputStream() throws IOException {
int level = obtainLockAndCheckState(); int level = obtainLockOrRaiseException();
try { try {
if (this.outputStream == null) { if (this.outputStream == null) {
Assert.notNull(this.asyncWebRequest, "Not initialized"); Assert.notNull(this.asyncWebRequest, "Not initialized");
@ -266,7 +299,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
@SuppressWarnings("NullAway") @SuppressWarnings("NullAway")
public PrintWriter getWriter() throws IOException { public PrintWriter getWriter() throws IOException {
int level = obtainLockAndCheckState(); int level = obtainLockOrRaiseException();
try { try {
if (this.writer == null) { if (this.writer == null) {
Assert.notNull(this.asyncWebRequest, "Not initialized"); Assert.notNull(this.asyncWebRequest, "Not initialized");
@ -284,7 +317,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void flushBuffer() throws IOException { public void flushBuffer() throws IOException {
int level = obtainLockAndCheckState(); int level = obtainLockOrRaiseException();
try { try {
getResponse().flushBuffer(); getResponse().flushBuffer();
} }
@ -296,25 +329,15 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
} }
} }
/** private int obtainLockOrRaiseException() throws AsyncRequestNotUsableException {
* 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 {
Assert.notNull(this.asyncWebRequest, "Not initialized"); Assert.notNull(this.asyncWebRequest, "Not initialized");
if (this.asyncWebRequest.state == State.NEW) { int result = this.asyncWebRequest.tryObtainLock();
return 0; if (result == -1) {
throw new AsyncRequestNotUsableException("Response not usable after " +
(this.asyncWebRequest.state == State.COMPLETED ?
"async request completion" : "response errors") + ".");
} }
return result;
this.asyncWebRequest.stateLock.lock();
if (this.asyncWebRequest.state == State.ASYNC) {
return 1;
}
this.asyncWebRequest.stateLock.unlock();
throw new AsyncRequestNotUsableException("Response not usable after " +
(this.asyncWebRequest.state == State.COMPLETED ?
"async request completion" : "response errors") + ".");
} }
void handleIOException(IOException ex, String msg) throws AsyncRequestNotUsableException { void handleIOException(IOException ex, String msg) throws AsyncRequestNotUsableException {
@ -360,7 +383,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void write(int b) throws IOException { public void write(int b) throws IOException {
int level = this.response.obtainLockAndCheckState(); int level = this.response.obtainLockOrRaiseException();
try { try {
this.delegate.write(b); this.delegate.write(b);
} }
@ -373,7 +396,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
} }
public void write(byte[] buf, int offset, int len) throws IOException { public void write(byte[] buf, int offset, int len) throws IOException {
int level = this.response.obtainLockAndCheckState(); int level = this.response.obtainLockOrRaiseException();
try { try {
this.delegate.write(buf, offset, len); this.delegate.write(buf, offset, len);
} }
@ -387,7 +410,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void flush() throws IOException { public void flush() throws IOException {
int level = this.response.obtainLockAndCheckState(); int level = this.response.obtainLockOrRaiseException();
try { try {
this.delegate.flush(); this.delegate.flush();
} }
@ -401,7 +424,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void close() throws IOException { public void close() throws IOException {
int level = this.response.obtainLockAndCheckState(); int level = this.response.obtainLockOrRaiseException();
try { try {
this.delegate.close(); this.delegate.close();
} }
@ -435,7 +458,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void flush() { public void flush() {
int level = tryObtainLockAndCheckState(); int level = this.asyncWebRequest.tryObtainLock();
if (level > -1) { if (level > -1) {
try { try {
this.delegate.flush(); this.delegate.flush();
@ -448,7 +471,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void close() { public void close() {
int level = tryObtainLockAndCheckState(); int level = this.asyncWebRequest.tryObtainLock();
if (level > -1) { if (level > -1) {
try { try {
this.delegate.close(); this.delegate.close();
@ -466,7 +489,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void write(int c) { public void write(int c) {
int level = tryObtainLockAndCheckState(); int level = this.asyncWebRequest.tryObtainLock();
if (level > -1) { if (level > -1) {
try { try {
this.delegate.write(c); this.delegate.write(c);
@ -479,7 +502,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void write(char[] buf, int off, int len) { public void write(char[] buf, int off, int len) {
int level = tryObtainLockAndCheckState(); int level = this.asyncWebRequest.tryObtainLock();
if (level > -1) { if (level > -1) {
try { try {
this.delegate.write(buf, off, len); this.delegate.write(buf, off, len);
@ -497,7 +520,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
@Override @Override
public void write(String s, int off, int len) { public void write(String s, int off, int len) {
int level = tryObtainLockAndCheckState(); int level = this.asyncWebRequest.tryObtainLock();
if (level > -1) { if (level > -1) {
try { try {
this.delegate.write(s, off, len); this.delegate.write(s, off, len);
@ -513,22 +536,6 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
this.delegate.write(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) { private void releaseLock(int level) {
if (level > 0) { if (level > 0) {
this.asyncWebRequest.stateLock.unlock(); this.asyncWebRequest.stateLock.unlock();

View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -84,7 +84,12 @@ class DefaultRenderingBuilder implements Rendering.RedirectBuilder {
@Override @Override
public DefaultRenderingBuilder status(HttpStatusCode status) { public DefaultRenderingBuilder status(HttpStatusCode status) {
this.status = status; if (this.view instanceof RedirectView) {
((RedirectView) this.view).setStatusCode(status);
}
else {
this.status = status;
}
return this; return this;
} }

View File

@ -23,6 +23,7 @@ import java.util.Map;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.http.HttpHeaders; import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -126,6 +127,16 @@ class DefaultRenderingBuilderTests {
assertThat(((RedirectView) view).isPropagateQuery()).isTrue(); 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 {} private static class Foo {}

View File

@ -16,6 +16,7 @@
package org.springframework.web.reactive.result.view; package org.springframework.web.reactive.result.view;
import java.net.URI;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.time.Duration; import java.time.Duration;
import java.util.Arrays; import java.util.Arrays;
@ -202,6 +203,20 @@ class ViewResolutionResultHandlerTests {
assertThat(exchange.getResponse().getHeaders().getFirst("h")).isEqualTo("h1"); 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 @Test
void handleWithMultipleResolvers() { void handleWithMultipleResolvers() {
testHandle("/account", testHandle("/account",