Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix NPE in Http1ClientCodec #2210

Merged
merged 2 commits into from
Oct 24, 2019
Merged

Fix NPE in Http1ClientCodec #2210

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e492d7
Fix NPE in Http1ClientCodec
minwoox Oct 24, 2019
6d29c1f
Remove if condition
minwoox Oct 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 42 additions & 39 deletions core/src/main/java/com/linecorp/armeria/internal/Http1ClientCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
public class Http1ClientCodec extends CombinedChannelDuplexHandler<HttpResponseDecoder, HttpRequestEncoder>
implements HttpClientUpgradeHandler.SourceCodec {

// Forked from Netty 4.1.34 at e0bbff74f7097f000472785982ad86c0ce891567
// Forked from Netty 4.1.41 at 9ec3411c91bdc50e78f3d50b393ab815d2be0f92
// - Made the class non-final so that we can intercept the close() request.
// - Handle 1xx responses correctly, not just 100 and 101.

Expand Down Expand Up @@ -190,7 +190,7 @@ protected void encode(
return;
}

if (msg instanceof HttpRequest && !done) {
if (msg instanceof HttpRequest) {
queue.offer(((HttpRequest) msg).method());
}

Expand Down Expand Up @@ -263,46 +263,49 @@ protected boolean isContentAlwaysEmpty(HttpMessage msg) {
// current response.
final HttpMethod method = queue.poll();

final char firstChar = method.name().charAt(0);
switch (firstChar) {
case 'H':
// According to 4.3, RFC2616:
// All responses to the HEAD request method MUST NOT include a
// message-body, even though the presence of entity-header fields
// might lead one to believe they do.
if (HttpMethod.HEAD.equals(method)) {
return true;

// The following code was inserted to work around the servers
// that behave incorrectly. It has been commented out
// because it does not work with well behaving servers.
// Please note, even if the 'Transfer-Encoding: chunked'
// header exists in the HEAD response, the response should
// have absolutely no content.
//
//// Interesting edge case:
//// Some poorly implemented servers will send a zero-byte
//// chunk if Transfer-Encoding of the response is 'chunked'.
////
//// return !msg.isChunked();
}
break;
case 'C':
// Successful CONNECT request results in a response with empty body.
if (statusCode == 200) {
if (HttpMethod.CONNECT.equals(method)) {
// Proxy connection established - Parse HTTP only if configured by
// parseHttpAfterConnectRequest, else pass through.
if (!parseHttpAfterConnectRequest) {
done = true;
queue.clear();
}
// If the remote peer did for example send multiple responses for one request
// (which is not allowed per spec but may still be possible) method will be null so guard against it
if (method != null) {
final char firstChar = method.name().charAt(0);
switch (firstChar) {
case 'H':
// According to 4.3, RFC2616:
// All responses to the HEAD request method MUST NOT include a
// message-body, even though the presence of entity-header fields
// might lead one to believe they do.
if (HttpMethod.HEAD.equals(method)) {
return true;

// The following code was inserted to work around the servers
// that behave incorrectly. It has been commented out
// because it does not work with well behaving servers.
// Please note, even if the 'Transfer-Encoding: chunked'
// header exists in the HEAD response, the response should
// have absolutely no content.
//
//// Interesting edge case:
//// Some poorly implemented servers will send a zero-byte
//// chunk if Transfer-Encoding of the response is 'chunked'.
////
//// return !msg.isChunked();
}
}
break;
break;
case 'C':
// Successful CONNECT request results in a response with empty body.
if (statusCode == 200) {
if (HttpMethod.CONNECT.equals(method)) {
// Proxy connection established - Parse HTTP only if configured by
// parseHttpAfterConnectRequest, else pass through.
if (!parseHttpAfterConnectRequest) {
done = true;
queue.clear();
}
return true;
}
}
break;
}
}

return super.isContentAlwaysEmpty(msg);
}

Expand Down