Passing an array and its size to an XPCOM method does not work

RESOLVED FIXED

Status

Core Graveyard
Java to XPCOM Bridge
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: Vladimir Korenev, Assigned: jhp (no longer active))

Tracking

({fixed1.8.1.8})

Trunk
fixed1.8.1.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.5) Gecko/20060802 Firefox/1.5.0.5
Build Identifier: 

There are some XPCOM methods that accept an array and its size, e.g. nsIWebBrowserStream::appendToStream(). If the size parameter comes after the array, it is not initialized when the array size is being calculated. This causes invalid data being passed to the method.

Reproducible: Always
(Reporter)

Comment 1

11 years ago
Created attachment 233754 [details] [diff] [review]
Patch
(Reporter)

Comment 2

11 years ago
Created attachment 233755 [details]
Test Case
Vladimir, if you want the patch to be reviewed, you need to set the review flags to it, probably jhpedemonte@gmail.com should review it (I'm not sure, though).
Thanks for the patch.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

11 years ago
Comment on attachment 233754 [details] [diff] [review]
Patch

I am currently taking a look at this and the other bugs opened by Vladimir.
Attachment #233754 - Flags: review?(jhpedemonte)
(Reporter)

Comment 5

11 years ago
Created attachment 240614 [details] [diff] [review]
The same patch without indentation

This patch does not change code indentation, so it is clearer.
Attachment #233754 - Attachment is obsolete: true
Attachment #240614 - Flags: review?(jhpedemonte)
Attachment #233754 - Flags: review?(jhpedemonte)

Updated

10 years ago
Duplicate of this bug: 375413
(Assignee)

Comment 7

10 years ago
Created attachment 263398 [details] [diff] [review]
patch

The problem was with methods in which the array size parameter came after the array parameter.  This patch fixes it.  Also handles sized strings, and handles an issue with byte arrays.
Attachment #240614 - Attachment is obsolete: true
Attachment #240614 - Flags: review?(jhpedemonte)
(Assignee)

Updated

10 years ago
Attachment #263398 - Attachment is patch: true
Attachment #263398 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Comment 8

10 years ago
Checked in to trunk. -> FIXED
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
There was no review needed for this?
Also, maybe this needs some automated tests?
(Assignee)

Comment 10

10 years ago
The patch includes tests.  They are run when 'make check' is called in the tests directory.  Currently, I think the only tbox that does this for XULRunner, though, is FF-XR tbox in MozillaExperimental.
(Assignee)

Comment 11

10 years ago
Created attachment 263614 [details] [diff] [review]
dependency fix

Check in caused a build break on the MozillaExperimental tbox.  The dependencies for MozillaInterfaces.jar aren't set up correctly.  This patch should fix it.  Already checked in to trunk.
(Assignee)

Comment 12

10 years ago
Comment on attachment 263398 [details] [diff] [review]
patch

Asking for 1.8.1.4 approval.  This patch is XULRunner only.  One of the files touched is in xpidl, but that bit of code (in xpidl_java) is only exercised when building XULRunner.

This patch is necessary to fix the use of |nsIWebBrowserStream.appendStream()|, an issue that has been reported by several JavaXPCOM users.
Attachment #263398 - Flags: approval1.8.1.4?

Comment 13

10 years ago
+1 :-P
Comment on attachment 263398 [details] [diff] [review]
patch

This is not something we're going to "slip in" if we need to respin our release candidates, so early next time.
Attachment #263398 - Flags: approval1.8.1.4? → approval1.8.1.5?
Comment on attachment 263398 [details] [diff] [review]
patch

Who reviewed the xpidl change? I know it's small, but some ownerish person should still put their stamp on it. Looks like dougt, shaver or bsmedberg maybe.
Comment on attachment 263398 [details] [diff] [review]
patch

Would like a review on at least the xpidl changes.
Attachment #263398 - Flags: review?(benjamin)
Attachment #263398 - Flags: review?(benjamin) → review+
Comment on attachment 263398 [details] [diff] [review]
patch

approved for 1.8.1.5, a=dveditz for release-drivers
Attachment #263398 - Flags: approval1.8.1.5? → approval1.8.1.5+
Comment on attachment 263398 [details] [diff] [review]
patch

closing tree early, this will have to wait for next time
Attachment #263398 - Flags: approval1.8.1.6+
Attachment #263398 - Flags: approval1.8.1.5-
Attachment #263398 - Flags: approval1.8.1.5+
(Assignee)

Updated

10 years ago
Keywords: fixed1.8.1.8
Can the original reporter please verify this fix with the 2.0.0.8 RC2 build of Firefox? If it cannot be verified with 2.0.0.8, can QA please be told of a way to verify the fix is present in our code?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.