From b9e4f160c58b427f6d287242b2aed2cd6ed2b2db Mon Sep 17 00:00:00 2001 From: Denys Ivano Date: Fri, 16 Mar 2018 17:14:10 +0200 Subject: [PATCH 1/2] DefaultResponseErrorHandler.hasError doesn't read response body. When response contains unknown HTTP status code, do not read and waste response body. Issue: SPR-16604 --- .../client/DefaultResponseErrorHandler.java | 9 ++- .../DefaultResponseErrorHandlerTests.java | 56 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java index f4d032ef84a3..7358d8856193 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java @@ -37,6 +37,7 @@ * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Denys Ivano * @since 3.0 * @see RestTemplate#setErrorHandler */ @@ -46,13 +47,15 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler { * Delegates to {@link #hasError(HttpStatus)} with the response status code. */ @Override - public boolean hasError(ClientHttpResponse response) throws IOException { + public boolean hasError(ClientHttpResponse response) throws IOException { + HttpStatus statusCode; try { - return hasError(getHttpStatusCode(response)); + statusCode = response.getStatusCode(); } - catch (UnknownHttpStatusCodeException ex) { + catch (IllegalArgumentException ex) { return false; } + return hasError(statusCode); } /** diff --git a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java index 950a21fdf137..3cb77573e098 100644 --- a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java @@ -18,6 +18,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; +import java.nio.charset.Charset; import org.junit.Test; @@ -25,6 +26,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.util.StreamUtils; import static org.junit.Assert.*; import static org.mockito.BDDMockito.*; @@ -34,9 +36,12 @@ * * @author Arjen Poutsma * @author Juergen Hoeller + * @author Denys Ivano */ public class DefaultResponseErrorHandlerTests { + private final Charset UTF8 = Charset.forName("UTF-8"); + private final DefaultResponseErrorHandler handler = new DefaultResponseErrorHandler(); private final ClientHttpResponse response = mock(ClientHttpResponse.class); @@ -122,4 +127,55 @@ public void hasErrorForUnknownStatusCode() throws Exception { assertFalse(handler.hasError(response)); } + @Test // SPR-16604 + public void bodyAvailableAfterHasErrorForUnknownStatusCode() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.TEXT_PLAIN); + TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes("UTF-8")); + + given(response.getStatusCode()).willThrow(new IllegalArgumentException("No matching constant for 999")); + given(response.getStatusText()).willReturn("Custom status code"); + given(response.getHeaders()).willReturn(headers); + given(response.getBody()).willReturn(body); + + assertFalse(handler.hasError(response)); + assertFalse(body.isClosed()); + assertEquals("Hello World", StreamUtils.copyToString(response.getBody(), UTF8)); + } + + class TestByteArrayInputStream extends ByteArrayInputStream { + + private boolean closed; + + public TestByteArrayInputStream(byte[] buf) { + super(buf); + this.closed = false; + } + + public boolean isClosed() { + return closed; + } + + @Override + public boolean markSupported() { + return false; + } + + @Override + public synchronized void mark(int readlimit) { + throw new UnsupportedOperationException("mark/reset not supported"); + } + + @Override + public synchronized void reset() { + throw new UnsupportedOperationException("mark/reset not supported"); + } + + @Override + public void close() throws IOException { + super.close(); + this.closed = true; + } + } + } From dbc1385d21cd940b9c6958eb3927e0586e93dbda Mon Sep 17 00:00:00 2001 From: Denys Ivano Date: Fri, 16 Mar 2018 19:40:25 +0200 Subject: [PATCH 2/2] Polishing --- .../web/client/DefaultResponseErrorHandler.java | 4 ++-- .../web/client/DefaultResponseErrorHandlerTests.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java index 7358d8856193..3ba5676675ed 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -47,7 +47,7 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler { * Delegates to {@link #hasError(HttpStatus)} with the response status code. */ @Override - public boolean hasError(ClientHttpResponse response) throws IOException { + public boolean hasError(ClientHttpResponse response) throws IOException { HttpStatus statusCode; try { statusCode = response.getStatusCode(); diff --git a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java index 3cb77573e098..368bb78bfceb 100644 --- a/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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.