Closed Bug 400485 Opened 17 years ago Closed 17 years ago

[FIX]Incompatibility: applet code parameter is fully-qualified in FF 3 (applet.code is absolute uri)

Categories

(Core Graveyard :: Plug-ins, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: kbrussel, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101605 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101605 Minefield/3.0a9pre

It appears that the "code" parameter to the applet tag is made a fully-qualified URL in some situations by Firefox 3. Previously it was passed along unmodified to the Java Plug-In and it was the plug-in's responsibility to turn it into a fully-qualified URL based on the codebase parameter. This is an incompatibility illustrated by the attached test case. This test case runs successfully on IE 6, IE 7, FF 1.5, FF 2.0 and Mozilla 1.8, so there is a strong indication that this change should be undone in FF 3.

Reproducible: Always

Steps to Reproduce:
1. Install FF 3 and Java.
2. Use Java Control Panel to open Java Console.
3. Navigate to included HTML file and watch output on Java Console.
Actual Results:  
NullPointerException is thrown in the Java Console.

Expected Results:  
Printed output ending in successful run of test is produced.
Attached file Test case
Component: General → Plug-ins
Keywords: regression
Product: Firefox → Core
QA Contact: general → plugins
Target Milestone: --- → mozilla1.9 M10
Version: unspecified → Trunk
Assignee: nobody → jst
Flags: blocking1.9?
Kenneth, I stepped through the plugin initialization code in a debugger and the value for the argument named "code" was always "JSAppletsComm1.class", i.e. the relative path. Can you give more details on the circumstances where you're seeing this come through as an absolute URL?
The test case uses the JSObject API to fetch the "code" member of window.document.applets[0]. To be honest, I'm unclear on exactly what is going on at that point -- whether the object that we return is supposed to be the scriptable object for the applet, and therefore recognized and unwrapped by the Java Plug-In, or whether it's supposed to be a JSObject representing the applet tag in the DOM. Anyway, assuming it's the DOM object for the applet tag, which it appears to be, then the JSObject fetch of the "code" member of this DOM element appears to be fully qualifying this URL. I am pretty sure, though not 100% sure, that this qualification is not occurring in the Java Plug-In.
Actually, given that this problem occurs with the old Java Plug-In as well as the new NPRuntime-based Java Plug-In, I'm 100% sure this is a change in behavior in the browser and not the plug-in.
Kenneth, thanks for the explanation. Turns out that this was an intentional (at the time at least) change in Firefox. It was made in bug 213701. Cc:ing some folks from that bug here.

Boris et al, what this comes down to is that the changes in bug 213701 changed apple.code to be an absolute URI and not just the value of the "codebase" attribute. The choices we have here are either to leave this as is, or to revert the applet.code part of the fix for bug 213701.

Thoughts?

Kenneth, do you have a feeling about how commonly this might be causing problems with Java stuff on the web? Have you seen this with this testcase only, or did this come from a real website/intranet or whatnot?
Status: UNCONFIRMED → NEW
Ever confirmed: true
So far we have seen this only with this test case and not with real applets on the web.

Note that the W3C specification seems fairly clear that the code parameter to the applet tag is supposed to be treated as a relative URI relative to the codebase and not fully resolved by the browser. See http://www.w3.org/TR/1999/REC-html401-19991224/struct/objects.html#h-13.4 .
Kenneth, the HTML specification just talks about the attribute.  If your testcase were doing applet.getAttribute("codebase") instead of applet.code, it would in fact get the original value.

The relevant specification for applet.code is http://www.w3.org/TR/2003/REC-DOM-Level-2-HTML-20030109/html.html#ID-31006348

Now here's the interesting part.  The DOM spec says:

  code of type DOMString
    Applet class file. See the code attribute definition in HTML 4.01. This
    attribute is deprecated in HTML 4.01.

In particular, it doesn't have the "URI [IETF RFC 2396]" thing it has for some other properties, and which would definitely mean that we need to resolve the absolute URI.  I seem to recall being told that this was an error in the spec and that some "lost" errata corrected it (but had gotten lost and then the Working Group in question was dissolved).  But one could make the case that we should not return an absolute URI for applet.code.

If we change that, we need to change page info accordingly.  As long as that happens, I don't have a strong opinion on what we do here one way or another.
Honestly, I don't care much either way here.

But given that it feels like we're doing the right thing here I'm reluctant to change it. So I'd say that we should WONTFIX this, but if we hear about more sites that break because of this we should revert. The beta release should tell us more.

Leaving as NEW in order to increase the changes that people will find the bug and dup against it.
Blocks: 213701
Flags: blocking1.9? → blocking1.9-
Summary: Incompatibility: applet code parameter is fully-qualified in FF 3 → Incompatibility: applet code parameter is fully-qualified in FF 3 (applet.code is absolute uri)
I disagree with the above comment. It is fairly clear to me based on the above readings of the spec that fully-qualifying the code parameter is the wrong thing to do. It is supposed to be interpreted *relative to* the codeBase, and in the case where multiple jar files are specified in the archive tag, it isn't possible for the browser to correctly fully qualify it. (I haven't written a test case to see whether FF 3 attempts to incorrectly do this.)

Rather than letting this bug twilight, I would appreciate it if you would please go ahead and revert back to the previous behavior, which is what other browsers as well as older Mozilla versions do.
Per comment #7 it does sound like we're doing the right thing. We probably don't do multiple URIs correctly though which would be a bug.
Comment #7 states that it isn't the case that the code attribute definitely needs to be resolved. This isn't the same thing as saying that FF 3 is doing the right thing. By my reading of the spec, which I don't think is invalid since the spec is ambiguous, it is the wrong thing to do to attempt to fully qualify the code attribute.

It seems counterproductive to continue to argue this point and/or require the generation of more test cases before the behavior would be reverted on the FF side.
I think I agree with Kenneth that we should change it.  The hard part are all the UI changes to page info.  The core change to just return the string is a one-liner...

Someone willing to step up for the UI end of things?
And I do think we should block on this.
Flags: blocking1.9- → blocking1.9?
I don't agree that we're returning the wrong thing.

First off, I think we're returning the right thing. In all other cases DOM properties return absolute URIs, and it seems to be an oversight in the spec that they don't explicitly say that it should be returned that way here.

Second, we haven't found a single real website that depends on returning a relative uri.
That last part is really key.  Making this a relative URI increases overall code complexity, so the number of sites affected matters for this decision.
Another test case (test.zip, attachment 286949 [details]) has been attached. To run it, unzip the test.zip, enable the Java Console in the Java Control Panel, and either serve up the test/ directory from a web server or just navigate to its index.html on the local disk. When served up from a random web server, the output to the Java Console is the following:

  Applet's code tag: http://foo.bar.com/test/B
  Applet's class: B
  Applet's code tag: http://foo.bar.com/test/A
  Applet's class: A

The code tags for the applets are incorrect. For applet B, the correct fully-qualified URI would be something like jar:http://foo.bar.com/test/B.jar!/B .

In this test case it is not possible for the browser to properly create an absolute URI for the "code" parameter of the applet. Doing so would require determining in which jar file the class is contained. This is the job of the Java Plug-In, not the browser.

I agree that there is currently no evidence of real web sites broken, but the behavior is definitely incorrect and has caused breakage of at least some of our internal tests.
Oh, the "code" attribute is not just relative to the codebase?  In that case, what we're doing is just wrong.  And then we might not need to change page info either, since we'd want it to show the attribute as is, right?
Flags: blocking1.9? → blocking1.9+
(In reply to comment #18)
> Oh, the "code" attribute is not just relative to the codebase?  In that case,
> what we're doing is just wrong.  And then we might not need to change page info
> either, since we'd want it to show the attribute as is, right?
> 

So what can we display in Page Info? We need at some point an absolute url for the Save As button. How do we resolve this if resolving it as relative to the codebase attribute is wrong? Or maybe we should just stop displaying applets in Page Info?
(In reply to comment #18)
> Oh, the "code" attribute is not just relative to the codebase?

Correct. For most modern applets it should be treated as verbatim.

> And then we might not need to change page info
> either, since we'd want it to show the attribute as is, right?

Not sure regarding page info, but yes, this particular attribute should be shown as is.
(In reply to comment #19)
> 
> So what can we display in Page Info? We need at some point an absolute url for
> the Save As button. How do we resolve this if resolving it as relative to the
> codebase attribute is wrong? Or maybe we should just stop displaying applets in
> Page Info?

I'm not sure what Page Info does, but if you're trying to fetch all of the content associated with the tag, then the algorithm would be something like:
 - Iterate down the entries in the archive parameter of the applet (typically a list of jar files)
 - Resolve each one relative to the codebase parameter (note that both absolute and relative URIs can be used as entries in the archive parameter, and relative ones are resolved relative to the codebase)
 - The code parameter might resolve to a .class file on the server relative to the codebase, or it might not (in which case it would be an entry in one of the jar files named in the archive tag). If it does (conditional HTTP GET?) then it would need to be saved.

I definitely wouldn't push for implementation of this algorithm, especially if nothing like it has ever been implemented in the browser for the applet tag before. However, for the purpose of just viewing the code parameter to the applet tag, it should be left verbatim.
Just to add my 2¢, any consideration of Page Info is flattering, but ultimately unnecessary. Nothing depends on Page Info, while the correct functioning of some subset of java applets might, so Page Info should be a secondary concern.

At any rate, the point is a bit moot, because whenever java is enabled Page Info ignores all applet tags, because not doing so has historically caused the browser to crash. Assuming we can stop doing that at some point, I would just disable the Save As button for applets. 
So does page info not do anything interesting with Applet.code right now?
(In reply to comment #23)
> So does page info not do anything interesting with Applet.code right now?
> 

It's used only in one place, in code that is disabled when java is enabled:
http://mxr.mozilla.org/seamonkey/source/browser/base/content/pageinfo/pageInfo.js#619
Attached patch PatchSplinter Review
This should really work.  I tried testing on the attached testcases, but they throw Java exceptions of all sorts (making it hard to tell whether they pass by looking at the Java console), and trying to look at Applet.code from script hangs when wrapping the Applet node.  We ask the jvm for a JSObject for it, it starts processing its work queue.... and then nothing happens.  Presumably some sort of deadlock?
Attachment #287041 - Flags: superreview?(jst)
Attachment #287041 - Flags: review?(jst)
Attachment #287041 - Flags: superreview?(jst)
Attachment #287041 - Flags: superreview+
Attachment #287041 - Flags: review?(jst)
Attachment #287041 - Flags: review+
Do we need to do something to make sure that calling addImage with something that's not an absolute URI like that won't be too weird?
Assignee: jst → bzbarsky
Summary: Incompatibility: applet code parameter is fully-qualified in FF 3 (applet.code is absolute uri) → [FIX]Incompatibility: applet code parameter is fully-qualified in FF 3 (applet.code is absolute uri)
(In reply to comment #25)
> I tried testing on the attached testcases, but they
> throw Java exceptions of all sorts (making it hard to tell whether they pass by
> looking at the Java console)

Did you try the "new test case"? That should run cleanly. Correct behavior is output to the Java Console looking like

  Applet's code tag: A
  Applet's class: A
  Applet's code tag: B
  Applet's class: B

> and trying to look at Applet.code from script
> hangs when wrapping the Applet node.  We ask the jvm for a JSObject for it, it
> starts processing its work queue.... and then nothing happens.  Presumably some
> sort of deadlock?

I'll double check to see whether this is happening with the new NPRuntime-based Java Plug-In. I can't vouch for the old OJI-based one.
Yes, I tried both testcases.  The second testcase did not produce that output.  Instead it says:

java.lang.NullPointerException
	at B.start(B.java:8)
	at sun.applet.AppletPanel.run(Unknown Source)
	at java.lang.Thread.run(Unknown Source)
java.lang.NullPointerException
	at A.start(A.java:8)
	at sun.applet.AppletPanel.run(Unknown Source)
	at java.lang.Thread.run(Unknown Source)


Note that in my experience that's about how things go with Java on Linux, esp. when running in a debug build (ABI mismatch?  I don't know). I've had it disabled in preferences for years as a result...
Attachment #286949 - Attachment is obsolete: true
Thanks for the fast turnaround.

(In reply to comment #28)
> Yes, I tried both testcases.  The second testcase did not produce that output. 
> Instead it says:
> 
> java.lang.NullPointerException
>         at B.start(B.java:8)
>         at sun.applet.AppletPanel.run(Unknown Source)
>         at java.lang.Thread.run(Unknown Source)
> java.lang.NullPointerException
>         at A.start(A.java:8)
>         at sun.applet.AppletPanel.run(Unknown Source)
>         at java.lang.Thread.run(Unknown Source)

Sorry. That turned out to be a bug in my test case where I had neglected to include the MAYSCRIPT attribute to the applet tag, which is necessary for the current / old OJI-based Java Plug-In. I had tested only with the new NPRuntime-based one (coming soon). Revised test case has been attached. Would appreciate it if you could verify with it. Just loading the test should produce the expected output to the Java Console. Clicking the "Run Test" button should show a JavaScript alert with the "code" attribute of the applet.

> Note that in my experience that's about how things go with Java on Linux, esp.
> when running in a debug build (ABI mismatch?  I don't know). I've had it
> disabled in preferences for years as a result...

I agree that the reliability of Java Plug-In on Unix platforms in particular has been very poor. This was one of the main motivations for the recent rewrite, which I believe will categorically solve this problem.
(In reply to comment #26)
> Do we need to do something to make sure that calling addImage with something
> that's not an absolute URI like that won't be too weird?
> 

addImage won't work without an absolute URI.

We should either try to implement the algorithm in comment 21 or maybe
completely remove applet tags support from Page Info (is has always been at
least partly broken anyway).

I'm going to file another bug so that we take care of the Page Info issue
separately.
Blocks: 402286
With the updated testcase, I just seem to hang. Breaking in gdb indicates that JSHandler() calls CNSAdapter_Liveconnect::GetMember which calls some JS code, which calls JavaObject_lookupProperty, which gets into ProxyJNIEnv::InvokeMethod, which calls CSecureJNIEnv::CallMethod, which ends up calling get_msg.  And there we sit in poll().

Hard to tell what else is going on, since any attempt to continue at this point makes the Java plugin call exit(), with the usual

  INTERNAL ERROR on Browser End: Pipe closed during read? State may be corrupt
  System error?:: Interrupted system call

business.

It's very good to hear that things will improve on Unix!

Florian, getting a separate bug filed on page info sounds great.  Please mention the number here when you do?
bz: it's 402286. He did made it a dependency even if he didn't put the bug number in a comment.
Checked in.

Someone who's browser doesn't hang when accessing applets from script should probably try checking in a regression test.  :(
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Fix verified with nightly build dated 11/08/2007. Thanks for the timely fix.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: