Don't remove content-length header in feign ribbon.

Previously, when feign used the Ribbon RestClient, an exception was thrown if Content-Length was set. Over time, feign has moved away from RestClient, yet removing the header still happened. Fixes gh-1705
2026-09-20 00:59:01 +00:00 · 2017-03-06 20:11:03 -07:00
parent 6df7190472
commit 15e01dfe5d
4 changed files with 208 additions and 5 deletions
@@ -120,8 +120,6 @@ public class FeignLoadBalancer extends
 		private Request toRequest(Request request) {
 			Map<String, Collection<String>> headers = new LinkedHashMap<>(
 					request.headers());
-			// Apache client barfs if you set the content length
-			headers.remove(Util.CONTENT_LENGTH);
 			return Request.create(request.method(),getUri().toASCIIString(),headers,request.body(),request.charset());
 		}

@@ -62,6 +62,7 @@ import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
@@ -455,8 +456,11 @@ public class FeignClientTests {
 		}

 		@RequestMapping(method = RequestMethod.POST, consumes = "application/vnd.io.spring.cloud.test.v1+json", produces = "application/vnd.io.spring.cloud.test.v1+json", path = "/complex")
-		String complex(String body) {
-			return "{\"value\":\"OK\"}";
+		String complex(@RequestBody String body, @RequestHeader("Content-Length") int contentLength) {
+			if (contentLength <= 0) {
+				throw new IllegalArgumentException("Invalid Content-Length "+ contentLength);
+			}
+			return body;
 		}

 		@RequestMapping(method = RequestMethod.GET, path = "/tostring")
@@ -42,6 +42,8 @@ import org.springframework.http.ResponseEntity;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RestController;
@@ -110,7 +112,14 @@ public class FeignHttpClientTests {
 		}

 		@RequestMapping(method = RequestMethod.PATCH, value = "/hellop")
-		public ResponseEntity<Void> patchHello() {
+		public ResponseEntity<Void> patchHello(@RequestBody Hello hello,
+											   @RequestHeader("Content-Length") int contentLength) {
+			if (contentLength <= 0) {
+				throw new IllegalArgumentException("Invalid Content-Length "+ contentLength);
+			}
+			if (!hello.getMessage().equals("foo")) {
+				throw new IllegalArgumentException("Invalid Hello: " + hello.getMessage());
+			}
 			return ResponseEntity.ok().header("X-Hello", "hello world patch").build();
 		}

@@ -0,0 +1,192 @@
+/*
+ * Copyright 2013-2017 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.springframework.cloud.netflix.feign.valid;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
+import org.springframework.cloud.netflix.feign.EnableFeignClients;
+import org.springframework.cloud.netflix.feign.FeignClient;
+import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient;
+import org.springframework.cloud.netflix.ribbon.RibbonClient;
+import org.springframework.cloud.netflix.ribbon.StaticServerList;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.http.ResponseEntity;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestHeader;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RestController;
+
+import com.netflix.loadbalancer.Server;
+import com.netflix.loadbalancer.ServerList;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
+import feign.Client;
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+/**
+ * @author Spencer Gibb
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@SpringBootTest(classes = FeignOkHttpTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = {
+		"spring.application.name=feignclienttest", "feign.hystrix.enabled=false",
+		"feign.okhttp.enabled=true" })
+@DirtiesContext
+public class FeignOkHttpTests {
+
+	@Value("${local.server.port}")
+	private int port = 0;
+
+	@Autowired
+	private TestClient testClient;
+
+	@Autowired
+	private Client feignClient;
+
+	@Autowired
+	private UserClient userClient;
+
+	@FeignClient("localapp")
+	protected interface TestClient extends BaseTestClient {
+	}
+
+	protected interface BaseTestClient {
+		@RequestMapping(method = RequestMethod.GET, value = "/hello")
+		Hello getHello();
+
+		@RequestMapping(method = RequestMethod.PATCH, value = "/hellop", consumes = "application/json")
+		ResponseEntity<Void> patchHello(Hello hello);
+	}
+
+	protected interface UserService {
+		@RequestMapping(method = RequestMethod.GET, value = "/users/{id}")
+		User getUser(@PathVariable("id") long id);
+	}
+
+	@FeignClient("localapp")
+	protected interface UserClient extends UserService {
+	}
+
+	@Configuration
+	@EnableAutoConfiguration
+	@RestController
+	@EnableFeignClients(clients = { TestClient.class, UserClient.class })
+	@RibbonClient(name = "localapp", configuration = LocalRibbonClientConfiguration.class)
+	protected static class Application implements UserService {
+
+		@RequestMapping(method = RequestMethod.GET, value = "/hello")
+		public Hello getHello() {
+			return new Hello("hello world 1");
+		}
+
+		@RequestMapping(method = RequestMethod.PATCH, value = "/hellop")
+		public ResponseEntity<Void> patchHello(@RequestBody Hello hello,
+											   @RequestHeader("Content-Length") int contentLength) {
+			if (contentLength <= 0) {
+				throw new IllegalArgumentException("Invalid Content-Length "+ contentLength);
+			}
+			if (!hello.getMessage().equals("foo")) {
+				throw new IllegalArgumentException("Invalid Hello: " + hello.getMessage());
+			}
+			return ResponseEntity.ok().header("X-Hello", "hello world patch").build();
+		}
+
+		@Override
+		public User getUser(@PathVariable("id") long id) {
+			return new User("John Smith");
+		}
+
+	}
+
+	@Test
+	public void testSimpleType() {
+		Hello hello = this.testClient.getHello();
+		assertNotNull("hello was null", hello);
+		assertEquals("first hello didn't match", new Hello("hello world 1"), hello);
+	}
+
+	@Test
+	public void testPatch() {
+		ResponseEntity<Void> response = this.testClient.patchHello(new Hello("foo"));
+		assertThat(response, is(notNullValue()));
+		String header = response.getHeaders().getFirst("X-Hello");
+		assertThat(header, equalTo("hello world patch"));
+	}
+
+	@Test
+	public void testFeignClientType() throws IllegalAccessException {
+		assertThat(this.feignClient, is(instanceOf(LoadBalancerFeignClient.class)));
+		LoadBalancerFeignClient client = (LoadBalancerFeignClient) this.feignClient;
+		Client delegate = client.getDelegate();
+		assertThat(delegate, is(instanceOf(feign.okhttp.OkHttpClient.class)));
+	}
+
+	@Test
+	public void testFeignInheritanceSupport() {
+		assertNotNull("UserClient was null", this.userClient);
+		final User user = this.userClient.getUser(1);
+		assertNotNull("Returned user was null", user);
+		assertEquals("Users were different", user, new User("John Smith"));
+	}
+
+	@Data
+	@AllArgsConstructor
+	@NoArgsConstructor
+	public static class Hello {
+		private String message;
+	}
+
+	@Data
+	@AllArgsConstructor
+	@NoArgsConstructor
+	public static class User {
+		private String name;
+	}
+
+	// Load balancer with fixed server list for "local" pointing to localhost
+	@Configuration
+	static class LocalRibbonClientConfiguration {
+
+		@Value("${local.server.port}")
+		private int port = 0;
+
+		@Bean
+		public ServerList<Server> ribbonServerList() {
+			return new StaticServerList<>(new Server("localhost", this.port));
+		}
+
+	}
+}