1081385 - exportFunction produces wrong .length attribute

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[kkapsner]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140923175406

Steps to reproduce:

I used exportFunction() to create a callable function in the unprivileged (unsafeWindow) context. In the loaded document I accessed the .length attribute of this function


Actual results:

I got a value of 0.


Expected results:

The real number of arguments of the exported function should be returned.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Gabor, can you look at this and put it in the right component?

Comment 2

[Gabor Krizsanits (INACTIVE)]

I think the fix is to get the length of the |callable| here http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/ExportHelpers.cpp#330 and then pass it to NewFunctionByIdWithReserved. I think this would be a good first bug candidate.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Do we want to just do .length, or JS_GetFunctionArity combined with JS_GetObjectFunction (and bail on non-function callables, but get "saner" results for functions if someone mucked with .length on them)?

Comment 4

[Gabor Krizsanits (INACTIVE)]

(In reply to Boris Zbarsky [:bz] from comment #3)
> Do we want to just do .length, or JS_GetFunctionArity combined with
> JS_GetObjectFunction (and bail on non-function callables, but get "saner"
> results for functions if someone mucked with .length on them)?

I would do JS_GetFunctionArity. I don't see any reason to port any mockery on the .length property to the exported version.

Comment 5

[Boris Zbarsky [:bzbarsky]]

It basically depends on what we want the behavior to be when exporting function proxies or other non-function callables.

Comment 6

[kkapsner]

Are there any news on this bug?

Comment 7

[Henry Miller]

A bit of background why I think is is important. It allows fingerprinting the browser for addons that use exportFunction() by checking if the length attribute matches the expected value.

Comment 8

[kkapsner]

Not only the value of the length property is wrong but also the property descriptor: ECMA says that [[Configurable]] should be true but after exportFunction it's false.

This prevents me from fixing this bug on the addon side.

Comment 9

[kkapsner]

Is this really that hard to fix? Or just not interesting enough?

Comment 10

[Boris Zbarsky [:bzbarsky]]

The hard part is that it's not clear what the right fix is for various edge cases.

That said I'm happy to just make a call on it.  Let's do comment 4.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Give functions created via exportFunction the same .length as the function being exported

MozReview-Commit-ID: 7NVkm2rqfs4

Comment 12

[kkapsner]

Thanks for taking the responsibility.

Comment 13

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8936711 [details] [diff] [review]
Give functions created via exportFunction the same .length as the function being exported

Review of attachment 8936711 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this.

::: js/xpconnect/src/ExportHelpers.cpp
@@ +334,5 @@
> +    unsigned nargs = 0;
> +    RootedObject unwrapped(cx, js::UncheckedUnwrap(callable));
> +    if (unwrapped) {
> +        if (JSFunction* fun = JS_GetObjectFunction(unwrapped)) {
> +            nargs = JS_GetFunctionArity(fun);

Nit: I think in js/xpconnect the {} should be avoided for one liner if branches (that used to be the case at least, I haven't been following any coding style discussions lately). Also it might make sense to merge the two if's.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> the {} should be avoided for one liner if branches 

Will do!

> Also it might make sense to merge the two if's.

Can't do that, because the second "if" is declaring "fun".

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74c036bf1c66
Give functions created via exportFunction the same .length as the function being exported.  r=gabor

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/74c036bf1c66