173069 - Default Plug-ins receive relative URLS

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[TaylorToddK]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0

Default plug-ins can receive relative URLS for PLUGINURL and PLUGINPAGE
attributes.  This means the plug-in would have to resolve the URL itself. 
Plug-ins are not initialized with the URL they are embedded in and can receive
both absolute and relative URLS.  It would beneficial if the plug-in manager
resolved all relative URLs to absolute ones, so plug-ins only have to handle one
scenario.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[rubydoo123]

handing over to Serge for plug-in manager work

Comment 2

[Peter Lubczynski]

plug-in manager work???

The fix-up of relative URLs for PLUGINSPAGE/PLUGINURL can probably happen on the
array we pass the default plugin right before we call NPP_New.

Comment 3

[Peter Lubczynski]

I think CODEBASE has the same problem with the ActiveX plugin.

Comment 4

[Peter Lubczynski]

-->taking bug to fix for ActiveX for 1.3

Comment 5

[rubydoo123]

adding the topembed+ marker

Comment 6

[rubydoo123]

moving to 1.3 beta

Comment 7

[Peter Lubczynski]

Attachment: patch v.1

patch to resolve relative URLS on PLUGINSPAGE or PLUGINSURL

Comment 8

[Peter Lubczynski]

Attachment: patch v.2

better patch: resolves relative urls for PLUGINURL or PLUGINSPAGE if it's in
either an attribute or PARAM tag.

Comment 9

[Darin Fisher]

Comment on attachment 109677 [details] [diff] [review]
patch v.2

>Index: nsObjectFrame.cpp

>+      nsCAutoString newURL, oldURL (ToNewUTF8String(value));

use:	NS_ConvertUCS2toUTF8 oldURL(value);

instead of ToNewUTF8String as it cuts out one buffer copy and
potentially avoids a heap allocation.


>+      if (NS_SUCCEEDED(baseURL->Resolve(oldURL, newURL))) {
>+        value.AssignWithConversion(newURL.get());

do you really mean to truncate any non-ASCII characters?  i.e.,
are you certain newURL is entirely ASCII?  if not, then use

	  value = NS_ConvertUTF8toUCS2(newURL);

>Index: nsObjectFrame.h

>+  void FixUpURLS(nsString &name, nsString &value);

looks to me like |name| could be a |const| param, i.e.:

    void FixUpURLS(const nsString &name, nsString &value);

Comment 10

[Peter Lubczynski]

Attachment: patch v.3

NS_MakeAbsoluteURI seems to do all of that

Comment 11

[Darin Fisher]

Comment on attachment 109683 [details] [diff] [review]
patch v.3

hmm... a little wierd to use an input string as an output string in the same
function call.	fortunately, in this case NS_MakeAbsoluteURI doesn't do
anything like truncate the output buffer before anything else.	i fear that if
NS_MakeAbsoluteURI ever changed its behavior in this regard that your code
would simply break.  that makes it fairly fragile, so maybe it would be better
to not use NS_MakeAbsoluteURI here.  make sense?

Comment 12

[Peter Lubczynski]

Attachment: patch v.4

oops, I meant to use a temp var instead of the same one for in/out. Here's that
patch.

Comment 13

[Darin Fisher]

Comment on attachment 109692 [details] [diff] [review]
patch v.4

sr=darin (although i would use a nsAutoString for newURL... no need for a new
patch if you agree.)

Comment 14

[av (gone)]

Comment on attachment 109692 [details] [diff] [review]
patch v.4

r=av

Comment 15

[Peter Lubczynski]

patch w/ AutoString in trunk, marking FIXED.