Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fail test - bytenode, ncc, fastify ? #34

Closed
yavorski opened this issue Sep 30, 2019 · 5 comments
Closed

fail test - bytenode, ncc, fastify ? #34

yavorski opened this issue Sep 30, 2019 · 5 comments

Comments

@yavorski
Copy link

#!/usr/bin/env bash

export __TEST_DIR__="bytenode-ncc-fastify-test"

mkdir $__TEST_DIR__
cd $__TEST_DIR__

npm init --yes
npm i fastify -E
npm i bytenode @zeit/ncc -E -D

touch index.js

read -r -d '' INDEX_JS << EOF
  'use strict';
  const build = require('fastify');
  const server = build({ logger: true });
  server.all('/', async () => ({ ok: 'ok' }));
  (async () => await server.listen())();
EOF

echo $INDEX_JS > index.js

npx @zeit/ncc build index.js --out dist
npx bytenode --compile dist/index.js

# this should run?
# but it does not...
# npx bytenode $__TEST_DIR__/dist/index.jsc
npx bytenode dist/index.jsc

# Error =>
# undefined:6
#     ?
#
# SyntaxError: Invalid or unexpected token
#     at Function (<anonymous>)
#     at build (evalmachine.<anonymous>:1:70996)
#     at Object.560 (evalmachine.<anonymous>:1:521710)
#     at __webpack_require__ (evalmachine.<anonymous>:1:769)
#     at Object.360 (evalmachine.<anonymous>:1:395429)
#     at __webpack_require__ (evalmachine.<anonymous>:1:769)
#     at Object.104 (evalmachine.<anonymous>:1:66439)
#     at __webpack_require__ (evalmachine.<anonymous>:1:769)
#     at startup (evalmachine.<anonymous>:1:1228)
#     at evalmachine.<anonymous>:1:1390


#### this however runs ok
####
#### node dist/index.js
#### node $__TEST_DIR__/dist/index.js
#### node ./bytenode-ncc-fastify-test/dist/index.js
@OsamaAbbas
Copy link
Collaborator

After a painful investigation, it seems that the issue lies in fast-json-stringify package.

From the package README file:

fast-json-stringify is significantly faster than JSON.stringify() for small payloads. Its performance advantage shrinks as your payload grows. It pairs well with flatstr, which triggers a V8 optimization that improves performance when eventually converting the string to a Buffer.

So, this might be the cause of the issue. I have not figured out the exact reason why it does not play nice with bytenode. Still investigating.

This is the portion that causes the error by the way:

image

@OsamaAbbas
Copy link
Collaborator

OsamaAbbas commented Oct 1, 2019

Found it.

Here https://github.com/fastify/fast-json-stringify/blob/master/index.js#L53-L62 :

    code += `
    ${$asString.toString()}
    ${$asStringNullable.toString()}
    ${$asStringSmall.toString()}
    ${$asNumber.toString()}
    ${$asNumberNullable.toString()}
    ${$asIntegerNullable.toString()}
    ${$asNull.toString()}
    ${$asBoolean.toString()}
    ${$asBooleanNullable.toString()}

The generated code uses .toString(); function dynamically. Bytenode removes the source code alltogether, so when you call .toString() on any function all you will get is an empty string (or, to be more accurate, a series of zero-width spaces).

So, in principle you can't use bytenode to protect a code that relies on Function.prototype.toString function. Protection here means no [dynamic] access to the function's source code, which contradicts -in principle- what fast-json-stringify is trying to do.

@yavorski
Copy link
Author

yavorski commented Oct 1, 2019

Wow nice thanks!
Makes sense now.

@OsamaAbbas
Copy link
Collaborator

I will add this to the "known limitations" in the README file soon.

Closing now.

@Nihiue
Copy link

Nihiue commented Mar 26, 2021

I got the same issue, some web framework dynamically loads its components, and uses toString to judge whether a class is loaded, which requires operator new to init.

see: https://github.com/miguelmota/is-class/blob/master/is-class.js#L13

I am digging into v8 to find some workaround.

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

No branches or pull requests

3 participants