Closed Bug 36212 Opened 20 years ago Closed 20 years ago

Possible API problem: was: Java POST via URLConnection fails over https

Categories

(Core Graveyard :: Java: OJI, defect, P3)

x86
Windows NT
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: igor, Assigned: edburns)

References

Details

(Whiteboard: [nsbeta2-])

Attachments

(6 files)

Applet performs a POST request in a generally accepted way, 
i.e. by getting an OutputStream out of URLConnection, and 
writing to it.

It works in any other browser.

Here's the test applet which posts to 
http(s)://my.uroam.com/javatest/javapost.php3 :

http://my.uroam.com/javatest/index.htm   - it will report that it works

https://my.uroam.com/javatest/index.htm  - it will throw an exception:

java.io.FileNotFoundException: https://my.uroam.com/javatest/javapost.php3

at sun.plugin.protocol.https.BrowserHttpsURLConnection.getInputStream
    (BrowserHttpsURLConnection.java:287)


the applet code is here: http://my.uroam.com/javatest/MainApplet.java

GET request will work fine.

Thanks!
Igor
Status: UNCONFIRMED → NEW
Ever confirmed: true
HTTPS support doesn't yet work in OJI and Java Plug-In.  We're planning on
implementing this for M16.  Stay tuned...
Status: NEW → ASSIGNED
Target Milestone: --- → M16
Nominating for Beta 2 as part of the security work being done by Jeff Dyer.  His
work should make it into M16, looks good so far.
Keywords: nsbeta2
Putting on [nsbeta2+] radar for beta2 fix. 
Whiteboard: [nsbeta2+]
M16 has been out for a while now, these bugs target milestones need to be 
updated.
Reassigning to Jeff.
Assignee: drapeau → jeff.dyer
Status: ASSIGNED → NEW
Target Milestone: M16 → M17
Per today's PDT, moving from [nsbeta2+] to [nsbeta2-]
Whiteboard: [nsbeta2+] → [nsbeta2-]
Assigning to Stanley for further investigation. Stanley - could you take a look 
at this, and comment on what you think the problem is? Thanks. 
Assignee: jeff.dyer → stanley.ho
Changing QA contact and add myself to cc list
QA Contact: paw → shrir
I got the Mozilla build yesterday (07/20/2000), and I was not able to get the 
browser to download any page through HTTPS at all. I have built the PSM and 
other things. Am I missing something? The investigation can't go further unless 
I got the browser to download page through HTTPS.
Status: NEW → ASSIGNED
I'll test this with the branch.
In Java Plug-in, we completely rely on the browser's HTTPS implementation to 
support HTTPS GET/POST. When HTTPS POST is used in the applet, we will delegate 
the call to nsPluginHostImpl::PostURL to perform the action.

However, in modules/plugin/nglsrc/nsPluginHostImpl.cpp, 
nsPluginHostImpl::PostURL() was implemented incorrectly as a GetURL().

NS_IMETHODIMP nsPluginHostImpl::PostURL(nsISupports* pluginInst,
									
	const char* url,
									
	PRUint32 postDataLen, 
									
	const char* postData,
									
	PRBool isFile,
									
	const char* target,
									
	nsIPluginStreamListener* streamListener,
									
	const char* altHost, 
									
	const char* referrer,
									
	PRBool forceJSEnabled,
									
	PRUint32 postHeadersLength, 
									
	const char* postHeaders)
{
  nsAutoString      string; string.AssignWithConversion(url);
  nsIPluginInstance *instance;
  nsresult          rv;

  // we can only send a stream back to the plugin (as specified 
  // by a null target) if we also have a nsIPluginStreamListener 
  // to talk to also
  if(target == nsnull && streamListener == nsnull)
	  return NS_ERROR_ILLEGAL_VALUE;

  rv = pluginInst->QueryInterface(kIPluginInstanceIID, (void **)&instance);

  if (NS_SUCCEEDED(rv))
  {
    if (nsnull != target)
    {
      nsPluginInstancePeerImpl *peer;

      rv = instance->GetPeer(NS_REINTERPRET_CAST(nsIPluginInstancePeer **, 
&peer));

      if (NS_SUCCEEDED(rv))
      {
        nsCOMPtr<nsIPluginInstanceOwner> owner;

        rv = peer->GetOwner(*getter_AddRefs(owner));

        if (NS_SUCCEEDED(rv))
        {
          if ((0 == PL_strcmp(target, "newwindow")) || 
              (0 == PL_strcmp(target, "_new")))
            target = "_blank";
          else if (0 == PL_strcmp(target, "_current"))
            target = "_self";

          rv = owner->GetURL(url, target, (void*)postData);
        }

        NS_RELEASE(peer);
      }
    }

    if (streamListener != nsnull)
      rv = NewPluginURLStream(string, instance, streamListener);

    NS_RELEASE(instance);
  }

  return rv;
}


Therefore, when HTTPS POST is used in the applet, the browser will actually try 
to perform a HTTPS GET on the server, which usually result in error, and it 
results as exception in the applet.

Also, the target param will normally be nsnull, so this is another bug. Since 
the bugs are in the plugin module, I will reassign to Ed.

Assignee: stanley.ho → edburns
Status: ASSIGNED → NEW
nsPluginHostImpl::GetURL will eventually delegate the call to 
pluginInstanceOwner::GetURL in modules\plugin\nglsrc\nsPluginViewer.cpp. 
However, pluginInstanceOwner::GetURL basically ignore the postData parameter.

Yup.
Status: NEW → ASSIGNED
I've posted to n.p.m.crypto:

Message Posted    

Your message has been submitted to netscape.public.mozilla.crypto. 

NOTE: If you made a mistake and need to cancel your message, you can cancel it 
from our cancel page.
To do so you will need this Message-ID: <8na3jp$rtn$1@nnrp1.deja.com>. 

Pushing out to M18.
Target Milestone: M17 → M18
Igor, can you please tar up your testcase so I can put it on a server inside my 
org's firewall so I can test this with PSM, which currently doesn't work 
through socks?

Thanks,

Ed
Stanley, I tried running the non https applet above and I found that 
nsPluginHostImpl::PostURL is never called.  Can you give me a way to make it so 
nsPluginHostImpl::PostURL is called?

Thanks,

Ed
Stanley, I find that the uroam applet never calls any POST code in oji-
plugin/src/win32/core.  Can we meet to work on this?

Ed
Stanley helped me obtain the following information on this bug.

problem 1: null "target" argument to nsPluginHostImpl::PostURL()isn't handled 
correctly.  It IS permissible, according to Stanley, to have a null target, in 
fact there is no other possible value in the case of POST.

problem 2. If you correct for problem 1, you find that PostURL calls 
nsPluginInstanceOwner::GetURL() in nsObjectFrame.cpp, which does nothing with 
the post data.  This appears to be incorrect.
Ok, when an applet does post over an https connection, eventually 
nsPluginHostImpl::PostURL() is called.

NS_IMETHODIMP nsPluginHostImpl::PostURL(
nsISupports* pluginInst,
const char* url,
PRUint32 postDataLen, 
const char* postData,
PRBool isFile,
const char* target,
nsIPluginStreamListener* streamListener,
const char* altHost, 
const char* referrer,
PRBool forceJSEnabled,
const char* postHeaders)

After making the above changes to handle a null "target" parameter, we will 
call 

            rv = owner->GetURL(url, target, (void*)postData);

owner here is an instance of nsIPluginInstanceOwner.  This interface has no 
PostURL method.  I propose we modify nsIPluginInstanceOwner::GetURL to add the 
postDataLen parameter, since we have no way of creating an input stream 
otherwise.

Gagan, Pollmann, what do you think?

Ed
Summary: Java POST via URLConnection fails over https → Possible API problem: was: Java POST via URLConnection fails over https
It's important that the nsIInputStream created from the post data implement 
nsIRandomAccessStore:

nsDocShell::DoURILoad():

         nsCOMPtr<nsIRandomAccessStore> postDataRandomAccess(do_QueryInterface
(aPostData));
         if (postDataRandomAccess)
         {
             postDataRandomAccess->Seek(PR_SEEK_SET, 0);
         }

I'm about to attach an unsafe fix to this bug.  It's unsafe because it
doesn't have any API change, and thus requires the postData to be const
char * and null-terminated.  This fix casts the postData back to const
char * from void *.

Files in this diff:

M modules/plugin/nglsrc/nsPluginHostImpl.cpp
M layout/html/base/src/nsObjectFrame.cpp

cvs diff -u modules/plugin/nglsrc/nsPluginHostImpl.cpp 
layout/html/base/src/nsObjectFrame.cpp
Here is what I believe to be the correct fix. This post correspondes to the 
fix, second iteration attachments.

M modules/plugin/nglsrc/nsIPluginInstanceOwner.h
M modules/plugin/nglsrc/nsPluginHostImpl.cpp
M modules/plugin/nglsrc/nsPluginInstancePeer.cpp
M modules/plugin/nglsrc/nsPluginViewer.cpp
M layout/html/base/src/nsObjectFrame.cpp

This checkin modifies nsIPluginInstanceOwner::GetURL to have a length
parameter for the post data.

cvs diff -u modules/plugin/nglsrc/nsIPluginInstanceOwner.h 
modules/plugin/nglsrc/nsPluginHostImpl.cpp 
modules/plugin/nglsrc/nsPluginInstancePeer.cpp 
modules/plugin/nglsrc/nsPluginViewer.cpp layout/html/base/src/nsObjectFrame.cpp

tar -cvf 36212.2.tar modules/plugin/nglsrc/nsIPluginInstanceOwner.h 
modules/plugin/nglsrc/nsPluginHostImpl.cpp 
modules/plugin/nglsrc/nsPluginInstancePeer.cpp 
modules/plugin/nglsrc/nsPluginViewer.cpp layout/html/base/src/nsObjectFrame.cpp
beard: will this affect the MRJ plugin?
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
verified fixed on windows build 2000090508m18.
Status: RESOLVED → VERIFIED
Stanley Sez: NOPE, it doesn't work.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
The problem now is that the nsIPluginStreamListener passed to 
nsIPluginManager::PostURL() never receives OnDataAvailable(), only OnStopRequest
().  This is a problem because the plugin can never then get the data.
nominating for nsbeta3
Keywords: nsbeta2nsbeta3
av: Please tell me if I'm wrong about this.

The reason our nsIPluginStreamListener instance never gets OnDataAvailable() is 
that we don't add ourselves to the loadgroup until after the channel's 
OnDataAvailable has been sent.  

Created bug 51821, on which this bug depends.
Depends on: 51821
Sean, I think you fixed the problem with OnDataAvailable some time ago. Could 
you please comment?
Afraid I can't shed any light on this.

I can't say that I ever call GetURL with both a target and a listener.  My rule 
of thumb is: if I want something back, create a listener and use a null target; 
if I want to let the browser handle it, pass in a target and use a null 
listener.

That said, I'm not using PostURL at all, nor have I done anything using https.


Depends on: 51919
51919 fixed.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
George, bug 36212 is for Win32, just FYI.  Do we need to make it
platform all?

Stanley, the testcase in the bug is not does not correctly exercise the
code in question.  Can you please post the testcase you're using?  I
think the main reason this bug is still popping up is that I don't have
knaw about your testcases.  There have been several iterations where my
testcases showed the bug fixed but yours did not.  Therefore, I'll take
no action on this until I have a testcase from you that, if it
succeedes, will indicate that the bug has been fixed.
This doesn't work with Sun.Net Java Mail client.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on bug 60089.
Depends on: 60089
I'm re-closing this.  I have written another bug, bug 60228, that covers this 
latest manifestation of the POST over HTTPS thru PROXY problem.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
I tried the testcases provided by the bug reporter on today's trunk build 1128 
on windows and they both work fine. Since another bug has been opened for the 
'https thru proxy' problem, marking this VERIFIED.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.