123974 - [viewpoint] 404 file not found errors are passed back to the plugin incorrectly

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Ari Berger]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
BuildID:    0.9.4 branch

Our plugin calls NPN_GetURLNotify for a file that does not exist. In this case:
instance: our plugin instance
url: http://cole.viewpoint.com/~aberger/standard_primitives/cone.html
target: NULL
notifyData: some pointer (not NULL)

We expect to get back an error in UrlNotify.  Instead, we get no error.  We get 
a new stream, the contents of which is the 404 error that the browser would 
give you if you type the URL in the address bar- something like, "404 error the 
document could not be found..."

We do see that the MIME type is text/html instead of the expected 
application/metastream.  But overall, that is a bad way to test for failure- 
the networking layer has no idea what MIME type to expect.

Reproducible: Always
Steps to Reproduce:
1.Install viewpoint plugin.
2.View above URL
3.Notice the parse error- we try to parse the 404 error as an XML file.
4. Look to see what Mozilla passes us- we get a NewStream instead of a Notify.

Comment 1

[serge (gone)]

Andrei, if you don't mind I can take care of this,
I've ran into similar problem with some plugger's bugs on linux.

Comment 2

[Peter Lubczynski]

-->over to Serge

Could we just check the HTTP status on the stream before giving it to the plugin
and call notify accordingly?

Comment 3

[serge (gone)]

Attachment: patch v1.

it checks http response header for > 206 at the beginning of
nsPluginStreamListenerPeer::OnStartReques() method

Comment 4

[av (gone)]

Comment on attachment 69406 [details] [diff] [review]
patch v1.

r=av. (Do you really want your name in comments?)

Comment 5

[serge (gone)]

Attachment: patch v1.1

no name in comment

Comment 6

[av (gone)]

Comment on attachment 69514 [details] [diff] [review]
patch v1.1

r=av

Comment 7

[Patrick C. Beard]

Comment on attachment 69514 [details] [diff] [review]
patch v1.1

In |nsPluginStreamInfo::GetLocalCachedFile()|, if mFilePath is nsnull,
then the value returned in *path should also be nsnull, so in that case
you should assigning nsnull to *path.

Comment 8

[serge (gone)]

Attachment: v1.2

changes in
nsPluginStreamInfo::GetLocalCachedFile(char** path)
{ 
  if (mFilePath)
    *path = PL_strdup(mFilePath);
  else
    *path = nsnull;
}

Comment 9

[Darin Fisher]

FWIW: prefer strdup to PL_strdup whereever possible since it might be much
better optimized on some systems.

Comment 10

[Patrick C. Beard]

Comment on attachment 69692 [details] [diff] [review]
v1.2

sr=beard

Comment 11

[serge (gone)]

>prefer strdup to PL_strdup ...
Darin, you are right if we do check for null pointer
there is no risk to use strdup after that.

Comment 12

[serge (gone)]

Checking in nsPluginHostImpl.cpp;
new revision: 1.359; previous revision: 1.358
Checking in ns4xPluginInstance.cpp;
new revision: 1.82; previous revision: 1.81

Comment 13

[shrirang khanzode]

'url not found' error.