409538 - Opening a .url file with auto-opening of downloads enabled can cause an infinite loop

Status: RESOLVED FIXED

(Camino Graveyard :: Downloading, defect)

Description

[Chris Mason]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/523.12.1 (KHTML, like Gecko) Version/3.0.4 Safari/523.12.2
Build Identifier: Version 2007112812 (1.5.4)

Camino set as default browser. Clicking on a url sent as a Apple Mail attachment. Failure of download to open the page in Camino and then redownloading and again failing to open, carries on forever and ever. Screen just fills up with an ever increasing number of .url files.

OSX 10.4.11
Mail Version 2.1.2 (753)

Reproducible: Always

Steps to Reproduce:
1.Does not occur in Safari - page opens fine.
2.I can email the specific .url attachment that cause the problem - [e-mail]
3.



Opened Camino and opened specific web page.

Comment 1

[Chris Mason]

Attachment: url file sent from VERY RELIABLE source. Works OK with Safari, however NOT with Camino

Comment 2

[Stuart Morgan]

Not a security issue, so unhiding.

The issue here is that for reasons that I don't understand, in certain cases (including opening from our manager) we are getting getting opens of the .url file not as a file open, but a GetURL, and our GetURL handler doesn't round-trip though our file decoder (giving us behavior like 363645.

The only reason this isn't just a minor bug is that in conjunction with auto-opening of downloaded files, it gets into a nasty loop.

Comment 3

[Stuart Morgan]

Attachment: fix

Comment 4

[Mark Mentovai]

Comment on attachment 294369 [details] [diff] [review]
fix

My only comments apply to existing stuff:

>+// This takes an NSURL to a local file, and if that file is a file that
>+// represents a URL, we return the URL it contains. Otherwise, we return the URL
>+// we originally got. Supports .url, .webloc and .ftploc files.

Check agreement: if it starts out with "this," it shouldn't switch to "we."

>+  OSType fileType = NSHFSTypeCodeFromFileType(NSHFSTypeOfFile(urlPathString));

We could just leave this as an NSString* and do string comparisons, but I guess there are advantages to the single conversion and simple integer comparisons.

>+  else if ([ext isEqualToString:@"webloc"] || [ext isEqualToString:@"ftploc"] ||
>+           fileType == 'ilht' || fileType == 'ilft')
>+    url = [NSURL URLFromInetloc:urlPathString];

Optional, but I like to {brace} the conditioned clause when the condition itself spills onto multiple lines.  If you brace the else, you should also brace the if so that everything in the if-else chain matches.

Comment 5

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: 1_5 branch patch

+ (NSURL*)decodeLocalFileURL:(NSURL*)url had moved around a bit between 1_5 branching and this bug, but I tracked it down.  

Just posting this patch for completeness (since it has that change and smorgan's checkin changes for mento's comment 4).

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment 299928 [details] [diff] checked in on the CAMINO_1_5_BRANCH.