Eric, Just to clarify things: there's <strong>no way</strong> we touch 2.0 branch for any other reason but fixing a <strong>serious</strong> bug.
As far as I am concerned the suggested patch is an enhancement, and not a bug. If any of my comments were interpreted otherwise, I offer my apologies. It was not meant that way. Anyways, if I understand Christian right, the idea is to drop those connections that are known for sure to be screwy, instead of reusing them and getting into a trouble later when processing subsequent responses. The logic in readStatusLine will be enhanced to optionally terminate the infinite loop. That is it. I hope that addresses your concerns somewhat. Oleg -----Original Message----- From: Eric Johnson [mailto:[EMAIL PROTECTED] Sent: Tuesday, November 11, 2003 15:35 To: Commons HttpClient Project Subject: Re: DO NOT REPLY [Bug 24560] - HttpClient loops endlessly while trying to retrieve status line Christian Kohlschütter wrote: >I perfectly agree - I do not see a bullet-proof solution either. > >I should correct my assumption: > >"Can we assume that reusing the HTTP connection is unreliable/should be >avoided if there are more bytes *INSTANTLY* available than specified with >Content-Length" > >"Instantly" means without waiting/blocking, so at least for this situation, a >simple workaround would be feasible. > >I think that the currently used SocketInputStream's available() method _does_ >return values > 0. > > Unfortunately, I think that depends. I seem to recall we had difficulties with this function in the past, particularly related to different JVM versions, and also with different implementations of secure sockets. Granted, some of those implementations were/are buggy, but we have to live with them, I think. Before we commit such a change to the 2.0 release branch, we'd have to run it through tests across numerous JVMs on numerous platforms with numerous JCE libraries. We also run the risk that the "available" function could misbehave not only by giving an incorrect response, but also by blocking for a short period of time (1ms?), which would be disastrous for performance. I think the "instantly" available criteria is misleading, too. There's absolutely no reason to prevent you from hitting a pathological case where the packet boundary splits right where the extra data is sent, thus leading the "instantly" available check to return false, even though the data would be read on the subsequent response. In fact, such behavior could be entirely dependent on the misbehaving server. The case that I've encountered stemmed from a server that tossed in an extra CRLF after a previous response. As I recall, the extra CRLF were, not surprisingly, written to the socket as a separate chunk, and therefore stood a good a chance as any of being received by the client as part of a separate packet, and therefore would not be "instantly" available anyway. In the end, I've got a few concerns: * I'm not sure I see the point of trying to catch the problem at the end of the previous response, which would seem to be the point of the "available" check, rather than at the beginning of reading the next response. * Are there particular servers that demonstrate bad behavior that we want to catch with this new option? Do we have test cases for those particular servers? * Has it been tested across a myriad of environments? Such changes for the 2.1 CVS HEAD are fine by me. I'm much more concerned about the 2.0 branch, however, and keeping what changes we do there to a minimum. This change seems to straddle the boundary between a bug-fix and additional functionality, at least from where I sit. Then again, I've not looked closely at the patch, I've only followed the discussion. -Eric. --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
