From b08ad44230269a2a8717c3c8a25a9b5539a55a22 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Tue, 3 Mar 2009 09:32:05 +0000 Subject: [PATCH] SPR-5536: RestTemplate does not do HTTP GET if it should --- .../http/client/CommonsClientHttpRequest.java | 4 +- .../http/client/SimpleClientHttpRequest.java | 8 ++- .../SimpleClientHttpRequestFactory.java | 8 ++- .../AbstractHttpRequestFactoryTestCase.java | 70 ++++++++++++++++--- 4 files changed, 74 insertions(+), 16 deletions(-) diff --git a/org.springframework.web/src/main/java/org/springframework/http/client/CommonsClientHttpRequest.java b/org.springframework.web/src/main/java/org/springframework/http/client/CommonsClientHttpRequest.java index ebf2df4d5f..b10d663929 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/client/CommonsClientHttpRequest.java +++ b/org.springframework.web/src/main/java/org/springframework/http/client/CommonsClientHttpRequest.java @@ -28,8 +28,6 @@ import org.apache.commons.httpclient.methods.RequestEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; -import org.springframework.http.client.AbstractClientHttpRequest; -import org.springframework.http.client.ClientHttpResponse; /** * {@link org.springframework.http.client.ClientHttpRequest} implementation that uses @@ -48,7 +46,7 @@ final class CommonsClientHttpRequest extends AbstractClientHttpRequest { private final HttpMethodBase httpMethod; - public CommonsClientHttpRequest(HttpClient httpClient, HttpMethodBase httpMethod) { + CommonsClientHttpRequest(HttpClient httpClient, HttpMethodBase httpMethod) { this.httpClient = httpClient; this.httpMethod = httpMethod; } diff --git a/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java b/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java index d4ec106de0..9b7a92a37c 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java +++ b/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java @@ -21,9 +21,9 @@ import java.net.HttpURLConnection; import java.util.List; import java.util.Map; -import org.springframework.util.FileCopyUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.util.FileCopyUtils; /** * {@link ClientHttpRequest} implementation that uses standard J2SE facilities to execute requests. @@ -38,7 +38,7 @@ final class SimpleClientHttpRequest extends AbstractClientHttpRequest { private final HttpURLConnection connection; - public SimpleClientHttpRequest(HttpURLConnection connection) { + SimpleClientHttpRequest(HttpURLConnection connection) { this.connection = connection; } @@ -56,7 +56,9 @@ final class SimpleClientHttpRequest extends AbstractClientHttpRequest { } } this.connection.connect(); - FileCopyUtils.copy(bufferedOutput, this.connection.getOutputStream()); + if (bufferedOutput.length > 0) { + FileCopyUtils.copy(bufferedOutput, this.connection.getOutputStream()); + } return new SimpleClientHttpResponse(this.connection); } diff --git a/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java b/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java index 7deeb716c9..a199e3a3af 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java +++ b/org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java @@ -21,8 +21,8 @@ import java.net.HttpURLConnection; import java.net.URI; import java.net.URLConnection; -import org.springframework.util.Assert; import org.springframework.http.HttpMethod; +import org.springframework.util.Assert; /** * {@link ClientHttpRequestFactory} implementation that uses standard J2SE facilities. @@ -51,7 +51,11 @@ public class SimpleClientHttpRequestFactory implements ClientHttpRequestFactory */ protected void prepareConnection(HttpURLConnection connection, String httpMethod) throws IOException { connection.setDoInput(true); - connection.setDoOutput(true); + if ("PUT".equals(httpMethod) || "POST".equals(httpMethod)) { + connection.setDoOutput(true); + } else { + connection.setDoOutput(false); + } connection.setRequestMethod(httpMethod); } diff --git a/org.springframework.web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java b/org.springframework.web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java index 85bdf86607..ee00428580 100644 --- a/org.springframework.web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java +++ b/org.springframework.web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.net.URI; import java.util.Arrays; import java.util.Enumeration; +import java.util.Locale; import javax.servlet.GenericServlet; import javax.servlet.ServletException; import javax.servlet.ServletRequest; @@ -37,9 +38,9 @@ import org.mortbay.jetty.Server; import org.mortbay.jetty.servlet.Context; import org.mortbay.jetty.servlet.ServletHolder; -import org.springframework.util.FileCopyUtils; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.util.FileCopyUtils; public abstract class AbstractHttpRequestFactoryTestCase { @@ -53,6 +54,12 @@ public abstract class AbstractHttpRequestFactoryTestCase { Context jettyContext = new Context(jettyServer, "/"); jettyContext.addServlet(new ServletHolder(new EchoServlet()), "/echo"); jettyContext.addServlet(new ServletHolder(new ErrorServlet(404)), "/errors/notfound"); + jettyContext.addServlet(new ServletHolder(new MethodServlet("DELETE")), "/methods/delete"); + jettyContext.addServlet(new ServletHolder(new MethodServlet("GET")), "/methods/get"); + jettyContext.addServlet(new ServletHolder(new MethodServlet("HEAD")), "/methods/head"); + jettyContext.addServlet(new ServletHolder(new MethodServlet("OPTIONS")), "/methods/options"); + jettyContext.addServlet(new ServletHolder(new MethodServlet("POST")), "/methods/post"); + jettyContext.addServlet(new ServletHolder(new MethodServlet("PUT")), "/methods/put"); jettyServer.start(); } @@ -104,8 +111,13 @@ public abstract class AbstractHttpRequestFactoryTestCase { ClientHttpRequest request = factory.createRequest(new URI("http://localhost:8889/echo"), HttpMethod.POST); byte[] body = "Hello World".getBytes("UTF-8"); FileCopyUtils.copy(body, request.getBody()); - request.execute(); - FileCopyUtils.copy(body, request.getBody()); + ClientHttpResponse response = request.execute(); + try { + FileCopyUtils.copy(body, request.getBody()); + } + finally { + response.close(); + } } @Test(expected = IllegalStateException.class) @@ -114,13 +126,40 @@ public abstract class AbstractHttpRequestFactoryTestCase { request.getHeaders().add("MyHeader", "value"); byte[] body = "Hello World".getBytes("UTF-8"); FileCopyUtils.copy(body, request.getBody()); - request.execute(); - request.getHeaders().add("MyHeader", "value"); + ClientHttpResponse response = request.execute(); + try { + request.getHeaders().add("MyHeader", "value"); + } + finally { + response.close(); + } + } + + @Test + public void httpMethods() throws Exception { + assertHttpMethod("get", HttpMethod.GET); + assertHttpMethod("head", HttpMethod.HEAD); + assertHttpMethod("post", HttpMethod.POST); + assertHttpMethod("put", HttpMethod.PUT); + assertHttpMethod("options", HttpMethod.OPTIONS); + assertHttpMethod("delete", HttpMethod.DELETE); } - /** - * Servlet that returns and error message for a given status code. - */ + private void assertHttpMethod(String path, HttpMethod method) throws Exception { + ClientHttpResponse response = null; + try { + ClientHttpRequest request = factory.createRequest(new URI("http://localhost:8889/methods/" + path), method); + response = request.execute(); + assertEquals("Invalid method", path.toUpperCase(Locale.ENGLISH), request.getMethod().name()); + } + finally { + if (response != null) { + response.close(); + } + } + } + + /** Servlet that returns and error message for a given status code. */ private static class ErrorServlet extends GenericServlet { private final int sc; @@ -135,6 +174,21 @@ public abstract class AbstractHttpRequestFactoryTestCase { } } + private static class MethodServlet extends GenericServlet { + + private final String method; + + private MethodServlet(String method) { + this.method = method; + } + + @Override + public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException { + HttpServletRequest httpReq = (HttpServletRequest) req; + assertEquals("Invalid HTTP method", method, httpReq.getMethod()); + } + } + private static class EchoServlet extends HttpServlet { @Override -- GitLab