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

.data() returns by reference instead of a copy, causing the outside to be likely to corrupt the guts on write #1302

Closed
StoneCypher opened this issue Oct 22, 2024 · 0 comments

Comments

@StoneCypher
Copy link
Owner

ugh

john@LAPTOP-PE9BBGOJ MINGW64 ~/projects/jssm (main)
$ node

Welcome to Node.js v22.1.0.
Type ".help" for more information.

> const j = require('.');
undefined

> const m = j.from('a -> b;', { data: { test: 'oldval' } });
undefined

> m.data();
{ test: 'oldval' }

> const z = m.data();
undefined

> z;
{ test: 'oldval' }

> z.test = 'newval';
'newval'

> m.data();
{ test: 'newval' }

the data inside m was changed because:

  1. m.data() returns its internal storage, instead of a copy thereof
  2. we assigned that to z
  3. we altered z, which is a reference to m._data
  4. that mutates m._data, which is almost certainly unintentional

this isn't strictly a bug, but, it violates the principle of least surprise at a level that'll get The Hague's attention

modifying to return a copy, but, this invokes the javascript "what kind of copy" nightmare, which may reflect a need for functional parameterization or to emphasize using a self-manageable class

everything is terrible, especially everything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant