[
https://issues.apache.org/jira/browse/HTTPCLIENT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15485867#comment-15485867
]
ASF GitHub Bot commented on HTTPCLIENT-1767:
--------------------------------------------
GitHub user ansell opened a pull request:
https://github.com/apache/httpclient/pull/61
HTTPCLIENT-1767: Null pointer dereference in EofSensorInputStream and
ResponseEntityProxy
This fixes HTTPCLIENT-1767 by adding null checks to ResponseEntityProxy and
dereferencing instance variables before null checks in EofSensorInputStream.
For reference here, the stacktrace, against httpclient-4.5.2, which I can't
reproduce reliably, but which should be fixed by these changes is:
```
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)
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ansell/httpclient HTTPCLIENT-1767
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/httpclient/pull/61.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 #61
----
commit 56b37dfdab78d651f95f7825b1c0f7daa166e8c5
Author: Peter Ansell <[email protected]>
Date: 2016-09-13T01:05:21Z
HTTPCLIENT-1767: Fix null pointer dereference after guard
EofSensorInputStream is generating NullPointerExceptions in some rare
situations. This commit fixes that behaviour for the check methods by
dereferencing the instance variable to a final local variable to ensure that if
it is not null at the null guard, that it will be not null after that point
also to successfully close/abort the stream
Signed-off-by: Peter Ansell <[email protected]>
commit b4a82de6a65fabc1cbbb57988fce046dd87b7e1e
Author: Peter Ansell <[email protected]>
Date: 2016-09-13T01:09:37Z
HTTPCLIENT-1767: Check parameters for null in ResponseEntityProxy
In some rare cases, null parameters are sent to ReponseEntityProxy methods,
this adds checks on those to ensure that the connections are still released,
but the null variable is not dereferenced.
Signed-off-by: Peter Ansell <[email protected]>
----
> 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]
