Peter Ansell created HTTPCLIENT-1767:
----------------------------------------
Summary: Null pointer dereference in EofSensorInputStream and
ResponseEntityProxy
Key: HTTPCLIENT-1767
URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1767
Project: HttpComponents HttpClient
Issue Type: Bug
Components: HttpClient
Affects Versions: 4.5.2
Reporter: Peter Ansell
The close implementation in EofSensorInputStream objects which are created from
ResponseEntityProxy.getContent signals that it is closed and in "EOF mode"
internally when its wrapped InputStream variable changes to null. There are
null guards present, but these can fail sporadically even in single-threaded
situations, due to the lack of a volatile label on the instance variable and
because the instance variable is dereferenced again after the null guard within
the method.
ResponseEntityProxy.streamClosed (which is called back from
EofSensorInputStream.checkClose) then operates on its parameter without a null
check, causing an NPE to be thrown.
The resulting stack trace has the following at its top:
{noformat}
java.lang.NullPointerException: null
at
org.apache.http.impl.execchain.ResponseEntityProxy.streamClosed(ResponseEntityProxy.java:140)
at
org.apache.http.conn.EofSensorInputStream.checkClose(EofSensorInputStream.java:228)
at
org.apache.http.conn.EofSensorInputStream.close(EofSensorInputStream.java:174)
{noformat}
The EofSensorInputStream class is labelled as @NotThreadSafe, but this issue
can be fixed without synchronisation, just by dereferencing to a local variable
before the null guard and using that local variable throughout the rest of the
method. This will also work without setting the variable to volatile, which may
be part of the issue but doesn't need to be fixed to stop this occurring.
For context, I am not intentionally trying to access the EofInputStream from
multiple threads and this issue is fairly difficult to reproduce but has
happened enough to want to report it and propose a fix. The code that is
reproducing the issue should have locked down access to the close method using
the following pattern which I have never had issues with in the past, but is
still managing to trigger this NPE in unusual circumstances:
At minimum I only really need the close method fixed, as its general contract
in java.io.Closeable says that it can be called multiple times without issue,
but the same changes could be propagated to other methods if necessary.
{noformat}
private final AtomicBoolean closed = new AtomicBoolean(false);
....
@Override
public void close() throws IOException {
// This is the only time closed is accessed or modified
if(closed.compareAndSet(false, true)) {
inputStream.close();
}
}
{noformat}
I will open a Pull Request on GitHub with a patch to fix this for the 4.5.x
series.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
