Skip to content

Conversation

@ikreymer
Copy link
Member

@ikreymer ikreymer commented Jun 15, 2022

The eval() rewriting can sometimes result in the following scenario:

eval('function test() {}');
...
test();

is rewritten to (simplified) effectively:

(function evalwrapper(x) { eval(rewrite(x)); })('function test() {}')
...
test(); <- test not defined, no longer evaluated in global scope.

The rewriting is done in this way to keep the eval() in this same local scope, but if the eval() is actually in the global scope, ideally, it should be kept there!

Fortunately, it seems that there is a mostly reliable way to detect a function in global scope by checking if arguments.caller.callee is null, eg:

function isGlobalScope() { return !arguments.callee.caller; }

console.log(isGlobalScope()); // global scope, prints 'true'

console.log((function() { return isGlobalScope(); })());  // in function declaration, prints 'false'

console.log((() => isGlobalScope())()); // in arrow function, prints 'false'

The above seems to work in Chrome and Firefox. In Safari 14.x, the last one also prints 'true', so there is at least one false positive. However, this seems to be fixed in Safari 15.x.

As an additional sanity checked, the this can also be checked to ensure that it matches the current window (as an additional guard for arrow function false positive).

Also fortunately, the eval() automatically evaluates in the global scope if eval is called indirectly after assignment.
This allows for this kind of setup:

function wrappedEval(x, isGlobal) {
  var eg = eval;
   return isGlobal ? eg(x) : eval(x);
}

This PR implements this functionality, along with change in server-side rewriting which rewrites:
eval(... -> .eval(this, !arguments.callee.caller, ....

I think this should work w/o breaking anything, but want to leave this up for any feedback..

(Sample site fixed by this: http://svlavra.dn.ua/)

ikreymer added 2 commits June 14, 2022 20:22
attempt to detect if eval() is called from a global scope by adding a function that loads '!arguments.callee.caller' to the wrapped eval.
if eval is global, then call assigned eval `var ge = eval; ge(...)` to force global context again.
This is to support usecase where a global eval(), eg. placed directly in <script> is evaluated in a local scope, causing declared functions and variables to not be global. not perfect (safari may have false negatives) but solves rewriting issue related to global eval()
ikreymer added a commit to webrecorder/wabac.js that referenced this pull request Jun 15, 2022
…ler' test to see if it is in global scope and run eval() in global scope

to work in combination with webrecorder/wombat#89
@ikreymer
Copy link
Member Author

ikreymer commented Jun 15, 2022

See webrecorder/wabac.js#71 for corresponding fixes in wabac.js

It adds rewriting of:
eval(x) to
WB_wombat_runEval2((_______eval_arg, isGlobal) => { var ge = eval; return isGlobal ? ge(_______eval_arg) : eval(_______eval_arg); }).eval(this, (function() { return !arguments.callee.caller })(),x)

by prepending additional arguments and a new function call.
(And fortunately, in case of no args,.eval(this, !arguments.callee.caller,) is also valid)

…' in try/catch in case in strict mode, and if so, set isGlobal to false
@ikreymer
Copy link
Member Author

ikreymer commented Jun 15, 2022

In strict mode, arguments.callee access will throw an exception, so instead rewriting:

eval(x) =>

WB_wombat_runEval2((_______eval_arg, isGlobal) => { var ge = eval; return isGlobal ? ge(_______eval_arg) : eval(_______eval_arg); }).eval(this, (function() { return arguments })(),x)

and handling the args.callee.caller in a try/catch block

@ikreymer ikreymer merged commit 7d970df into main Jun 15, 2022
@ikreymer ikreymer deleted the eval-with-force-global branch June 15, 2022 16:52
ikreymer added a commit to webrecorder/wabac.js that referenced this pull request Jun 15, 2022
* eval improvements: eval() rewriting to pass '.eval(this, arguments, ...)' to allow wombat to test if eval is should be evaluated in local or global scope, to work in to work in combination with webrecorder/wombat#89
(requires wombat 3.3.7)
ikreymer added a commit to webrecorder/wabac.js that referenced this pull request Jun 15, 2022
live proxy config fixes:
- strip out current self origin from x-orig-location if included, using a relative location
- support array list and dict list of live proxy origins

dsrules: raise maxbitrate for twitter video to 5000000 to match pywb/browsertrix-crawler, save maxbitrate used if save option provided

multiwacz loading change:
- compute and hash of wacz to collection path (instead of page id)
- attempt to load from same wacz, if available, otherwise look in all waczs

blockloaders: support for non-http fetch (#67)
- rename HttpRangeLoader -> FetchRangeLoader to indicate support beyond http but with fetch
- check if fetch protocol matches current location protocol, if so, use fetchloader
- check if fetch() to custom protocol returns, assume fetch is supported, if fetch throws an exception, assume it's not supported

remotewarcproxy: Fix kiwix loading (fixes #69)
- get actual prefix from config.prefix instead of sourceUrl, which now stores original 'prefix:...' id
- readd 'process/browser' for standalone build
- update webpack 5 to latest

eval improvements (#71):
- add tests ensuring '_eval' and '$eval' are not rewritten
- support eval being global-scope aware
- update eval rewriting to pass '.eval(this, arguments, ...)' to WB_wombat_runEval2 (from wombat 3.3.7) to allow wombat to determine if eval is should be evaluated in local or global scope, (see: webrecorder/wombat#89 for more details)

dependency update: wombat 3.3.7, warcio 1.5.1, latest webpack 5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants