287603 - Enable WPS URLObjects on the commandline

Status: RESOLVED FIXED

(Core Graveyard :: Cmd-line Features, enhancement)

Description

[Rich Walsh]

This enhancement allows Mozilla et al. to properly handle WPS URLObjects
specified on the commandline.  Currently, moz apps display the URL they contain;
 this patch will cause them to load the URL instead.  When implemented, users
will finally be able to open Url objects by dropping them on  a Moz or Firefox
program object.

There are two parts to this patch.  The first adds an OS/2 version of
ReadUrlFile() to nsFileProtocolHandler.cpp.  Before Moz reads any datafile, it
calls this method to determine if the file contains a URI.  If so, the method
reads the file and returns the URI which Moz then loads.

The second part of the patch adds some infrastructure to nsLocalFileOS2 to
identify URL objects.  nsLocalFileOS2.idl describes a new interface with two
methods:
- GetFileTypes() returns a scriptable array of filetype strings;  it is not used
currently.
- IsFileType() takes a filetype string and returns true if it matches one of the
file's types;  ReadUrlFile() uses this method.

nsLocalFileOS2.cpp implements these methods along with a private method: 
GetEA().  This is a fairly generic method for fetching any single extended
attribute.  If there's any need, this could be made public and be added to the
new interface.

Comment 1

[Rich Walsh]

Attachment: new interface - nsILocalFileOS2.idl

Comment 2

[Rich Walsh]

Attachment: update to nsFileProtocolHandler.cpp

Comment 3

[Rich Walsh]

Attachment: updates to files in xpcom/io

Comment 4

[Mike Kaply [:mkaply]]

This is cool stuff.

Any reason not to keep the #ifdefs in ReadURLFile?

Comment 5

[Rich Walsh]

(In reply to comment #4)
> Any reason not to keep the #ifdefs in ReadURLFile?

They're all still there, just structured differently.  When Win was the only
platform implementing this, an all-in-one #ifdef worked.  Now it's #ifdef
Win/OS2/other.  The Win-specific #ifdef's are only evaluated within its scope.
If you prefer, I can put OS/2 first & leave the existing stuff alone.

Comment 6

[Peter Weilbacher]

The patch works great here.

After looking at the patched files I also finally understood the new #ifdefs
and they seem OK to me.

>+    PRBool isUrl;
>+    rv = os2File->IsFileType(NS_LITERAL_CSTRING("UniformResourceLocator"),
>+                             &isUrl);

What about files with type "URL" or "WebExplorer_Url"? At the moment you only 
test for "UniformResourceLocator", but I have several (old and new) files with
the other types, although I am not sure which program creates the "URL" ones.

>+    // get a buffer, read the entire file, then create
>+    // an nsURI;  we assume the string is already escaped
>+    char * buffer = (char*)NS_Alloc(fileSize+1);
>+    if (buffer) {
>+        PRInt32 cnt = PR_Read(file, buffer, fileSize);

What happens if the file is very large (and does not just contain a URI)?
Perhaps you should test if the file size is less than some typical URI length
before doing the NS_Alloc? Hmm, I cannot find any restriction of length in the
RFCs... Some of the longest URLs I have seen are from Bugzilla queries and they
are less than 1000 chars, so a limit to 10000 chars should be safe.

Comment 7

[Rich Walsh]

(In reply to comment #6)
> What about files with type "URL" or "WebExplorer_Url"? At the moment you only
> test for "UniformResourceLocator", but I have several (old and new) files
> with the other types, although I am not sure which program creates the "URL"
> ones.

I haven't seen a "WebExplorer_Url" in years. And "URL"??  Anyone who writes a
program like that should get his act together and fix it - IMHO, of course.  If
it's really, _really_ necessary (and desirable), I could add this.  However,
note that no one has ever complained that you can't *drop* these on a Moz app. 
If there were really a demand for this, we'd have heard about it by now.

> What happens if the file is very large (and does not just contain a URI)?
> Perhaps you should test if the file size is less than some typical URI length
> before doing the NS_Alloc? [...] a limit to 10000 chars should be safe.

That's not unreasonable.  Actually, I'd be inclined to make the limit 4096.

Comment 8

[Rich Walsh]

FWIW...  This patch has an interesting side-effect.  Every time the browser
reads local content (e.g. ua.css), it calls ReadURLFile().  Because of this, any
such file can be relocated and replaced with a URL object.

For example, I moved bin/res/builtin/platformHTMLBindings.xml to my Desktop and
replaced it with a URL object pointing to its new location.  To test, I added a
new key binding to the file on my Desktop, then started Fx.  Sure enough, my
change was in effect (Ctrl-Q to Copy).

I have no idea if this has any practical benefit (or security implication) but I
thought it interesting enough to mention.  Although I haven't tested it, the
same effect should already be present in the Win version.

If you want to see what files are affected, add this toward the top of the OS/2
version of nsFileProtocolHandler::ReadURLFile():
=====
        return NS_ERROR_NOT_AVAILABLE;

+   static int ctr = 0;
+   nsCAutoString path;
+   os2File->GetNativePath(path);
+   printf( "ReadURLFile - ctr= %d  path= '%s'\n", ++ctr, path.get());
+
    // see if this file is a WPS UrlObject
=====

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

> Although I haven't tested it, the
> same effect should already be present in the Win version.

nah, on windows, those files need to end in ".url"

Comment 10

[Mike Kaply [:mkaply]]

Where is GetFileTypes called from?

Comment 11

[Mike Kaply [:mkaply]]

Never mind - r/sr=mkaply, a=mkaply. I'll check this in.

Comment 12

[Mike Kaply [:mkaply]]

Fix checked in to trunk.