If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Camino1.6

Status

Camino Graveyard
Downloading
--
critical
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Chris Mason, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.12})

unspecified
Camino1.6
PowerPC
Mac OS X
fixed1.8.1.12

Details

(Whiteboard: [camino-1.5.5])

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
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 - chris@chrismason.com
3.



Opened Camino and opened specific web page.
(Reporter)

Comment 1

10 years ago
Created attachment 294356 [details]
url file sent from VERY RELIABLE source. Works OK with Safari, however NOT with Camino
(Assignee)

Comment 2

10 years ago
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.
Assignee: nobody → stuart.morgan
Group: security
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

10 years ago
Created attachment 294369 [details] [diff] [review]
fix
Attachment #294369 - Flags: superreview?(mark)
(Assignee)

Updated

10 years ago
Summary: Failure of download to open and then redownloading and again failing to open, carries on forever → Opening a .url file with auto-opening of downloads enabled can cause an infinite loop

Comment 4

10 years ago
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.
Attachment #294369 - Flags: superreview?(mark) → superreview+
Flags: camino1.5.5?
(Assignee)

Comment 5

10 years ago
Landed on trunk and MOZILLA_1_8_BRANCH.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.1.12
Resolution: --- → FIXED
Target Milestone: --- → Camino1.6
Created attachment 299928 [details] [diff] [review]
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).
Attachment 299928 [details] [diff] checked in on the CAMINO_1_5_BRANCH.
Flags: camino1.5.5? → camino1.5.5+
Whiteboard: [camino-1.5.5]
You need to log in before you can comment on or make changes to this bug.