53347 - Acrobat Forms Support Problem: Form fails to submit

Status: VERIFIED FIXED

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

Description

[lmcquarr]

1 - Using M17 win build (current build is completely broken with respect to PDF
files), go to the above URL.

2 - Type something in the box and then press submit.

3 - CRASH!

Here is the callstack:

GKPLUGIN! 603965cb()
GKPLUGIN! 603940f3()
GKPLUGIN! 60391662()
NPN_PostURL(_NPP * 0x02597aa8, const char * 0x02582260, const char * 0x00000000,
unsigned long 52, const char * 0x0012fa38, unsigned char 1) line 209 + 32 bytes
P

Comment 1

[lmcquarr]

Now that 53122 is fixed, I can use today's build.  This crash still happens.

Comment 2

[sean echevarria]

no crash with proposed fix for bug 53399 - but the form doesn't ever update the
bottom box after you press submit.  Looks like a stream is getting created when
submit is clicked and then nothing happens.

Comment 3

[lmcquarr]

Thank you, [e-mail], for you prompt attention to this!

Comment 4

[ekrock's old account (dead)]

High profile partner. Setting P1. PDF forms are widely used on the web for 
complex governmental forms, etc.

Since crash no longer occurs, changing Summary from "Acrobat Forms Support 
Problem:  Crash in NPN_PostURL" to "Acrobat Forms Support Problem:  Form fails 
to submit"

Comment 5

[lmcquarr]

In the meantime,  it is also critical to support Acrobat Forms.
Unfortunately, support of Acrobat forms is going to exploit
a handful  of undocumented NS API features.
This bug that makes it impossible for me to
do any futher testing  since even the most
basic Acrobat form crashes.   

Comment 6

[ekrock's old account (dead)]

Increasing Severity to blocker as this is blocking Adobe from doing further QA 
on Acrobat support within Moz/N6. Adobe is also unlikely to be willing to have 
its installer install into Moz/N6 if PDF form submission is not supported.

Comment 7

[karnaze (gone)]

Marking nsbeta3+.

Comment 8

[Michael La Guardia]

Given that we are two days away from the close of engineering work on pr3 and
there is no indication of a fix in the bug, I think we can't hold pr3 for this.
 Marking nsbeta3-.  Nominating for rtm.

Comment 9

[karnaze (gone)]

Adding rtm+.

Comment 10

[Phil Peterson]

PDT marking [rtm need info] since no patch or reviewers

Comment 11

[av (gone)]

Looks like xpconnect problem to me. The console reports xpconnect error. BTW, 
Reset button works fine. Reassigning.

Comment 12

[John Bandhauer]

This does not crash for me. I don't see an error report in recent branch or 
trunk builds. What error do you see? Please attach it to the bug. If you can 
show me something that shows I should investigate I will. Is there some evidence 
that xpconnect is reporting some internal problem as opposed to errors in JS 
code or other callers?

Comment 13

[Phil Schwartau]

Setting default QA -

Comment 14

[av (gone)]

OK, I don't see errors in today's build.

Reporter: what result should I see upon pressing Submit?
Debug console responds with 
'created stream for http://access.adobe.com/cgi-bin/browser/ping-adf.pl#FDF'
and some time later:
'killing stream for http://access.adobe.com/cgi-bin/browser/ping-adf.pl#FDF'

Comment 15

[ekrock's old account (dead)]

Liz, are you still seeing this on your machine in a recent (last day or two)
Mozilla daily build? Please let us know ASAP so we can determine if this is
still an oustanding issue.

Andrei: It sounds like you're no longer seeing a crash. Are you also seeing the
form submit as it should? Is this now a true WORKSFORME?

Comment 16

[av (gone)]

I am not sure what I should see when the form is submitted. In fact, I see 
nothing. But there is indeed no crash.

Comment 17

[av (gone)]

OK, now I have some ideas how we can fix this. The problem is that data to 
submit is stored by Acrobat in temp file first and then PostURL is called to 
post it but the current code just lack the implementation of this 'file case'
and sends the file path as the data to submit. The plan is to read the file 
first and feed PostURL with correct data. Then delete the file according to the 
spec for NPN_PostURL.

Comment 18

[av (gone)]

I'll try to provide a patch over hte weekend.

Comment 19

[lmcquarr]

Also be aware that this code path uses an undocumented (I believe) NS 
API convention where the first lines
of the temp file are HTTP headers to be used and the rest is
the data.  I think that there are two <CR> separating the headers from the
data (off the top of my head).   

Comment 20

[lmcquarr]

When you are testing your fix, and you press the SUBMIT button, you
should see FDF (a file format similar to PDF) displayed in the box
that you type in.  If you see an HTTP headers, then mozilla is not
properly parsing out the HTTP headers.  (Compare to Nav 4.7X to see
exactly what should be displayed).

BTW -- the convention of putting the HTTP headers as the first part of the file
*IS* a documented feature:

Possible URL types include FTP, HTTP, mail, and news. For protocols where the 
headers must be distinguished from the body, such as HTTP,
the buffer or file should contain the headers, followed by a blank line, then 
the body. If no custom headers are required, simply add a blank line
('\n') to the beginning of the file or buffer. NOTE: Passing headers (even a 
blank line) in a memory buffer is not supported by
NPN_PostURL. If you need to specify headers (or leave a blank line for the 
defaults) in a memory buffer, you must use
NPN_PostURLNotify. 


(This is from  
http://cgi.netscape.com/eng/mozilla/3.0/handbook/plugins/pi3.htm#npnposturl)

Comment 21

[av (gone)]

OK, let me be more specific. Using 4.75 I type qqq in there and get thefollowing 
upon pressing the Submit button:

%FDF-1.2   1 0 obj <<  /FDF << /Fields [ << /V (You may type something in here 
if you wish)/T (Gibberish)>>  << /T (Submit)>> ]  /F 
(http://access.adobe.com/browser/pingform.pdf)>>  >>  endobj trailer << /Root 1 
0 R   >> %%EOF 

Is this what I am supposed to see?

Comment 22

[lmcquarr]

Yes -- that is FDF.  (Note, of course, that this is a contrived example.)

Comment 23

[ekrock's old account (dead)]

Note to eng/PDT/all who prioritize/triage bugs: we have an additional reason to
fix this bug. Adobe has indicated to me that if we fix this bug (and they do not
then uncover other truly grievous problems with PDF forms support), that they
will then be willing to have their installer install into Moz/Netscape 6 in
spite of other issues that have been Futured such as the lack of byte range
support. (If they don't do this, then users will have to manually install
Acrobat into Moz/N6 even after running the Acrobat installer.) So this is a P1
high profile partner issue.

Comment 24

[av (gone)]

Attachment: Prototype patch

Comment 25

[av (gone)]

I just attached a prototype fix. I has some hardcoded hacks so still needs to be 
polished, but I wanted to make sure I am on a right track. It now reads the file 
before creating a new stream, and debugging shows that the data goes out. I can 
even see the response from the server as we call NPP_NewStream, NPP_WriteReady 
and NPP_Write, so the plugin _gets_ the data back. But for some reason I don't 
see it visually.

lmcquarr: I think I need a further word from you at this point.

Comment 26

[av (gone)]

lmcquarr: what I see we pass back to the plugin in NPP_Write is a buffer 559 
long which reads like this:
"%FDF-1.2
1 0 obj
<<
/FDF <</Fields[<</T (Ping Result)/V 
<254644462d312e32202020312030206f626a203c3c20202f464446203c3c202f4669656c6473205
b203c3c202f562028596f75206d6179207479706520736f6d657468696e6720696e2068657265206
96620796f752077697368292f5"

Does it ring any bells?

Comment 27

[lmcquarr]

That's correct.  

But also, we set the stream mode to NP_ASFILE, to tell
you to dowload it to your cache and give us the file name.  That means
for the NPP_Write call, we just return the length, and wait
to be called via NPP_StreamAsFile. 

Comment 28

[av (gone)]

Emergency! I cannot get to access.adobe.com any more.

Comment 29

[av (gone)]

OK, I talked to gagan and found out that currently we cannot send any arbitrary 
stream to cache. So, looks like we need to implement some stream cache emulation 
for this specific purpose. 

This is what happens: when we got the stream with the ping info back from the 
server and call NPP_NewStream, the plugin asks to prepare a cache file and give 
it instead. Currently used method nsPluginHostImpl::SetUpCache does not do the 
right thing, it actually calls NS_OpenURI and caches whatever comes back, but 
this is not what we want, as we already have a stream and don't need anything 
new. 

Comment 30

[av (gone)]

EMERGENCY!!!

Not Found
The requested URL /browser/pingform.pdf was not found on this server.

Comment 31

[lmcquarr]

Sorry!  I've had the flu and while I was gone, a non technical marketing person
accidentally deleted some files out there.  I put the pingform.pdf back,
and will work on putting a few other Acrobat forms test cases out there again
(the other test suite we set up  Netscape in 
http://access.adobe.com/browsers/netscape is still there).

Comment 32

[ekrock's old account (dead)]

Unfortunately, fixing this bug required writing too much new code (cacheing,
etc.) that might regress other plug-ins even if it worked correctly for Acrobat.
Given the limited time before ship and the risk of regressing the product
seriously enough to stop-ship, we unfortunately will have to postpone taking
this fix until after N6 RTM. Marking ns601.

Comment 33

[av (gone)]

Setting mozilla0.8

Comment 34

[bobj]

Updated Keywords.  Replaced "nsbeta3, rtm" with "nsbeta1".

Comment 35

[av (gone)]

Attachment: crash stack

Comment 36

[av (gone)]

Please ignore the attachment. It was intendent for different bug.

Comment 37

[ekrock's old account (dead)]

Nominating as a high-priority bug for nsbeta1. This is one of the bugs in plug-
in API backward compatibility of most concern to Adobe.

Comment 38

[Peter Lubczynski]

Well, looks like Acrobat is writing out this temp file:
C:\Documents and Settings\peterlubczynski\LOCALS~1\Temp>pdf32e2.tmp
That I have attached.

I think we assume that (void *) postData is pointing to the data when in fact, 
it's pointing to a file.

Looking at comments above from [e-mail] say that we aren't correctly 
checking the flag that this is a file instead of a read data.

I think Eric Pollmann has something in the works for when forms POST files as 
this is a similar problem. Perhpas some of that logic can be (re)used here.

Darin, does the new netlib some features for this kind of POST?

Comment 39

[Peter Lubczynski]

Attachment: file created by acrobat POST

Comment 40

[Darin Fisher]

necko's support for posting is very raw at the moment... you have to pass it
a input stream to some data/file that is properly formatted.

Comment 41

[Peter Lubczynski]

Darin,

Is there an example of such code someplace in the tree? Thanks!

Comment 42

[Eric Pollmann]

I'm not Darin.  :)  But there are examples of both a string and a file being
converted into an input stream for netlib here in nsFormFrame's OnSubmit:

http://lxr.mozilla.org/seamonkey/source/layout/html/forms/src/nsFormFrame.cpp#915

It's not really pretty, but it does the trick!

Comment 43

[Eric Pollmann]

Also, in both of these cases, a decent amount of pre-processing has been done to
the data that is going to be passed to netlib:

For a standard url encoded form post, we generate a string buffer of name=value
pairs, url escaped (probably what the plugin wants to do?):
http://lxr.mozilla.org/seamonkey/source/layout/html/forms/src/nsFormFrame.cpp#1228
and prepend headers:
http://lxr.mozilla.org/seamonkey/source/layout/html/forms/src/nsFormFrame.cpp#1285

For multipart form post, we generate a temp file with name=value pairs, each of
which is separated by headers:
http://lxr.mozilla.org/seamonkey/source/layout/html/forms/src/nsFormFrame.cpp#1355

(This code is in the midst of being moved and rewritten to fix a slew of bugs!)

Comment 44

[karnaze (gone)]

Pollmann, what should we do at this point. Is this your bug?

Comment 45

[Eric Pollmann]

Hate to play hot potato, but this probably really is Andre's bug.  :)  The form
submit code path in layout is completely separate from the NPN_PostURLNotify
code path, and I have zero familiarity with NPN_PostURLNotify, or how changes to
this code might impact this and other plugins.

However, I am more than happy to help with this one - perhaps we can take the
working code from layout and use that as an example for how NPN_PostURLNotify
should be bundling up and handing off form submit data to Necko?

Comment 46

[Eric Pollmann]

Attachment: patch 1

Comment 47

[Eric Pollmann]

Attachment: patch 2

Comment 48

[Eric Pollmann]

Patch 2 fixes the bug for me, I see the same results as described above for 4.x.
(Patch 2 replaces patch 1, which was a little, er, crashy.  :))

Outstanding issues:

1) We are assuming right now that the first 8 characters of aPostData are going
to be "file:///" in the file post case.  Andre, is that a valid assumption?

2) We are not applying FixPostData to the data in the isFile case.  Are there
plugins that require this coddling?

3) I see an instance of AcroRd32.exe hanging around after the app exits (I've
been running this test under viewer).  If it is not manually killed (i.e. Task
Manager), the plugin will not load the next time.  Is this a related problem?

Comment 49

[Eric Pollmann]

Attachment: patch 3

Comment 50

[Eric Pollmann]

Patch 3 adjust for the leak fix I recently checked in for layout's form
submission - it will now assume postDataStream comes back addRef'd in the file case.

I also added a little bit of logic to check if the file name really begins with
"file:///" or not.  If not, it tries to pass it through unmodified (Andre, is
this okay?  Localfile will just return an error and we will give up early if the
filename is invalid: and it would likely be invalid anyway if it doesn't start
with file:/// and we lopped off the first 8 characters.  Do you know what Nav
4.x did if the filename was invalid like this?)

Still seeing:

3) I see an instance of AcroRd32.exe hanging around after the app exits.  If it
is not manually killed (i.e. Task Manager), the plugin will not load the next time.

4) Who is responsible for deleting the temporary file generated here after it is
uploaded to the server?

Comment 51

[av (gone)]

Didn't have chance to look at it today... 

>3) I see an instance of AcroRd32.exe hanging around after the app exits.  If it
>is not manually killed (i.e. Task Manager), the plugin will not load the next 
>time.

I always saw this. But I don't have problems with loading the plugin again. 
Seems to me just how it works. Or maybe a bug on their side. I see a break point 
hit on calling shutdown library and unloading nppdf32.dll.

4) Who is responsible for deleting the temporary file generated here after it is
uploaded to the server?

If they don't do it we probably should take care of this.

Comment 52

[av (gone)]

We shouldn't worry about acrrd32.exe remaining in memory since it does the same 
with 4.x.

Comment 53

[av (gone)]

Looks like we should delete the file. Acrobat uses our API call NPN_PostURL. The 
spec says we are responsible for deleting the file: 
http://developer/docs/manuals/communicator/plugin/pgfn2.htm#1007707

Comment 54

[Peter Lubczynski]

I think the file should be deleted when OnStopRequest fires? 

Andrei, what do you think?

Comment 55

[av (gone)]

Yes, this is a possibility. I'll figure it out.

Another issue (Eric mentioned that in earlier comment) when remaining in memory 
acrrd32.exe blocks us from loading the plugin second time... If you see this 
just update the tree, my yesterday's checkin took care of this one.

Comment 56

[av (gone)]

Bug 75826 is now fixed. I cannot really tell if the form submission actually 
works, but at least I don't see any difference with 4.x now.

Liz, would you check it out and confirm or give us a more revealing test case so 
we can confirm it?

Marking fixed.

Comment 57

[lmcquarr]

Sorry for the delay (sick kid kept me out of work for two days).

Here is an easy test case:

http://access.adobe.com/browser/pingform.pdf

Go to this URL and type any thing you want. Press submit.
When you do, we call  NPN_PostURL.

You will know everything is working if you see something like
this populate the large grey box on the left:

%FDF-1.2   1 0 obj <<  /FDF << /Fields [ << /V (You may type something in here 
if you wish)/T (Gibberish)>>  << /T (Submit)>> ]  /F 
(http://access.adobe.com/browser/pingform.pdf)/ID [ 
<160bbb62aaa6ebe41da6613b185c3724><843579f805b9c66041e13a566488878f> ] >>  >>  
endobj trailer << /Root 1 0 R   >> %%EOF 

(This is FDF -- a second file format associated with PDF and Acrobat forms.)


I will do some testing are likely to be other things that are broken
associated with Acrobat forms.  Unfortunately, Acrobat forms uses
some of the undocumented features in the old NS API.   

Comment 58

[av (gone)]

That's what I see now and there is no difference from 4.x behaviour. Can we now 
assume this is working?

Comment 59

[shrirang khanzode]

I see the same too on 0419 trunk build on windows.

Comment 60

[lmcquarr]

Yipee, Hooray!  Thank you!

I now need to get some Acrobat forms QA folks to do some more testing,
if I can (most are out recovering from shipping Acrobat 5). 

As I mentioned earlier, some features of Acrobat forms exercise
undocumented aspects of the old NS_API.  I expect, therefore, other
things to be broken. 

Fixing this bug, however, will meet 80% or more of Acrobat forms needs.

Comment 61

[lmcquarr]

Worked for me with today's build!  Great job!  

Have a relaxing and refreshing sabbatical, Andrei. 

Comment 62

[shrirang khanzode]

verified on 0423 build windows trunk.