Closed
Bug 1131707
Opened 10 years ago
Closed 10 years ago
XPConnect Function forwarders don't properly forward construction
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: fdsc, Assigned: bholley)
References
Details
Attachments
(3 files, 2 obsolete files)
User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.93 Safari/537.36
Steps to reproduce:
My extension "Http UserAgent Cleaner" have function for substitution time zone.
Example
var httpuacleaner_getTimezoneOffset = Cu.exportFunction(function()
{
return -valTZ;
}, window.wrappedJSObject);
Object.defineProperty(window.wrappedJSObject.Date.prototype, "getTimezoneOffset", {value: httpuacleaner_getTimezoneOffset, enumerable : true, configurable : true});
---------
However, when I try the whole Date constructor with their function, the Date has been exported without prototype. I did not find help on how to export the prototype to create the object.
However, this means the inability to create a full object with the possibility of export.
Actual results:
Exported Date.prototype is undefinded. Date instanceof Function is true.
Expected results:
Date.prototype must be exported
Summary: Components.utils.exportFunction does not export prototype to window.wrappedJSObject → Components.utils.exportFunction does not export new date.prototype to window.wrappedJSObject
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to fdsc from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/537.36 (KHTML, like
> Gecko) Chrome/27.0.1453.93 Safari/537.36
>
> Steps to reproduce:
>
> My extension "Http UserAgent Cleaner" have function for substitution time
> zone.
>
> Example
>
> var httpuacleaner_getTimezoneOffset = Cu.exportFunction(function()
> {
> return -valTZ;
> }, window.wrappedJSObject);
>
> Object.defineProperty(window.wrappedJSObject.Date.prototype,
> "getTimezoneOffset", {value: httpuacleaner_getTimezoneOffset, enumerable :
> true, configurable : true});
>
> ---------
>
> However, when I try the whole Date constructor with their function, the Date
> has been exported without prototype. I did not find help on how to export
> the prototype to create the object.
Can you clarify what you mean by this? Maybe one of:
|new Date();|
|new window.Data();|
|new window.wrappedJSObject.Date()|
|window.eval('new Date()')|
See https://developer.mozilla.org/en-US/docs/Xray_vision
Hello
Install extension from attachement, load page with JavaScript enabled.
In console FireBug enter commands and see
// For Object.defineProperty(window.wrappedJSObject.Date.prototype,"toString"
// if in the extension make with uncomment code
> new Date()
Date {This is DatePTest Date Object.toString}
> Date()
"Mon May 04 2015 23:51:57 GMT+0300"
Date().toString()
"Mon May 04 2015 23:52:36 GMT+0300"
// For Object.defineProperty(window.wrappedJSObject, "Date"
// In extension now
// It's work. Time has been changed
> Date()
Date {Sat May 02 2015 02:00:20 GMT+0300}
// It's not work
> new Date()
TypeError: Date is not a constructor
> Date.prototype
undefined
---------------------------------------------------------------------------
Code:
var DateObject = Cu.exportFunction(function(val)
{
console.error('DateObject called');
if (val)
{
console.error('with walue ' + value);
return new window.Date(val - 1000*3600*7*10);
}
else
{
console.error('without walue');
return new window.Date(new window.Date().getTime() - 1000*3600*7*10);
}
}, window.wrappedJSObject);
var DateObjectClassStr = Cu.exportFunction(function()
{
return 'This is DatePTest Date Class.toString';
}, window.wrappedJSObject);
DateObject.prototype = Cu.createObjectIn(window.wrappedJSObject);
DateObject.prototype.toString = DateObjectClassStr;
Object.defineProperty(window.wrappedJSObject, "Date", {value: DateObject, enumerable : true, configurable : true});
---------------------------------------------------------------------------
In DateObject constructor function returned object.
E.g.
var F = function() {return {a: 'b'};};
F() returned
Object { a="b"}
new F() returned
Object { a="b"}
But Date() returned Date
new Date() returned error
Similary new window.Date() and eval('new Date()')
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to fdsc from comment #2)
> DateObject.prototype = Cu.createObjectIn(window.wrappedJSObject);
This isn't going to work - DateObject itself is just a wrapper around the underlying closure that you pass to exportFunction. DateObject.prototype will not automatically be the prototype of the objects that your anonymous function creates.
(In reply to fdsc from comment #2)
> var customProto = Cu.cloneInto({ toString: DateObjectClassStr }, window.wrappedJSObject, { cloneFunctions: true });
> var DateObject = Cu.exportFunction(function(val)
> {
> console.error('DateObject called');
> var d;
> if (val)
> {
> console.error('with walue ' + value);
> d = new window.Date(val - 1000*3600*7*10);
> }
> else
> {
> console.error('without walue');
> d = window.Date(new window.Date().getTime() - 1000*3600*7*10);
> }
> Object.setPrototypeOf(d.wrappedJSObject, customProto);
> }, window.wrappedJSObject);
But do you really want to be creating an entirely custom proto? It seems like you should just patch over 'toString' on the existing proto.
> do you really want to be creating an entirely custom proto?
I already have the toString function and many other functions-substitutes in prototype. They are in https://addons.mozilla.org/ru/firefox/addon/http-useragent-cleaner/ now (in developer channel). If want, see lib/main.js file with keyword window.wrappedJSObject.Date.prototype
But I need to control precisely the object constructor and Date() function calls.
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to fdsc from comment #4)
> But I need to control precisely the object constructor and Date() function
> calls.
Right - so just override the date constructor like you're doing - no need to mess with the prototype. |new window.Date(...)| will create an object whose prototype is window.Date.prototype (though you'll need to waive Xrays to see the custom modifications from chrome).
I need to override three things:
1. new Date()
2. Date()
3. new Date().functions calls
It is not only the constructor
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to fdsc from comment #6)
> I need to override three things:
> 1. new Date()
> 2. Date()
These two can be fixed by overriding the constructor...
> 3. new Date().functions calls
...and this by monkey-patching window.Date.prototype.wrappedJSObject, right? Or am I missing something?
It doesn't work.
I think that the old prototype is overwritten with the overwrite new constructor. It would be strange if it worked, and it really doesn't work.
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to fdsc from comment #8)
> Created attachment 8601178 [details]
> workaround not work
Can you please post a reduced code snippet rather than attaching the entire add-on? The issue we're discussing here is a very specific one, and most situations can be demonstrated in a couple of lines of independent sample code.
Reporter | ||
Comment 10•10 years ago
|
||
I just did what you said. If you want to see the code, I have attached an illustration in extension.
If you trust me, cause code is not necessary. It trivial. If not, you should see a working version, as excerpts of code can be wrong.
In the attached addition there is nothing interesting, except that I already wrote. Your suggestion does not work. I have attached in case you want to look at working code.
Assignee | ||
Comment 11•10 years ago
|
||
I'd like to help you, but I don't have time to look through the entire extension. If you want to continue here, please post a small self-contained snippet that I can execute in scratchpad which demonstrates the problem you're running into.
Reporter | ||
Comment 12•10 years ago
|
||
I told you that your solution does not work. I don't understand what code you want.
If you are too lazy to unzip the file, I will do it for you.
const {Cc, Ci, Cu, Cr, Cm, components} = require("chrome");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
var DatePTest = {};
DatePTest.getProtocolForDocument = function(document, withoutDoublePoint)
{
var protocol = '';
if (document.location && document.location.protocol)
protocol = document.location.protocol;
else
try
{
protocol = /^[^:]+:/i.exec(document.URL)[0];
}
catch (e)
{
return "";
}
if (withoutDoublePoint)
return protocol.substring(0, protocol.length - 1);
return protocol;
};
DatePTest.observer = function(subject)
{
var document = subject.subject;
var aTopic = subject.type;
var aData = subject.data;
if (!document || (!document.URL && !document.location)/* || !document.defaultView || !document.defaultView.navigator*/)
{
return;
}
var protocol = "";
protocol = DatePTest.getProtocolForDocument(document);
if (
protocol == "resource:"
|| protocol == "data:"
|| protocol == "about:"
|| protocol == "moz-safe-about:"
|| protocol == "moz-filedata:"
|| protocol == "moz-icon:"
|| protocol == "mediasource:"
|| protocol == "chrome:"
|| protocol == "blob:"
|| protocol == "file:"
|| protocol == "view-source:"
)
{
// console.warn("HTTP UA Cleaner ignored document " + document.URL/*document.location.href*/);
return;
}
var window = document.defaultView;
if (!window)
return;
var DateObject = Cu.exportFunction(function(val)
{
console.error('DateObject called');
if (val)
{
console.error('with walue ' + value);
return new window.Date(val - 1000*3600*7*10);
}
else
{
console.error('without walue');
return new window.Date(new window.Date().getTime() - 1000*3600*7*10);
}
}, window.wrappedJSObject);
/*
var GetDateObject = Cu.exportFunction(function(val)
{
return DateObject;
}, window.wrappedJSObject);
*/
var DateObjectStr = Cu.exportFunction(function()
{
return 'This is DatePTest Date Object.toString';
}, window.wrappedJSObject);
var DateObjectClassStr = Cu.exportFunction(function()
{
return 'This is DatePTest Date Class.toString';
}, window.wrappedJSObject);
/*
DateObject.prototype = Cu.createObjectIn(window.wrappedJSObject);
DateObject.prototype.toString = DateObjectClassStr;
*/
/*if (1 == 1)
{*/
Object.defineProperty(window.wrappedJSObject, "Date", {value: DateObject, enumerable : true, configurable : true});
/*}
else
{*/
Object.defineProperty(window.Date.prototype.wrappedJSObject, "toString", {value: DateObjectStr, enumerable : true, configurable : true});
//}
};
exports.main = function (options, callbacks)
{
var events = require("sdk/system/events");
events.on("document-element-inserted", DatePTest.observer);
};
exports.onUnload = function (reason)
{
};
Reporter | ||
Comment 13•10 years ago
|
||
Previous implementation was incorrect.
> Date.prototype
Date {This is DatePTest Date Object.toString}
> Date()
Date {This is DatePTest Date Object.toString}
> new Date()
TypeError: Date is not a constructor
Date.prototype succeed to override.
But in prototype have overrided object. And Date not a constructor.
I tried to override it in the following ways
// 1
var a = window.Date.prototype;
... (Date override)
window.wrappedJSObject.Date.prototype = a;
// 2
window.wrappedJSObject.Date = Cu.cloneInto(window.Date.prototype, window.wrappedJSObject);
// 3
var PGet = function()
{
console.error('PGet');
return Cu.cloneInto(window.Date.prototype, window.wrappedJSObject);
};
var PSet = function()
{
console.error('PSet');
};
Object.defineProperty(window.wrappedJSObject.Date, "prototype", {get: PGet, set: PSet, enumerable : true, configurable : true});
// 4
window.wrappedJSObject.Date.prototype = Cu.cloneInto(window.Date.prototype, window.wrappedJSObject);
Attachment #8601178 -
Attachment is obsolete: true
No longer depends on: 861219
Summary: Components.utils.exportFunction does not export new date.prototype to window.wrappedJSObject → Components.utils.exportFunction does not export date.prototype and other func.prototype to window.wrappedJSObject (exported func not constructor)
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to fdsc from comment #12)
> If you are too lazy to unzip the file, I will do it for you.
Hey now - I'm under no obligation to help you out here. If you're hoping to get people to voluntarily take time and offer suggestions, it helps to be more friendly. ;-)
The reason self-contained testcases are useful is that they allow people to focus on the problem at hand and actually _execute_ the sample code in a simple way. You may be very familiar with your extension, but a random person helping you in their spare time is unlikely to want to install your extension, figure out how it works, and reproduce the issue.
I just threw together a testcase of the form I was asking for. You should be able to execute it in a browser scratchpad, and see it work properly.
Because I was able to run the code while hacking it up, I noticed that exported functions don't transparently forward the |construct| bit, which means that they treat |new foo()| and |foo()| identically (this may or may not be the problem you were running into). This is usually fine, but Date has different behavior between the two for legacy reasons. The eval() workaround in the sample code should work fine for your purposes, but I'll file a bug to fix things to be more transparent.
var sb = new Cu.Sandbox(null);
var dateCtor = sb.Date;
var dateProto = sb.Date.prototype;
var wrappedCtor = Cu.exportFunction(function(val) {
val = val || (new Date()).getTime();
var d = new dateCtor(val - 1000*3600*7*10);
console.log(Cu.waiveXrays(d).getTime());
return d;
}, sb);
Cu.waiveXrays(sb).__wrappedCtor = wrappedCtor;
sb.eval('Date = function(val) { "use strict"; return this ? __wrappedCtor(val) : __wrappedCtor(val).toString();};')
dateProto.wrappedJSObject.toString = Cu.exportFunction(function() { return "custom date string"; }, sb);
((new Date()).getTime()) + ", " +
sb.eval('(new Date()).getTime()') + ", " +
(new Date().toString()) + ", " +
sb.eval('(new Date()).toString()')
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #14)
> I'll file a bug to fix things to be more transparent.
Actually I'll just use this bug, if that's alright with you. Please file followups if you run into additional issues.
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8601701 -
Flags: review?(gkrizsanits)
Assignee | ||
Updated•10 years ago
|
Summary: Components.utils.exportFunction does not export date.prototype and other func.prototype to window.wrappedJSObject (exported func not constructor) → XPConnect Function forwarders don't properly forward construction
Assignee | ||
Comment 17•10 years ago
|
||
Reporter | ||
Comment 18•10 years ago
|
||
Frankly, I don't understand.
I don't understand how you run the privileged code without starting the extension.
I completely misunderstood that code that you mentioned. Could you explain where that execute.
Your code is too complicated for me. I don't think this code is simpler than the extension.
---------------------------------------------------------------------------------
And, I noticed that the constructor works without exporting. I'm not sure that this is not a bug. It seemed to me that such functions should be prohibited for export.
Object.defineProperty(window.wrappedJSObject, 'Date',
{value: function(val)
{
if (this.document || this.Cu)
{
return new window.Date().wrappedJSObject.toString();
}
if (val)
{
return new window.Date(val);
}
else
{
return new window.Date();
}
}
, enumerable : true, configurable : true
});
And new Date() and Date() is works. (and eval is too)
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to fdsc from comment #18)
> I don't understand how you run the privileged code without starting the
> extension.
https://developer.mozilla.org/en-US/docs/Tools/Scratchpad#Running_Scratchpad_in_the_browser_context
(Sorry, I seem to remember pasting that link into this bug before, but I guess I didn't).
You can also go to |about:| and paste it into the console.
> And, I noticed that the constructor works without exporting. I'm not sure
> that this is not a bug. It seemed to me that such functions should be
> prohibited for export.
For legacy reasons, we need to allow certain functions to be called. But they don't appear like normal functions (and you can't do .bind etc on them), which is why exportFunction is preferable.
Comment 20•10 years ago
|
||
Comment on attachment 8601701 [details] [diff] [review]
Transparently forward the construct bit for function forwarders. v1
Review of attachment 8601701 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/xpconnect/src/ExportHelpers.cpp
@@ +328,2 @@
> if (!thisObj) {
> return false;
I don't get it. Seems like this will return false always in the args.isConstructing case now... which I don't think is the goal here. (also the {} seems to be unnecessary)
@@ +346,5 @@
>
> RootedValue fval(cx, ObjectValue(*unwrappedFun));
> + if (args.isConstructing()) {
> + if (!JS::Construct(cx, fval, args, args.rval())) {
> + return false;
Nit: extra {}
Attachment #8601701 -
Flags: review?(gkrizsanits) → review-
Reporter | ||
Comment 21•10 years ago
|
||
> sb.eval('Date = function(val) { "use strict"; return this ? __wrappedCtor(val) : __wrappedCtor(val).toString();};')
It's all wonderful. But what I need to replace sb.eval in the extension? Am I have the eval command inside the window in extension privileged code?
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to fdsc from comment #21)
> It's all wonderful. But what I need to replace sb.eval in the extension? Am
> I have the eval command inside the window in extension privileged code?
You can do window.eval over Xrays, yes.
Be careful though that replacing the Date constructor on arbitrary webpages may cause problems if those webpages are also doing funny things with Date.
Also, in the above code |new Date() instanceof Date| will return false, which will probably break things. You should be able to fix this by doing |wrappedCtor.wrappedJSObject.prototype = dateProto|. You should test it to make sure it works.
Assignee | ||
Comment 23•10 years ago
|
||
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #20)
> I don't get it. Seems like this will return false always in the
> args.isConstructing case now... which I don't think is the goal here. (also
> the {} seems to be unnecessary)
Hah, good catch! Yeah, that pre-existing code seems pretty bogus, and it was actually preventing the failures in the test from being caught - see bug 1162187.
Assignee | ||
Comment 24•10 years ago
|
||
Attachment #8601701 -
Attachment is obsolete: true
Attachment #8602261 -
Flags: review?(gkrizsanits)
Comment 25•10 years ago
|
||
Comment on attachment 8602261 [details] [diff] [review]
Transparently forward the construct bit for function forwarders. v2
Review of attachment 8602261 [details] [diff] [review]:
-----------------------------------------------------------------
(In reply to Bobby Holley (:bholley) from comment #23)
> actually preventing the failures in the test from being caught - see bug
> 1162187.
Was wondering how did the test pass... Fun times... :)
Attachment #8602261 -
Flags: review?(gkrizsanits) → review+
Comment 26•10 years ago
|
||
Comment 27•10 years ago
|
||
Assignee: nobody → bobbyholley
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 28•7 years ago
|
||
Moving from Core::Untriaged to Core::General https://bugzilla.mozilla.org/show_bug.cgi?id=1407598
Component: Untriaged → General
You need to log in
before you can comment on or make changes to this bug.
Description
•