IllegalRefCountException in FullHttp[Request|Response].hashCode()

Motivation:
FullHttp[Request|Response].hashCode() uses a releasable object and in vulnerable to a IllegalRefCountException if that object has been released.

Modifications:
- Ensure the released object is not used.

Result:
No more IllegalRefCountException.
This commit is contained in:
Scott Mitchell 2015-11-09 11:45:37 -08:00
parent 120ffaf880
commit 0d71744d5b
2 changed files with 63 additions and 7 deletions

View File

@ -17,15 +17,19 @@ package io.netty.handler.codec.http;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled; import io.netty.buffer.Unpooled;
import io.netty.util.IllegalReferenceCountException;
/** /**
* Default implementation of {@link FullHttpRequest}. * Default implementation of {@link FullHttpRequest}.
*/ */
public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHttpRequest { public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHttpRequest {
private static final int HASH_CODE_PRIME = 31;
private final ByteBuf content; private final ByteBuf content;
private final HttpHeaders trailingHeader; private final HttpHeaders trailingHeader;
private final boolean validateHeaders; private final boolean validateHeaders;
/**
* Used to cache the value of the hash code and avoid {@link IllegalRefCountException}.
*/
private int hash;
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri) { public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri) {
this(httpVersion, method, uri, Unpooled.buffer(0)); this(httpVersion, method, uri, Unpooled.buffer(0));
@ -163,11 +167,23 @@ public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHt
@Override @Override
public int hashCode() { public int hashCode() {
int result = 1; int hash = this.hash;
result = HASH_CODE_PRIME * result + content().hashCode(); if (hash == 0) {
result = HASH_CODE_PRIME * result + trailingHeaders().hashCode(); if (content().refCnt() != 0) {
result = HASH_CODE_PRIME * result + super.hashCode(); try {
return result; hash = 31 + content().hashCode();
} catch (IllegalReferenceCountException ignored) {
// Handle race condition between checking refCnt() == 0 and using the object.
hash = 31;
}
} else {
hash = 31;
}
hash = 31 * hash + trailingHeaders().hashCode();
hash = 31 * hash + super.hashCode();
this.hash = hash;
}
return hash;
} }
@Override @Override

View File

@ -15,9 +15,11 @@
*/ */
package io.netty.handler.codec.http; package io.netty.handler.codec.http;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled; import io.netty.buffer.Unpooled;
import io.netty.util.IllegalReferenceCountException;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
/** /**
* Default implementation of a {@link FullHttpResponse}. * Default implementation of a {@link FullHttpResponse}.
@ -27,6 +29,10 @@ public class DefaultFullHttpResponse extends DefaultHttpResponse implements Full
private final ByteBuf content; private final ByteBuf content;
private final HttpHeaders trailingHeaders; private final HttpHeaders trailingHeaders;
private final boolean validateHeaders; private final boolean validateHeaders;
/**
* Used to cache the value of the hash code and avoid {@link IllegalRefCountException}.
*/
private int hash;
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status) { public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status) {
this(version, status, Unpooled.buffer(0)); this(version, status, Unpooled.buffer(0));
@ -164,6 +170,40 @@ public class DefaultFullHttpResponse extends DefaultHttpResponse implements Full
return duplicate; return duplicate;
} }
@Override
public int hashCode() {
int hash = this.hash;
if (hash == 0) {
if (content().refCnt() != 0) {
try {
hash = 31 + content().hashCode();
} catch (IllegalReferenceCountException ignored) {
// Handle race condition between checking refCnt() == 0 and using the object.
hash = 31;
}
} else {
hash = 31;
}
hash = 31 * hash + trailingHeaders().hashCode();
hash = 31 * hash + super.hashCode();
this.hash = hash;
}
return hash;
}
@Override
public boolean equals(Object o) {
if (!(o instanceof DefaultFullHttpResponse)) {
return false;
}
DefaultFullHttpResponse other = (DefaultFullHttpResponse) o;
return super.equals(other) &&
content().equals(other.content()) &&
trailingHeaders().equals(other.trailingHeaders());
}
@Override @Override
public String toString() { public String toString() {
return HttpMessageUtil.appendFullResponse(new StringBuilder(256), this).toString(); return HttpMessageUtil.appendFullResponse(new StringBuilder(256), this).toString();