[
https://issues.apache.org/jira/browse/HTTPCLIENT-951?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877822#action_12877822
]
Thierry Guérin commented on HTTPCLIENT-951:
-------------------------------------------
> At the moment there are no plans for 4.0.2. I do not see this issue as severe
> enough to warrant a bug fix release but am willing to reconsider
ok. As for me I find it a pretty severe bug, because the workaround is to use
ByteArrayInputStreams, and this would quickly be a memory hog, but then again,
as no one reported this bug before, I guess not many people use the same
scenario as me :)
>> * InputStreamEntity does not comply with this (isStreaming still returns
>> !consumed whereas it should always return true)
> I am pretty sure this has been changed in SVN trunk. Just pull the latest
> code snapshot.
my bad, didn't update before diffing. getContent is still broken though ;)
>> * there is no way now to test whether a stream has been consumed or not.
>The trouble is we cannot add methods to the existing interfaces including
>HttpEntity as long as we want to remain 4.x API compatible. Another
>problem is that there is simply no reliable way of knowing whether an entity
>has been really consumed without wrapping the underlying
>content stream, which is something I would like to avoid.
>I think this is the job of the request wrapper object to maintain the
>conversational state of the current request and to know whether the
>enclosed entity has been written out to the socket or not. I believe it should
>be possible to resolve the problem just by tweaking
>EntityEnclosingRequestWrapper class.
Well, seeing that EntityEnclosingRequestWrapper has no way of knowing what's
done with the entity once getEntity has been called, I don't really see how you
would know the enclosed entity has been written out to the socket without
wrapping the entity itself and overriding writeTo (other than using events but
I don't think you want to go there).
The solution may lie in RequestWrapper.getExecCount, which only seems targeted
at knowing if a request can be repeated (and is the source of the bug in
DefaultRequestDirector.tryExecute: "wrapper.getExecCount() > 1 &&
!wrapper.isRepeatable()"). From what I understand, a non repeatable
entityEnclosingRequest can be repeated as long as its underlying entity hasn't
been consumed; Is this correct? If so, then getExecCount is broken because its
sole purpose is to know if an entityEnclosingRequest can be repeated, and it
doesn't reflect the entity's status.
What do you think?
>That is why it is super-critical that we have a test case for the problem.
Here they are (testClientAuthentication.diff). Unfortunately, the
testBasicAuthenticationSuccessOnNonRepeatablePutExpectContinue() test can't
pass even if the bug is fixed, because LocalTestServer sends a 100 (instead of
a 401) during the handshake when it receives a header with Expect: Continue. I
don't feel confident enough with the code to try and fix that, sorry.
The second test
(testBasicAuthenticationFailureOnNonRepeatablePutDontExpectContinue()) is
somewhat redundant with testBasicAuthenticationFailureOnNonRepeatablePost(),
but as it explicitly specifies
setBooleanParameter(CoreProtocolPNames.USE_EXPECT_CONTINUE, false), I find it
useful. Feel free to discard it.
Thierry
> Incorrect handling of InputStreams when connecting to a server that requires
> authentication
> -------------------------------------------------------------------------------------------
>
> Key: HTTPCLIENT-951
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-951
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Components: HttpClient
> Affects Versions: 4.0 Final, 4.0.1, 4.1 Alpha1, 4.1 Alpha2
> Environment: Windows XP, Java 1.6.20
> Reporter: Thierry Guérin
> Fix For: 4.1 Alpha3
>
> Attachments: httpClient.diff, httpClientTrunk.diff, httpCore.diff,
> httpCoreTrunk.Diff, testClientAuthentication.diff
>
>
> I'm trying to upload a file to a WebDav server (mod_dav on Apache Web Server
> 2.2.14) that has basic (or digest, the result is the same) authentication
> enabled.
> I'm using the following code:
> String url = "http://myserver/dir/test2.gif";;
> File file = new File("d:/test2.gif");
> DefaultHttpClient httpClient = new DefaultHttpClient();
> HttpPut put = new HttpPut(url);
> put.setEntity(new InputStreamEntity(new FileInputStream(file),
> file.length()));
>
> URI uri = put.getURI();
> httpClient.getCredentialsProvider().setCredentials(new
> AuthScope(uri.getHost(), uri.getPort()),
> getCredentials());
>
> put.getParams().setBooleanParameter(CoreProtocolPNames.USE_EXPECT_CONTINUE,
> true);
> HttpResponse response = httpClient.execute(put);
> System.out.println(response.getStatusLine());
> When running the above code, I'm getting a
> org.apache.http.client.NonRepeatableRequestException: Cannot retry request
> with a non-repeatable request entity. I tested both the latest alpha & the
> svn head. Doing the same thing in HttpClient 3.1 worked as expected.
> This could be normal, as I'm using an InputStream that is indeed not
> repeatable, but as I'm also using Expect: 100-Continue, the stream shouldn't
> have been consumed with the first connection (the one that gets a code 401
> from the WebDav server), and only in the second one, when the credentials are
> provided.
> The problem is that DefaultRequestDirector.execute doesn't take this into
> account and assumes that if a request has been tried once, its associated
> entity (if any) has been consumed.
> Here's the fix that I came up with:
> Change DefaultRequestDirector.execute so that if the wrapper is an
> EntityEnclosingRequestWrapper, it checks if the entity has actually been
> consumed before throwing a NonRepeatableRequestException. I'm using the
> method isStreaming() from HttpEntity, as it's the closest thing to what I was
> looking for. Reading the JavaDoc, it could lead to the situation where an
> entity has started streaming but has not yet finished, and so is not in a
> state where it can be used. However I don't think that's a problem as the
> javadoc for HttpEntity.getContent() states that it can't be called two times
> on a non-repeatable entity, so it's just a matter of when the request will
> fail.
> This lead me to also modify InputStreamEntity (from the httpCore project) as
> it didn't comply with the javadoc. With these two modifications, The file
> upload completes successfully.
> I also modified:
> * TestInputStreamEntity.testBasics() (from the httpCore project) test so
> that it complies with getContent()'s Javadoc.
> * TestDefaultClientRequestDirector.FaultyHttpRequestExecutor because it
> didn't consume the entity's content.
> All the tests from both httpCore and httpClient pass.
> I tested both InputStreamEntity and BasicHttpEntity.
>
> Please keep in mind that I am by no means an httpClient (or http, for that
> matter) expert, and these modifications may have some unexpected side-effects
> that I did not foresee, contain plain dumb code, or whatever, so it would be
> great if someone could review my changes and give their opinion.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
![http://myserver/dir/test2.gif"](http://myserver/dir/test2.gif"){kind=link}