If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in M18

Status

Core Graveyard
Java: OJI
P3
normal
VERIFIED FIXED
18 years ago
7 years ago

People

(Reporter: Igor Plotnikov, Assigned: edburns)

Tracking

Trunk
x86
Windows NT
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta2-])

Attachments

(6 attachments)

(Reporter)

Description

18 years ago
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

Updated

18 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 1

18 years ago
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

Comment 2

18 years ago
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

Comment 3

18 years ago
Putting on [nsbeta2+] radar for beta2 fix. 
Whiteboard: [nsbeta2+]

Comment 4

18 years ago
M16 has been out for a while now, these bugs target milestones need to be 
updated.
(Assignee)

Comment 5

18 years ago
Reassigning to Jeff.
Assignee: drapeau → jeff.dyer
Status: ASSIGNED → NEW
Target Milestone: M16 → M17

Comment 6

17 years ago
Per today's PDT, moving from [nsbeta2+] to [nsbeta2-]
Whiteboard: [nsbeta2+] → [nsbeta2-]

Comment 7

17 years ago
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

Comment 8

17 years ago
Changing QA contact and add myself to cc list
QA Contact: paw → shrir

Comment 9

17 years ago
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
(Assignee)

Comment 10

17 years ago
I'll test this with the branch.

Comment 11

17 years ago
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

Comment 12

17 years ago
nsPluginHostImpl::GetURL will eventually delegate the call to 
pluginInstanceOwner::GetURL in modules\plugin\nglsrc\nsPluginViewer.cpp. 
However, pluginInstanceOwner::GetURL basically ignore the postData parameter.

(Assignee)

Comment 13

17 years ago
Yup.
Status: NEW → ASSIGNED
(Assignee)

Comment 14

17 years ago
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>. 

(Assignee)

Comment 15

17 years ago
Pushing out to M18.
Target Milestone: M17 → M18
(Assignee)

Comment 16

17 years ago
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
(Assignee)

Comment 17

17 years ago
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
(Assignee)

Comment 18

17 years ago
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
(Assignee)

Comment 19

17 years ago
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.
(Assignee)

Comment 20

17 years ago
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
(Assignee)

Comment 21

17 years ago
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);
         }

(Assignee)

Comment 22

17 years ago
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
(Assignee)

Comment 23

17 years ago
Created attachment 13090 [details] [diff] [review]
Fix: first iteration, unified diffs.
(Assignee)

Comment 24

17 years ago
Created attachment 13152 [details] [diff] [review]
Fix: second iteration, unified diffs
(Assignee)

Comment 25

17 years ago
Created attachment 13153 [details] [diff] [review]
Fix: second iteration, patch
(Assignee)

Comment 26

17 years ago
Created attachment 13154 [details]
Fix, second iteration, tar of modified files.
(Assignee)

Comment 27

17 years ago
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

Comment 28

17 years ago
beard: will this affect the MRJ plugin?
(Assignee)

Comment 29

17 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 30

17 years ago
Created attachment 13429 [details] [diff] [review]
Fix for bustage, iteration one.
(Assignee)

Comment 31

17 years ago
Created attachment 13430 [details] [diff] [review]
Fix for bustage, iteration two.

Comment 32

17 years ago
verified fixed on windows build 2000090508m18.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 33

17 years ago
Stanley Sez: NOPE, it doesn't work.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 34

17 years ago
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.
(Assignee)

Comment 35

17 years ago
nominating for nsbeta3
Keywords: nsbeta2 → nsbeta3
(Assignee)

Comment 36

17 years ago
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.  

(Assignee)

Comment 37

17 years ago
Created bug 51821, on which this bug depends.
Depends on: 51821

Comment 38

17 years ago
Sean, I think you fixed the problem with OnDataAvailable some time ago. Could 
you please comment?

Comment 39

17 years ago
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.


(Assignee)

Updated

17 years ago
Depends on: 51919
(Assignee)

Comment 40

17 years ago
51919 fixed.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 41

17 years ago
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.
(Assignee)

Comment 42

17 years ago
This doesn't work with Sun.Net Java Mail client.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 43

17 years ago
Depends on bug 60089.
Depends on: 60089
(Assignee)

Comment 44

17 years ago
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
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 45

17 years ago
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

Updated

7 years ago
Component: Java: OJI → Java: OJI
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.