1418769 - Remove toSource and uneval

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tom S [:evilpie]]

Almost all built-ins in Firefox have a 'toSource' prototype method that is non-standard. uneval also tries to call 'toSource' or falls back to generic code for serializing objects.

'toSource' can be a useful debugging tool, but at least within Firefox we can use the better devtools instead.

Comment 1

[Nicolas B. Pierron [:nbp]]

Note, our test suite depends on that to avoid being too verbose. I guess most of these could be replace by multiple line strings, but we should double check if this is fine for fuzzers.

Comment 2

[Christian Holler (:decoder)]

I've been using uneval in the past in fuzzing (especially for testcase reduction when combining multiple files). But at some point I switched to template strings because that makes a lot of things easier. I just checked and there is no dependency to either uneval or toSource in LangFuzz.

Putting a needinfo on :gkw though to check jsfunfuzz.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Thanks for the headsup, decoder.

A cursory jsfunfuzz check shows that toSource seems easy to remove:

https://github.com/MozillaSecurity/funfuzz/search?utf8=%E2%9C%93&q=toSource&type=

but uneval seems more prevalent:

https://github.com/MozillaSecurity/funfuzz/search?utf8=%E2%9C%93&q=uneval&type=

With the decompiler gone, the last I heard about reconstructing a prettified testcase from a minified one was something via AST, but I'm not sure what that led to. Now we use external beautifiers, but just sayin'.

I'll keep an eye on this bug, don't let this deter you folks from whatever you plan to do with uneval and toSource.

Comment 4

[Jason Orendorff [:jorendorff]]

Totally OK with dropping this stuff, especially toSource. Maybe we should keep uneval as a testing function...?

Comment 5

[Jan de Mooij [:jandem]]

We should consider adding a RealmOption for this stuff. Then we could easily turn this off for content code and reduce our attack surface, without worrying about browser chrome/devtools and shell consumers. Just need to make sure the Xray code works correctly.

Comment 6

[Tom S [:evilpie]]

I will look into Jan's idea. However conditionally defining functions is always a bit complicated, but I think we can solve this by introducing a JSFunctionSpec for toSource, or maybe just match the name?

Comment 7

[Michal Čaplygin [:myf]]

I know it is too late to enter discussion, but I feel obligated to plead in favor keeping toSource, at least in devtools (Web Console) context (if possible).

While Rich Outputs ("Xrays"(?)) in the Console are excellent and keep improving, I've found invoking good old trusty toSource very convenient to this days:

• On strings it unambiguously reveals control and "non-ASCII" code points (i.e. something that could be cumbersomely emulatable with .split and .map):
  • » '🛩️\r\n🛩'.toSource()
  • « "(new String(\"\\uD83D\\uDEE9\\uFE0F\\r\\n\\uD83D\\uDEE9\"))"
• On objects it conveniently reveals all method sources (i.e. something that could be hardly emulated in concise oneliner):
  • » ({a:function(){/*foo*/},b:function(){/*bar*/}}).toSource()
  • « "({a:(function(){/*foo*/}), b:(function(){/*bar*/})})"

Even though this plead is most probably futile, I owe this ancient little gem too much time it saved me and too much pride for neat non-standard benefit in favourite browser it gave me to remain silent. Then be this comment a sincere eulogy.

---

(I don't remember I've ever used uneval, but I can imagine I could switch to it if it remained as the only option after toSource removal.)

Comment 8

[Tom S [:evilpie]]

After removing toSource completely, the following tests (D56935 and D56940) that use toSource only conditionally should be updated.