286546 - NPP_Write returns incorrect offset and data after a call to NPN_RequestRead

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect)

Description

[salil kapoor]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1

I am developing a plugin to open files from inside the browser. When my MIME
type is accessed, the browser returns seekable as 1 in the call to
NPP_NewStream, because of which we make calls to NPN_RequestRead and this
results in corrupt data and offset in the calls of NPP_Write. The data contains
part of the HTML header - "Content-Range: byte <from>-<to>/size\0\0\0\0"
followed by the actual requested data. When run on other browsers like Opera,
seekable returns 0 and I work absolutely fine. 

Reproducible: Always

Steps to Reproduce:
1. Make a dummy plugin to handle a particular mime type.
2. Put a large file (about 2 MB) on a server that does not support streaming.
3. Try to access the file in the browser and intermittently give a call to
NPN_RequestRange.

Actual Results:  
The file returned was incorrect and had corrupt data.

Expected Results:  
The file should have been received correctly.

Comment 1

[salil kapoor]

Attachment: A dummy plugin to show the error condition.

Contains the plugin and the source for reproducing the problem.

Comment 2

[salil kapoor]

I have also made a dummy plugin and submitted in the attachments section to 
reproduce the problem. In the dummy plugin i am using PDF files for reference.
To reproduce -
1. Untar the attachment and install the plugin.
2. Go to www.amd.com (or any other site that has PDF files. You can also 
search on google with filetype as PDF).
3. Click on the PDF link.

Observation ....
1. After sometime the browser shows "stopped" in the status bar.
2. Open the file /tmp/nppdfmessages and observe the messages. During NPP_Write 
calls, you can see an NPN_RequestRead call showing the offset requested. Now 
observe the offset in NPP_Write after the call to NPN_RequestRead. It should 
be 0.

In order to see what data is received, you will need to alter the plugin code 
a little. In this you will observe that a part of the HTTP header comes in the 
data stream itself.

Comment 3

[Christopher Blizzard (:blizzard)]

-> Over to darin to have a look

Comment 4

[Darin Fisher]

I'm not that familiar with the plugin code, and it may be some time before I
have a chance to investigate this.

Comment 5

[salil kapoor]

I was looking at it further and also going through the HTTP RFC and it turns 
out the that servers might be seekable, but when they return a partial stream, 
the http header that they send is not completely correct. They have a double 
delimiter before "Content-Range bytes" section of the header, because of which 
it is considered as part of the data itself and given to us at offset 0.
So we need to find out why this is happening.

Comment 6

[Darin Fisher]

Sali, can you please capture a Mozilla HTTP log per the instructions on this site:

  http://www.mozilla.org/projects/netlib/http/http-debugging.html

Please use the "create a new attachment" link above to attach the resulting log
file to this bug report.  Thanks!

Comment 7

[Johnny Stenback (:jst)]

Attachment: Make NPN_RequestRead() immedately terminate the current "stream".

This fixes some problems related to this, but I'm not sure if it fixes the real
problem as reported here, if any, since either I don't understand what the
problem is, or the instructions for how to reproduce are not clear enough. What
I did notice while looking into this is that while we're writing data to a
plugin, if the plugin calls NPN_RequestRead() passing in a range it wants to
read from the stream, we will in some cases continue to write data to the
plugin from the offset where we were even after the call to NPN_RequestRead().
This fixes that, and makes is such that the NPP_Write() call after the plugin
called NPN_RequestRead() is the first chunk out of the wanted range, and not
more data from where we happened to be in the stream when we got the call to
NPN_RequestRead().

Please do test this if possible, and let me know if this fixes the problems, if
not, please give a clear explanation of how to reproduce the problem with exact
urls etc (and browser proxy info etc). And show us that HTTP log too, that
Darin asked for.

Comment 8

[salil kapoor]

I went throught he changes (in the attachment) made by Johnny. They don't seem 
to solve my problem as it is not the destruction of the stream but incorrect 
offsets which was causing the problem. The changes made may be infact become 
problematic because it could disrupt the current architecture in various other 
plugins if we simply destroy the stream on getting an NPN_RequestRead from 
within the call to NPP_Write. I'm not sure if we should be doing that.

Comment 9

[salil kapoor]

Attachment: The log file created using Firefox on Windows.

The procedure followed ... (make sure Adobe reader OR the dummy plugin given in
the bug is installed)

1. Set the environment variables to those required to enable logging.
2. Started firefox from the command line
3. In the google bar searched for "salil filetype:PDF"
4. Select the first link in the search result.

Observation - 
1. The link starts to open and some downloading starts.
2. The status quickly switches to Stopped and if you trued it on Adobe Reader,
then nothing shows up.

I then closed firefox. and went through the log file. It can be clearly noted
that towards the end of the log when we do an "http request" for a given range,
the corresponding "http response" does not contain the range field which (on
checking through ethereal) is present immediately after the delimiter specified
in the response header.

Comment 10

[Darin Fisher]

Can you also attach the ethereal trace?

Comment 11

[salil kapoor]

Attachment: Attaching the ethereal log.

Comment 12

[Boris Zbarsky [:bzbarsky]]

I really don't know enough about this code or the plugin api to reasonably
review this patch.....

Comment 13

[Darin Fisher]

Comment on attachment 178311 [details] [diff] [review]
Make NPN_RequestRead() immedately terminate the current "stream".

This looks alright to me.  I would probably use nsPIPluginStreamListener as a
name for the new interface since we have a convention for nsPI elsewhere and it
results in something shorter ;-)

Comment 14

[Darin Fisher]

Perhaps part of the problem here is that the nsPluginStreamListenerPeer doesn't
know to QI the nsIRequest passed to it in OnStartRequest to nsIMultipartChannel
in order to access the underlying nsIHttpChannel.  It seems like there is code
in OnStartRequest and SetUpStreamListener (and maybe elsewhere) that might be
affected.

Comment 15

[salil kapoor]

Any progress on the bug ?? Anything that I can do on my side to help in the 
fix ??

Comment 16

[Rudi Sherry]

(In reply to comment #5)
> I was looking at it further and also going through the HTTP RFC and it turns 
> out the that servers might be seekable, but when they return a partial stream, 
> the http header that they send is not completely correct. They have a double 
> delimiter before "Content-Range bytes" section of the header, because of which 
> it is considered as part of the data itself and given to us at offset 0.
> So we need to find out why this is happening.

I researched this and found the double delimiter is (incorrectly) added to the 206 response by the  
BlueCoat proxy software (SG version 3.2.4.x).  BlueCoat is one of the leading vendors of firewall  
software in the US so can affect many medium and large businesses.  Anyone not using this proxy will  
not see this double delimiter.  BlueCoat is aware of this problem and I assume they are addressing it.

The double delimiter is the only problem with the 206 response -- if you ignore it, the response is 
complete and correct.  I believe Mozilla should handle this pathological case to avoid this problem for 
businesses that are using that particular BlueCoat version.  I am willing to test it if necessary, since not 
everyone has access to BlueCoat's software.

I am adding this same comment to 286853, since I believe it's the same problem.  I'm not as familiar  
with the bug-reporting process so I'll leave it to others to change the status or close one of the two 
bugs.

Comment 17

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 18

[Darin Fisher]

Confirming, and reassigning to jst since he wrote the patch for this bug :)

Comment 19

[Benjamin Smedberg]

It's not clear to me whether there's a bug here... I don't think there is, but I'm going to mark this INCOMPLETE and if somebody can provide a testcase with more details we can reopen this (or you can file a new bug).