246280 - Empty Open with default application should not be available

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[timeless]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040609

Steps.
1. click url link
2. look at dialog

Expected results:
no option to open with default application
(i guess)

Actual results:
option to open with default application

Comment 1

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

Attachment: patch

something like this, maybe...

Comment 2

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

Attachment: patch v2

this is slightly better

Comment 3

[Darin Fisher]

Comment on attachment 151304 [details] [diff] [review]
patch v2

should we maybe try to get this on the 1.7 branch?

Comment 4

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

Comment on attachment 151304 [details] [diff] [review]
patch v2

yes, I think this should go onto the 1.7 branch. I'll request approval once I
have sr.

Comment 5

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

(this bug may also fix bug 246267, by making sure that the temp file name does
not end in a dot)

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 151304 [details] [diff] [review]
patch v2

       url->GetFileExtension(aExtension);
+      // Windows ignores terminating dots. So we have to as well, so
+      // that our security checks do "the right thing"
+      // In case the extension consisted only of the dot, the code below will
+      // extract an extension from the filename
+      if (!aExtension.IsEmpty() && aExtension.Last() == '.')
+	 aExtension.Truncate(aExtension.Length() - 1);
+    }

Seems to me like windows ignores any number of terminating '.'s, so this code
should do the same, no? Same thing in the last chunk of this diff.

sr=jst with that fixed.

Comment 7

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

Attachment: patch v3

good point. fortunately, nsCString has this friendly Trim method, which this
patch uses :)
(with the parameter set to only trim at the end)

Comment 8

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

Attachment: patch v4

let's try using the right variable

Comment 9

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

v4 checked in
Checking in nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <-- 
nsExternalHelperAppService.cpp
new revision: 1.264; previous revision: 1.263
done

Comment 10

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

Comment on attachment 152464 [details] [diff] [review]
patch v4

This patch trims trailing dots before extension checking; thus treating .exe.
files the same as .exe files: Disallow "open with application" in the helper
app dialog, and makes "launch file" from the progress dialog/download manager
show a warning.


(previous versions of this patch have the r and sr marked)

Comment 11

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

fixed on trunk

Comment 12

[Jesse Ruderman]

*** Bug 246184 has been marked as a duplicate of this bug. ***

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 152464 [details] [diff] [review]
patch v4

a=asa for checkin to aviary and 1.7.x branch.

Comment 14

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

(I'm assuming the comment is correct, rather than patch flags)

fixed on 1.7 and aviary branch.