Skip to content

Protect against combine argument being broken via Array.slice#801

Merged
zerobias merged 7 commits intomasterfrom
fix-combine-leaking-internals
Oct 22, 2022
Merged

Protect against combine argument being broken via Array.slice#801
zerobias merged 7 commits intomasterfrom
fix-combine-leaking-internals

Conversation

@AlexandrHoroshih
Copy link
Member

@AlexandrHoroshih AlexandrHoroshih commented Oct 9, 2022 •
edited
Loading

Effector uses Array.slice internally to prepare argument object for a user, there is a case, where it was broken

Changelog:

  • Added tests against breaking combine argument in userland via .slice = undefined or similiar code
  • Changed arr.slice() to [...arr]

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2022 •
edited
Loading

🚛 size-compare report

Comparing e835447b...8a5a32ee

File +/- Base Current +/- gzip Base gzip Current gzip
npm/effector/compat.js +0.01% 27.7 kB 27.7 kB +0.01% 11 kB 11 kB
npm/effector/effector.cjs.js -0.01% 24.2 kB 24.2 kB = 10.4 kB 10.4 kB
npm/effector/effector.mjs -0.01% 24.2 kB 24.2 kB = 10.4 kB 10.4 kB
npm/effector/effector.umd.js -0.01% 24.1 kB 24.1 kB -0.01% 10.4 kB 10.4 kB
Files wasn't changed
File +/- Base Current +/- gzip Base gzip Current gzip
npm/effector/babel-plugin-react.js = 6.14 kB 6.14 kB = 1.57 kB 1.57 kB
npm/effector/babel-plugin.js = 27.1 kB 27.1 kB = 5.12 kB 5.12 kB
npm/effector/fork.js = 377 B 377 B = 233 B 233 B
npm/effector/fork.mjs = 197 B 197 B = 167 B 167 B
npm/effector-react/compat.js = 8.57 kB 8.57 kB = 3.33 kB 3.33 kB
npm/effector-react/effector-react.cjs.js = 7.88 kB 7.88 kB = 3.21 kB 3.21 kB
npm/effector-react/effector-react.mjs = 7.52 kB 7.52 kB = 3.2 kB 3.2 kB
npm/effector-react/effector-react.umd.js = 8.44 kB 8.44 kB = 3.3 kB 3.3 kB
npm/effector-react/scope.js = 7.45 kB 7.45 kB = 3.09 kB 3.09 kB
npm/effector-react/scope.mjs = 7.12 kB 7.12 kB = 3.1 kB 3.1 kB
npm/effector-react/ssr.js = 571 B 571 B = 263 B 263 B
npm/effector-react/ssr.mjs = 230 B 230 B = 171 B 171 B
npm/effector-solid/effector-solid.cjs.js = 4.04 kB 4.04 kB = 1.9 kB 1.9 kB
npm/effector-solid/effector-solid.mjs = 3.83 kB 3.83 kB = 1.92 kB 1.92 kB
npm/effector-solid/effector-solid.umd.js = 4.28 kB 4.28 kB = 1.97 kB 1.97 kB
npm/effector-solid/scope.js = 3.75 kB 3.75 kB = 1.74 kB 1.74 kB
npm/effector-solid/scope.mjs = 3.5 kB 3.5 kB = 1.74 kB 1.74 kB
npm/effector-vue/compat.js = 1.79 kB 1.79 kB = 864 B 864 B
npm/effector-vue/composition.cjs.js = 4.19 kB 4.19 kB = 1.88 kB 1.88 kB
npm/effector-vue/composition.mjs = 4 kB 4 kB = 1.91 kB 1.91 kB
npm/effector-vue/effector-vue.cjs.js = 1.71 kB 1.71 kB = 832 B 832 B
npm/effector-vue/effector-vue.mjs = 1.51 kB 1.51 kB = 800 B 800 B
npm/effector-vue/effector-vue.umd.js = 1.98 kB 1.98 kB = 938 B 938 B
npm/effector-vue/ssr.cjs.js = 561 B 561 B = 346 B 346 B
npm/effector-vue/ssr.mjs = 540 B 540 B = 331 B 331 B
npm/forest/forest.cjs.js = 34.3 kB 34.3 kB = 10.6 kB 10.6 kB
npm/forest/forest.mjs = 33.6 kB 33.6 kB = 10.6 kB 10.6 kB
npm/forest/forest.umd.js = 34.5 kB 34.5 kB = 10.6 kB 10.6 kB
npm/forest/server.js = 5.79 kB 5.79 kB = 2.06 kB 2.06 kB
npm/forest/server.mjs = 5.76 kB 5.76 kB = 2.03 kB 2.03 kB

@sergeysova sergeysova deleted the fix-combine-leaking-internals branch October 11, 2022 09:53
@AlexandrHoroshih AlexandrHoroshih restored the fix-combine-leaking-internals branch October 22, 2022 10:22
@AlexandrHoroshih AlexandrHoroshih force-pushed the fix-combine-leaking-internals branch from 47a506d to 154b73d Compare October 22, 2022 10:47
@AlexandrHoroshih AlexandrHoroshih marked this pull request as ready for review October 22, 2022 10:47
@AlexandrHoroshih AlexandrHoroshih changed the title Protect against combine argument mutations Protect against combine argument breakage via Array.slice Oct 22, 2022
@AlexandrHoroshih AlexandrHoroshih changed the title Protect against combine argument breakage via Array.slice Protect against combine argument being broken via Array.slice Oct 22, 2022
zerobias
zerobias approved these changes Oct 22, 2022
View reviewed changes
Copy link
Member

@zerobias zerobias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks 👍

@zerobias zerobias merged commit 5a3b308 into master Oct 22, 2022
@zerobias zerobias deleted the fix-combine-leaking-internals branch October 22, 2022 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zerobias zerobias zerobias approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AlexandrHoroshih @zerobias