You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by nobodyiam <gi...@git.apache.org> on 2018/03/27 10:46:42 UTC
[GitHub] httpasyncclient pull request #8: Fix the issue when redirecting a post reque...
GitHub user nobodyiam opened a pull request:
https://github.com/apache/httpasyncclient/pull/8
Fix the issue when redirecting a post request, original 'Host' header is carried
## Issue description
We have a situation that allows 307 redirects for POST methods, which works well when using HttpClient.
However, when using HttpAsyncClient, we found the redirected request carried the wrong 'Host' header, which caused the requests failed with error responses like '421 Misdirected Request'.
## Demo to duplicate the issue
Attached is a simple demo to dupliate the issue: [issue-duplication-demo.zip](https://github.com/apache/httpasyncclient/files/1851502/issue-duplication-demo.zip)
In this demo, we have a web controller that returns 307 redirect to `https://hc.apache.org`:
```java
@RestController
public class Test307Controller {
@PostMapping("/test307")
@RequestMapping(value = "/test307", method = {RequestMethod.GET, RequestMethod.POST})
public ModelAndView redirectPostToPost(HttpServletRequest request) {
request.setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, HttpStatus.TEMPORARY_REDIRECT);
return new ModelAndView("redirect:https://hc.apache.org");
}
}
```
And we have 2 tests that send a http post to this controller using HttpClient and HttpAsyncClient respectively, which both expect a 200 response.
```Java
@Test
public void testSyncPost() throws Exception {
CloseableHttpClient httpclient = createSyncClient();
try {
HttpPost request = new HttpPost("http://localhost:8080/test307");
CloseableHttpResponse response = httpclient.execute(request);
System.out.println("Response status line: " + response.getStatusLine());
assertEquals(200, response.getStatusLine().getStatusCode());
} finally {
httpclient.close();
}
}
@Test
public void testAsyncPost() throws Exception {
CloseableHttpAsyncClient httpclient = createAsyncClient();
try {
httpclient.start();
HttpPost request = new HttpPost("http://localhost:8080/test307");
Future<HttpResponse> future = httpclient.execute(request, null);
HttpResponse response = future.get();
System.out.println("Response status line: " + response.getStatusLine());
assertEquals(200, response.getStatusLine().getStatusCode());
} finally {
httpclient.close();
}
}
```
However, only the HttpClient case will succeed and the HttpAsyncClient will always fail with a http 421 response.
After downloading the demo to your computer, you may run `mvn clean test` directly and shall see the `testSyncPost` case succeeds and the `testAsyncPost` case fails.
## Locating the issue
After debugging both the HttpClient and HttpAsyncClient codes, we found the difference is [HttpClient](https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java#L122) passed `currentRequest.getOriginal()` to `redirectStrategy.getRedirect` method, while [HttpAsyncClient](https://github.com/apache/httpasyncclient/blob/4.1.x/httpasyncclient/src/main/java/org/apache/http/impl/nio/client/MainClientExec.java#L591) passed `currentRequest` directly to `redirectStrategy.getRedirect` method.
[HttpClient logic](https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java#L122):
```java
final HttpRequest redirect = this.redirectStrategy.getRedirect(currentRequest.getOriginal(), response, context);
```
[HttpAsyncClient logic](https://github.com/apache/httpasyncclient/blob/4.1.x/httpasyncclient/src/main/java/org/apache/http/impl/nio/client/MainClientExec.java#L591):
```java
final HttpUriRequest redirect = this.redirectStrategy.getRedirect(currentRequest, currentResponse, localContext);
```
Since the current request contains the 'Host' header, the redirected request will fail.
For the demo case, the redirected request to `hc.apache.org` looks like the following, which is obviously wrong since the `Host` header is still `localhost:8080`:
```
POST / HTTP/1.1
Host: localhost:8080
...
User-Agent: Apache-HttpAsyncClient/4.1.3 (Java/1.8.0_74)
```
## Fixing the issue
The fix to this issue is simple: passing `currentRequest.getOriginal()` to `redirectStrategy.getRedirect` method, just the same logic as what [HttpClient](https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/impl/execchain/RedirectExec.java#L122) does.
So please help to review this change, we do need this change available ASAP.
Thanks!
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nobodyiam/httpasyncclient 4.1.x
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/httpasyncclient/pull/8.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #8
----
commit e47ed00e488a6b2d84ad8043fdb197f8a7b2716c
Author: Jason Song <no...@...>
Date: 2018-03-27T10:07:17Z
fix the issue when redirecting a post request, the original 'Host' header is carried, which will cause errors like '421 Misdirected Request'
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by nobodyiam <gi...@git.apache.org>.
Github user nobodyiam commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
BTW, it seems this project's travis-ci has some issue in executing the 'rake' command, which causes the ci failed.
I have run `mvn clean test` locally, with no test failed.
```
[INFO] Reactor Summary:
[INFO]
[INFO] Apache HttpComponents AsyncClient .................. SUCCESS [ 2.344 s]
[INFO] Apache HttpAsyncClient ............................. SUCCESS [ 12.965 s]
[INFO] Apache HttpAsyncClient Cache ....................... SUCCESS [ 5.083 s]
[INFO] Apache HttpAsyncClient OSGi bundle ................. SUCCESS [ 1.241 s]
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
@nobodyiam Looks reasonable and well researched to me. Would you mind, though, adding a test case to the existing integration test suite?
https://github.com/apache/httpasyncclient/blob/4.1.x/httpasyncclient/src/test/java/org/apache/http/nio/client/integration/TestRedirects.java
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
@nobodyiam Change-set committed as 401cebe5d1492102c2cd3edaa777fc67b1b424a5. Please review and close the PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by nobodyiam <gi...@git.apache.org>.
Github user nobodyiam commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
@ok2c Thanks! BTW, is there an estimate date for 4.1.4 release?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by nobodyiam <gi...@git.apache.org>.
Github user nobodyiam commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
@ok2c sure
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
@nobodyiam No immediate plans. It is likely to take a few more months.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient pull request #8: Fix the issue when redirecting a post reque...
Posted by nobodyiam <gi...@git.apache.org>.
Github user nobodyiam closed the pull request at:
https://github.com/apache/httpasyncclient/pull/8
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpasyncclient issue #8: Fix the issue when redirecting a post request, ori...
Posted by nobodyiam <gi...@git.apache.org>.
Github user nobodyiam commented on the issue:
https://github.com/apache/httpasyncclient/pull/8
@ok2c I added a test case `testPostRedirectWithDifferentHost` to [TestRedirects](https://github.com/apache/httpasyncclient/blob/4.1.x/httpasyncclient/src/test/java/org/apache/http/nio/client/integration/TestRedirects.java), which fails with the previous code and succeeds with the patched one, please help to review it. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org