This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 64012 - URLMapper.findURL(fo, NETWORK) returns file URL
Summary: URLMapper.findURL(fo, NETWORK) returns file URL
Status: RESOLVED FIXED
Alias: None
Product: ide
Classification: Unclassified
Component: Internal Server (show other bugs)
Version: 5.x
Hardware: All All
: P3 blocker (vote)
Assignee: Petr Pisl
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-09-09 22:10 UTC by Rich Unger
Modified: 2006-06-05 00:36 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
patch (1.74 KB, patch)
2005-09-12 20:35 UTC, Rich Unger
Details | Diff
MasterURLMapperTest.java (1.54 KB, text/plain)
2005-09-21 14:43 UTC, rmatous
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rich Unger 2005-09-09 22:10:20 UTC
For a FileObject in an open project, URLMapper(fo, URLMapper.NETWORK) is
returning a file:// URL.
Comment 1 Rich Unger 2005-09-10 00:16:52 UTC
originally found in 4.1, but I've just confirmed it's still a bug in 5.0 build
200509051800
Comment 2 Rich Unger 2005-09-12 20:35:58 UTC
Created attachment 24713 [details]
patch
Comment 3 Rich Unger 2005-09-12 20:41:53 UTC
I made a patch.  There are 2 problems here:

The URLMapper in openide/masterfs (MasterURLMapper) never checks the URL type
argument, so it will return a file URL, despite the fact that a NETWORK url was
requested.

The HttpServerURLMapper contains a puzzling line, with the comment:
"if the file is on the localhost, don't return URL with HTTP"

This seems to me to be incorrect.  This mapper returns immediately if the type
is not NETWORK.  So, it should only be concerned with return http URLs, whether
or not the file is on localhost.  Of course, the string "localhost" should be
replaced by the host name (which it is, at least in my testing).

So, I just removed that line, and it worked fine.
Comment 4 _ rkubacki 2005-09-20 10:04:10 UTC
Martin, please can you evaluate this? 
Comment 5 Petr Jiricka 2005-09-20 10:51:25 UTC
Looking at the CVS log, I see that the controversial line was put in to fix
issue 39072 (by Petr Pisl), so we have to be careful here. Petre, can you please
evaluate?

Radku, can you please look at the MasterFS part of this? 

Thanks.

Comment 6 Petr Jiricka 2005-09-20 10:57:40 UTC
BTW, unit tests would be useful.
Comment 7 Rich Unger 2005-09-20 20:01:29 UTC
Issue 39072 seems to ignore the semantic that if the client asks for
type=NETWORK, they want an http URL.
Comment 8 Jesse Glick 2005-09-21 05:38:31 UTC
Agreed, file: should not be returned for NETWORK. It is fine for EXTERNAL.
(Whether http://localhost:8082/etc is acceptable for NETWORK is another
question, but we cannot control what the local host name is set to.)
Comment 9 rmatous 2005-09-21 08:56:55 UTC
I agree with MasterFS part of the patch.
Comment 10 Petr Pisl 2005-09-21 12:13:57 UTC
I agree with the patch. And I also agree with Petr Jiricka, that unit tests
would be useful.
Comment 11 rmatous 2005-09-21 14:43:12 UTC
Created attachment 25031 [details]
MasterURLMapperTest.java
Comment 12 Rich Unger 2005-10-13 22:26:59 UTC
I'd feel a lot better if we could target this for 5.0.  I really need it for
V-Builder, and it just needs to be checked in!
Comment 13 Martin Grebac 2005-10-19 09:10:17 UTC
Seems like everybody agrees on a fix, so is anyone going to commit the patch
into cvs?
Comment 14 Rich Unger 2005-10-26 21:10:59 UTC
I checked in the httpserver portion.  I do not have access to MasterFS.
Comment 15 rmatous 2005-11-21 08:45:10 UTC
/cvs/openide/masterfs/src/org/netbeans/modules/masterfs/MasterURLMapper.java,v<--
 new revision: 1.12; previous revision: 1.11

/cvs/openide/masterfs/test/unit/src/org/netbeans/modules/masterfs/MasterURLMapperTest.java,v
 initial revision: 1.1
Comment 16 Petr Blaha 2006-03-13 13:44:36 UTC
TM 5.0 -> TBD
Comment 17 Rich Unger 2006-03-13 17:49:57 UTC
TBD?  Can we set this for the next release please?!  I made the patch in plenty
of time for 5.0, there was no dissent, I checked in the half that I had access
to in CVS.  What's the holdup here?  It's a very simple fix, and without the fix
the documented API's assumptions are violated.
Comment 18 Petr Pisl 2006-03-14 10:16:35 UTC
The code was already rewriten. So it should work. I tried it on release55 branch. 

I close the issue as fixed. Feel free to reopen the issue, if it doesn't work
for you.