v2.x - do not mutate core-util-is module

[kumavis]

report

v2.x modifes the exports of core-util-is

v2.x is still used by browserify's via stream-browserify

The replacements inserted by the build process include some workaround for util and inherits. This seems to be an unintentional modification of the core-util-is.

readable-stream/lib/_stream_readable.js

Lines 66 to 69 in b3cf9b1

	/*<replacement>*/
	var util = require('core-util-is');
	util.inherits = require('inherits');
	/*</replacement>*/

fix

The fix uses Object.create(...) to create a new object with the core-util-is module.exports set as its prototype. This means all the content of the original module.exports is exposed but the object can be decorated without mutating core-util-is.

I originally used Object.assign({}, ...) but Object.create has even wider support.

platform support details

• Object.create (ES5)
• Object.assign (ES6)

context:

I'm working on a set of tools to help defend against software supplychain attacks. One of the imposed limitations is that you cannot modify the module.exports of a module from another package. readable-stream@2.x is one of the only packages I've found that breaks this limitation. readable-stream@3.x has such issue.

[mcollina]

v2.x still support Node.js 0.8, and I think Object.create() is not present there. Can you address that?

Overall I would concentrate your efforts in getting stream-browserify updated to v3.x as v2.x is ancient.

[kumavis]

test errors seem unrelated

[mcollina]

Does this apply to v3 as well?

[kumavis]

@mcollina thanks for the quick response!

did a quick test:
Object.create does work in node v0.8
Object.assign is not present in node v0.8

readable-stream on  v2-no-util-mut [?] is 📦 v2.3.6 via ⬢ v0.8.28 took 18s 
⇡0% ➜ node
> process.version
'v0.8.28'
> Object.create
[Function: create]
> x={a:1};y=Object.create(x)
{}
> y.b=2
2
> y.b
2
> y.a
1
> x
{ a: 1 }
> Object.assign
undefined

[kumavis]

v3.x does not mutate core-util-is 👍

readable-stream/lib/_stream_readable.js

Line 92 in edd8c2d

require('inherits')(Readable, Stream);

[kumavis]

Overall I would concentrate your efforts in getting stream-browserify updated to v3.x as v2.x is ancient.

I agree, though I am not a maintainer on browserify/* and v3 is a breaking change. Trying to get the minimal fix through 🙇‍♂️

seems stream-browserify CI expects 0.8 support

here is a relevant browserify land stuff
PR browserify/stream-browserify#18
issue browserify/stream-browserify#17
issue browserify/browserify#1880

[kumavis]

@mcollina sorry to bring this ancient maintenance task upon you but it would unblock me on my quest to improve the security of open source projects. browserify's technical debt should not be your burden to bear. I will attempt to push through a breaking change on their end for streams@3

I humbly ask for your time in landing this 2.x patch

[kumavis]

@mcollina seems like the CI tries to install npm v6 which is not supported by node v4

[kumavis]

@mcollina Resolved the CI issues that were already present in the v2.x branch. modified the CI script to only update the npm version for those flagged legacy. otherwise default npm is used.

[ljharb]

It would be great to land this, and make the transition from v2 to v3 easier for people by reducing the delta between them.

[mcollina]

@ljharb from an API perspective, readable-stream v2 is a cut of the stream module from
Node 8 (there has been no semver-major changes from Node 4 to 8). readable-stream v3 is a cut from Node 10. readable-stream v4 will be a cut from 12, or possibly 14. This module has very little control on what it can ship, minus the minimal Node.js and browser version.

Given the massive adoption of new ES5 and ES6 features in Node core, it has been deemed too hard for the current maintainers to ship support for IE < 11 and old nodes in v3: making the test harness “pass” with polyfills of new JS features was gargantuan with very little benefit. We are of course happy to take PRs that improve that situation.

(I’m currently on vacation, I’ll land this as soon as I can).

[mcollina]

lgtm

[mcollina]

released as v2.3.7.