You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@river.apache.org by Peter Jones <pe...@sun.com> on 2008/05/01 07:52:40 UTC
Re: [jira] Resolved: (RIVER-9) PreferredClassLoader should try to
avoid making a direct check against the first URL
On Sun, Feb 10, 2008 at 04:23:07AM -0800, Mark Brouwer (JIRA) wrote:
>
> [ https://issues.apache.org/jira/browse/RIVER-9?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> Mark Brouwer resolved RIVER-9.
> ------------------------------
>
> Resolution: Fixed
>
> Applied code modification, given no objection I assume people have reviewed but forgot to mention they liked what they saw.
Sorry for the belated review. Changes look fine, with one minor
quibble about this comment change:
- * causing a duplicate request of the JAR file on first
- * use. (For HTTP-protocol URLs, the initial request will
- * use HEAD instead of GET.)
+ * causing duplicate requests of the JAR file on first use when
+ * our first attempt fails. (For HTTP-protocol URLs, the initial
+ * request will use HEAD instead of GET.)
With these changes, it will not be the "initial" request that uses
HEAD instead of GET-- rather, it would just be the fallback request to
determine existence of the JAR file if the initial request fails.
Like I said here, however:
http://mail-archives.apache.org/mod_mbox/incubator-river-dev/200701.mbox/%3c20070116212605.GP20922@east%3e
I still can't completely explain why the separate initial check for
the existence of the JAR file is even necessary. There was some
discussion elsewhere in that river-dev thread that the problem was
about not being able to distinguish FileNotFoundException occurring
because the JAR file definitely does not exist (like after HTTP
response 404) from FNFE occurring because the JAR file exists but
contains no META-INF/PREFERRED.LIST entry-- but I don't see how that
could be the issue here, because while an FNFE may be an ambiguous
failure, PreferredClassLoader.isPreferredResource just wants to return
false in either case anyway.
I thought that it had something to do with the "jar:" URL handler, in
some circumstances at least, not necessarily throwing
FileNotFoundException, but instead some other IOException, in the case
of an HTTP 404 response-- hence the HTTP response code analysis in
PCL.getPreferredConnection. But I can't say sure what relevant
circumstances would cause that. A basic test I just ran against a
range of JDK versions shows that attempting to retrieve contents of a
non-existent JAR file through a "jar:" URL does throw a non-FNFE
IOException in 1.2.x, but it does seem to throw a FNFE in 1.3 and
later, and I would not have thought that PreferredClassLoader's
initial implementation was intended to accommodate running on JDK
1.2.x. But there are very likely cases that I'm not thinking of...
The other option that I can think of is that PreferredClassLoader was
trying to be careful to only consider 4xx HTTP responses to indicate
that the JAR file definitely does not exist, not 5xx HTTP responses,
which are logically less definitive, but still cause
FileNotFoundException. This seems like a plausibile explanation, but
it doesn't ring a bell with me (Bob?).
At any rate, even though I can't completely explain the need for the
separate existence check, I wouldn't advocate removing it altogether
without much more consideration. In the meantime, your changes seem
reasonable to me.
-- Peter
Re: [jira] Resolved: (RIVER-9) PreferredClassLoader should try to
avoid making a direct check against the first URL
Posted by Peter Jones <pe...@sun.com>.
On Mon, May 19, 2008 at 08:54:10PM +0200, Mark Brouwer wrote:
> Peter Jones wrote:
>> I finally uncovered an old (early 2001) email that was the code review
>> request for the change which added PreferredClassLoader's extra
>> retrieval of the first JAR file without using the "jar:" URL handler--
>> but the explanation it contains does not make sense to me. I'm now
>> inclined to think that we should just remove the extra retrieval,
>> simplifying this rather confusing code (and obviating the original
>> need for the RIVER-9 change).
>>
>
> Hi Peter, Bob,
>
> Sorry for the late response (was on holidays for while) and I'm having a
> lot of problems lately with regard to my cheiron.org mail adres.
>
> I would like to leave the code as it is for now. If the 'intriging' code
> is really not necessary it would be a good thing to simplify, but one
> could argue whether the spec for the JarURLConnection methods that throw
> IOException is enough to assume FileNotFoundException will be thrown in
> case a connection can be established and it can be determined the file
> doesn't exist.
>
> The implementations might seem to do the right thing, but as they don't
> specify when FileNotFoundException is thrown I think there is also no
> harm in the way the current codebase deals with it.
The HttpURLConnection.getErrorStream spec mentions the 404->FNFE
behavior in passing:
Returns the error stream if the connection failed but the server
sent useful data nonetheless. The typical example is when an HTTP
server responds with a 404, which will cause a
FileNotFoundException to be thrown in connect, but the server sent
an HTML help page with suggestions as to what to do.
One might read this "will cause" statement as a spec requirement, but
it is admittedly indirect in its placement and unclear in its coverage.
I'm OK with leaving the code as it is (with your RIVER-9 change) for
now-- it seems like the lower risk choice-- but I think that it would
be valuable to revise the code comments to clarify that the
PreferredClassLoader implementation is making this extra effort to
cover itself in the case of an imagined JarURLConnection
implementation behavior that is not believed to actually exist.
(Otherwise someone studying the code might reasonably assume that
there was a motivating case in practice and struggle to understand
what it could be, before discovering this mailing list thread.)
> One might create an issue for this, however.
Yup. I'll file one.
-- Peter
Re: [jira] Resolved: (RIVER-9) PreferredClassLoader should try to
avoid making a direct check against the first URL
Posted by Mark Brouwer <ma...@marbro.org>.
Peter Jones wrote:
> I finally uncovered an old (early 2001) email that was the code review
> request for the change which added PreferredClassLoader's extra
> retrieval of the first JAR file without using the "jar:" URL handler--
> but the explanation it contains does not make sense to me. I'm now
> inclined to think that we should just remove the extra retrieval,
> simplifying this rather confusing code (and obviating the original
> need for the RIVER-9 change).
>
Hi Peter, Bob,
Sorry for the late response (was on holidays for while) and I'm having a
lot of problems lately with regard to my cheiron.org mail adres.
I would like to leave the code as it is for now. If the 'intriging' code
is really not necessary it would be a good thing to simplify, but one
could argue whether the spec for the JarURLConnection methods that throw
IOException is enough to assume FileNotFoundException will be thrown in
case a connection can be established and it can be determined the file
doesn't exist.
The implementations might seem to do the right thing, but as they don't
specify when FileNotFoundException is thrown I think there is also no
harm in the way the current codebase deals with it.
One might create an issue for this, however.
Regards,
--
Mark Brouwer
Re: [jira] Resolved: (RIVER-9) PreferredClassLoader should try to
avoid making a direct check against the first URL
Posted by Peter Jones <pe...@sun.com>.
I finally uncovered an old (early 2001) email that was the code review
request for the change which added PreferredClassLoader's extra
retrieval of the first JAR file without using the "jar:" URL handler--
but the explanation it contains does not make sense to me. I'm now
inclined to think that we should just remove the extra retrieval,
simplifying this rather confusing code (and obviating the original
need for the RIVER-9 change).
-- Peter
Re: [jira] Resolved: (RIVER-9) PreferredClassLoader should try to
avoid making a direct check against the first URL
Posted by Peter Jones <pe...@sun.com>.
On Thu, May 01, 2008 at 05:28:45PM -0400, Bob Scheifler wrote:
> Peter Jones wrote:
>> The other option that I can think of is that PreferredClassLoader was
>> trying to be careful to only consider 4xx HTTP responses to indicate
>> that the JAR file definitely does not exist, not 5xx HTTP responses,
>> which are logically less definitive, but still cause
>> FileNotFoundException.
Oops, I had gotten confused by looking at too many (old) JDK versions
yesterday-- the above (FNFE thrown for any response code 400 or above)
was true through 1.3.1, but since 1.4 and the fix for 4160499, a FNFE
is thrown only if the response code is 404 or 410, otherwise a generic
IOException is thrown, consistent with what Bob says below.
>> This seems like a plausibile explanation, but it doesn't ring a
>> bell with me (Bob?).
>
> The only difference I can find is that PreferredClassLoader treats
> 403 (FORBIDDEN) as definitely does not exist, whereas HttpURLConnection
> throws an IOException for it. Doesn't seem like we would have really
> gone to that much trouble for that one case, though.
Agreed.
-- Peter
Re: [jira] Resolved: (RIVER-9) PreferredClassLoader should try to
avoid making a direct check against the first URL
Posted by Bob Scheifler <Bo...@Sun.COM>.
Peter Jones wrote:
> The other option that I can think of is that PreferredClassLoader was
> trying to be careful to only consider 4xx HTTP responses to indicate
> that the JAR file definitely does not exist, not 5xx HTTP responses,
> which are logically less definitive, but still cause
> FileNotFoundException. This seems like a plausibile explanation, but
> it doesn't ring a bell with me (Bob?).
The only difference I can find is that PreferredClassLoader treats
403 (FORBIDDEN) as definitely does not exist, whereas HttpURLConnection
throws an IOException for it. Doesn't seem like we would have really
gone to that much trouble for that one case, though.
- Bob