Last Comment Bug 348710 - Passing an array and its size to an XPCOM method does not work
: Passing an array and its size to an XPCOM method does not work
: fixed1.8.1.8
Product: Core Graveyard
Classification: Graveyard
Component: Java to XPCOM Bridge (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: jhp (no longer active)
: 375413 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2006-08-15 02:58 PDT by Vladimir Korenev
Modified: 2014-09-24 05:43 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Patch (7.24 KB, patch)
2006-08-15 02:59 PDT, Vladimir Korenev
no flags Details | Diff | Splinter Review
Test Case (7.26 KB, text/plain)
2006-08-15 03:01 PDT, Vladimir Korenev
no flags Details
The same patch without indentation (2.88 KB, patch)
2006-09-29 06:59 PDT, Vladimir Korenev
no flags Details | Diff | Splinter Review
patch (42.52 KB, patch)
2007-05-01 15:23 PDT, jhp (no longer active)
benjamin: review+
dveditz: approval1.8.1.5-
dveditz: approval1.8.1.8+
Details | Diff | Splinter Review
dependency fix (1.09 KB, patch)
2007-05-03 09:22 PDT, jhp (no longer active)
no flags Details | Diff | Splinter Review

Description Vladimir Korenev 2006-08-15 02:58:59 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20060802 Firefox/
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
Comment 1 Vladimir Korenev 2006-08-15 02:59:45 PDT
Created attachment 233754 [details] [diff] [review]
Comment 2 Vladimir Korenev 2006-08-15 03:01:23 PDT
Created attachment 233755 [details]
Test Case
Comment 3 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-08-24 04:09:41 PDT
Vladimir, if you want the patch to be reviewed, you need to set the review flags to it, probably should review it (I'm not sure, though).
Thanks for the patch.
Comment 4 jhp (no longer active) 2006-08-24 08:01:26 PDT
Comment on attachment 233754 [details] [diff] [review]

I am currently taking a look at this and the other bugs opened by Vladimir.
Comment 5 Vladimir Korenev 2006-09-29 06:59:28 PDT
Created attachment 240614 [details] [diff] [review]
The same patch without indentation

This patch does not change code indentation, so it is clearer.
Comment 6 zug_treno 2007-03-28 00:50:47 PDT
*** Bug 375413 has been marked as a duplicate of this bug. ***
Comment 7 jhp (no longer active) 2007-05-01 15:23:40 PDT
Created attachment 263398 [details] [diff] [review]

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.
Comment 8 jhp (no longer active) 2007-05-02 09:40:03 PDT
Checked in to trunk. -> FIXED
Comment 9 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-05-02 09:42:34 PDT
There was no review needed for this?
Also, maybe this needs some automated tests?
Comment 10 jhp (no longer active) 2007-05-02 09:57:21 PDT
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.
Comment 11 jhp (no longer active) 2007-05-03 09:22:41 PDT
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.
Comment 12 jhp (no longer active) 2007-05-03 09:33:29 PDT
Comment on attachment 263398 [details] [diff] [review]

Asking for 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.
Comment 13 Philippe Ombredanne 2007-05-03 09:45:19 PDT
+1 :-P
Comment 14 Daniel Veditz [:dveditz] 2007-05-10 08:27:53 PDT
Comment on attachment 263398 [details] [diff] [review]

This is not something we're going to "slip in" if we need to respin our release candidates, so early next time.
Comment 15 Daniel Veditz [:dveditz] 2007-06-25 10:57:17 PDT
Comment on attachment 263398 [details] [diff] [review]

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 16 Daniel Veditz [:dveditz] 2007-06-28 11:58:38 PDT
Comment on attachment 263398 [details] [diff] [review]

Would like a review on at least the xpidl changes.
Comment 17 Daniel Veditz [:dveditz] 2007-07-02 11:12:10 PDT
Comment on attachment 263398 [details] [diff] [review]

approved for, a=dveditz for release-drivers
Comment 18 Daniel Veditz [:dveditz] 2007-07-11 13:52:43 PDT
Comment on attachment 263398 [details] [diff] [review]

closing tree early, this will have to wait for next time
Comment 19 Al Billings [:abillings] 2007-10-15 17:42:52 PDT
Can the original reporter please verify this fix with the RC2 build of Firefox? If it cannot be verified with, can QA please be told of a way to verify the fix is present in our code?

Note You need to log in before you can comment on or make changes to this bug.