hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HTTPCLIENT-1767) Null pointer dereference in EofSensorInputStream and ResponseEntityProxy
Date Tue, 13 Sep 2016 18:35:21 GMT

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15488016#comment-15488016

ASF GitHub Bot commented on HTTPCLIENT-1767:

Github user ok2c commented on the issue:

    Committed to SVN trunk and 4.5.x branch. Please review and close.

> 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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org

View raw message