286576 - LiveConnect exceptions are not propagated to Java

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Java: Live Connect, defect)

Description

[Rich Giuli]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040616
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040616

Here is the code in jsj_JSObject.c:

385     /* Warnings are not propagated as Java exceptions - they are simply
386        ignored.  Ditto for exceptions that are duplicated in the form
387        of error reports. */
388     if (report && (report->flags & (JSREPORT_WARNING | JSREPORT_EXCEPTION)))
389         return;

Note that:
JSREPORT_WARNING = 0x1
JSREPORT_EXCEPTION = 0x2
(JSREPORT_WARNING | JSREPORT_EXCEPTION)=(0x1 | 0x2)=0x3
So, the exception check is checking that (report->flags & 0x3) != 0
Suppose report->flags is 0x2, which does not indicate a warning, but a real
exception.
However, (0x2 & 0x3) is 0x2, which is non-zero. 
So, if report->flags is 0x2 only, which does not indicate a warning, the if
statement is true.

I propose the if statement be changed to:
if (report && (report->flags & JSREPORT_EXCEPTION) && (report->flags &
JSREPORT_WARNING))
        return;

This properly checks that the flag is an exception AND the flag is a warning. My
tests show that making this change allows Javascript errors to be propagated
back into Java.

Reproducible: Always

Steps to Reproduce:
Create a Javscript method which throws an exception. For example:
var foo = null;
function jsThrowEx() {
    return foo.bogusMethod();
}

Then invoke this method from java. Ex:
JSObject obj = ...;
obj.call("jsThrowEx", null);
Actual Results:  
The method returns an no exception is thrown. However, there was a javascript
error so an exception should be thrown stating "foo has no properties".

Expected Results:  
A JSException should have been thrown due to the Javascript error. Making the
change I suggest above causes the exception to be thrown.

Comment 1

[Kyle Yuan]

Good catch, Rich. But you proposed change allows the JSREPORT_WARNING to be
thrown to java as an exception. I think we should only check the
JSREPORT_WARNING flag.

Comment 2

[Kyle Yuan]

Attachment: patch

Comment 3

[Kyle Yuan]

mass reassign my bugs to Pete Zha.

Comment 4

[Pete Zha]

mass reassign to Alfred

Comment 5

[Alfred Peng]

Rich, could you attach one specific test case? With the proposed change, there is still "foo has no properties" in the Error Console and no Exception throw in the Java code.

Comment 6

[Benjamin Smedberg]

Firefox code moved from custom Liveconnect code to the NPAPI/NPRuntime bridge a while back. Mass-closing the bugs in the liveconnect component which are likely invalid. If you believe that this bug is still relevant to modern versions of Firefox, please reopen it and move it the "Core" product, component "Plug-Ins".

Comment 7

[Alfred Peng]

Comment on attachment 180448 [details] [diff] [review]
patch

Remove old review request which is not needed any more.