273978 - Plugin Finder dialog hangs with mime type including plus

Status: RESOLVED FIXED

(Toolkit Graveyard :: Plugin Finder Service, defect)

Description

[Phil Ringnalda (:philor)]

Split off from bug 273950 where the real mime type was image/svg+xml - with a
mime type that includes a plus (or just type="+"), the pfs wizard sends
something, receives some reply, but then just hangs. Offhand, seems likely to be
a confusion between +, space, and maybe %2B somewhere along the line.

Comment 1

[Phil Ringnalda (:philor)]

Attachment: testcase type="foo/bar+baz"

Comment 2

[Phil Ringnalda (:philor)]

JS Console:
Error: aPluginInfo has no properties
Source File: chrome://mozapps/content/plugins/pluginInstallerWizard.js
Line: 102

102: this.mPluginNotFoundArray[aPluginInfo.requestedMimetype] = new
PluginInfo(aPluginInfo);

Doesn't that if/else need to handle three cases? 
aPluginInfo.pid != -1 // found
aPluginInfo.pid == -1 // not found
aPluginInfo == null // hosed

Comment 3

[Doron Rosenberg (IBM)]

The code that passes in aPluginInfo should make sure its not null, the bug lies
there.

Comment 4

[Jesse Ruderman]

Even if it's true that the caller shouldn't pass null, the code in
pluginInfoReceived is clearly wrong: it null-checks the parameter but
immediately "dereferences" it in both branches.  The null check should either be
fixed or removed.

onDatasourceError at
http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js#104
contains the line gPluginInstaller.pluginInfoReceived(null).  If Doron is right,
that line is completely wrong.

Why does a "+" in the mime type result in onDatasourceError being called?

Comment 5

[Doron Rosenberg (IBM)]

The code is a bit of a mess due to last minute aviary changes that went in.  You
are right, the null check should be in both branches.

Comment 6

[Phil Ringnalda (:philor)]

Ah, Firefox requests
https://pfs.mozilla.org/plugins/PluginFinderService.php?mimetype=foo/bar+baz&appID=(yadda-yadda)
 and PHP helpfully sets $_GET["mimetype"] to "foo/bar baz" assuming it's a query
string with spaces encoded as plusses. Too late to have 1.0 send
mimetype=foo/bar%2Bbaz, but can the PHP just str_replace(" ", "+",
$_GET["mimetype"])? Surely any incoming mimetype with an actual space in it has
more trouble already than we'll cause by replacing it with a plus.

Comment 7

[Doron Rosenberg (IBM)]

the code at
http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js#27
should be urlencoding the mimetype before appending it to the URL.

Comment 8

[Doron Rosenberg (IBM)]

Attachment: patch.

Comment 9

[Doron Rosenberg (IBM)]

checked in.

Comment 10

[Doron Rosenberg (IBM)]

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