[
https://issues.apache.org/jira/browse/HTTPCLIENT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15488728#comment-15488728
]
ASF GitHub Bot commented on HTTPCLIENT-1767:
--------------------------------------------
Github user ansell commented on the issue:
https://github.com/apache/httpclient/pull/61
Thanks for patching this so promptly! The commit you applied looks correct.
> 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
> Fix For: 4.5.3, 5.0 Alpha2
>
>
> 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]
