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

Fix for possible stack overflow issues due to Array.push in combination with spread operator #2057

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/tall-nails-retire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"effect": patch
"@effect/platform": patch
"@effect/schema": patch
---

Fix for possible stack overflow errors when using Array.push with spread operator arguments
4 changes: 4 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ module.exports = {
"object-shorthand": "error",
"prefer-destructuring": "off",
"sort-imports": "off",
"no-restricted-syntax": ["error", {
"selector": "CallExpression[callee.property.name='push'] > SpreadElement.arguments",
"message": "Do not use spread arguments in Array.push"
}],
"no-unused-vars": "off",
"prefer-rest-params": "off",
"prefer-spread": "off",
Expand Down
5 changes: 4 additions & 1 deletion packages/effect/src/ReadonlyArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1545,7 +1545,10 @@ export const flatMap: {
}
const out: Array<B> = []
for (let i = 0; i < self.length; i++) {
out.push(...f(self[i], i))
const inner = f(self[i], i)
for (let j = 0; j < inner.length; j++) {
out.push(inner[j])
}
}
return out
}
Expand Down
4 changes: 3 additions & 1 deletion packages/effect/src/internal/differ/readonlyArrayPatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ export const patch = Dual.dual<
break
}
case "Append": {
readonlyArray.push(...head.values)
for (const value of head.values) {
readonlyArray.push(value)
}
patches = tail
break
}
Expand Down
4 changes: 3 additions & 1 deletion packages/effect/src/internal/stm/tMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,9 @@ const toReadonlyArray = <K, V>(self: TMap.TMap<K, V>): STM.STM<never, never, Rea
let index = 0
while (index < capacity) {
const bucket = buckets.chunk[index]
builder.push(...tRef.unsafeGet(bucket, journal))
for (const entry of tRef.unsafeGet(bucket, journal)) {
builder.push(entry)
}
index = index + 1
}
return builder
Expand Down
16 changes: 12 additions & 4 deletions packages/effect/src/internal/stm/tPriorityQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ export const takeAll = <A>(self: TPriorityQueue.TPriorityQueue<A>): STM.STM<neve
tRef.modify(self.ref, (map) => {
const builder: Array<A> = []
for (const entry of map) {
builder.push(...entry[1])
for (const value of entry[1]) {
builder.push(value)
}
}
return [builder, SortedMap.empty(SortedMap.getOrder(map))]
})
Expand Down Expand Up @@ -228,7 +230,9 @@ export const takeUpTo = dual<
while ((next = iterator.next()) && !next.done && index < n) {
const [key, value] = next.value
const [left, right] = pipe(value, ReadonlyArray.splitAt(n - index))
builder.push(...left)
for (const value of left) {
builder.push(value)
}
if (right.length > 0) {
updated = SortedMap.set(updated, key, right as [A, ...Array<A>])
} else {
Expand All @@ -244,7 +248,9 @@ export const toChunk = <A>(self: TPriorityQueue.TPriorityQueue<A>): STM.STM<neve
tRef.modify(self.ref, (map) => {
const builder: Array<A> = []
for (const entry of map) {
builder.push(...entry[1])
for (const value of entry[1]) {
builder.push(value)
}
}
return [Chunk.unsafeFromArray(builder), map]
})
Expand All @@ -254,7 +260,9 @@ export const toArray = <A>(self: TPriorityQueue.TPriorityQueue<A>): STM.STM<neve
tRef.modify(self.ref, (map) => {
const builder: Array<A> = []
for (const entry of map) {
builder.push(...entry[1])
for (const value of entry[1]) {
builder.push(value)
}
}
return [builder, map]
})
4 changes: 3 additions & 1 deletion packages/platform/src/internal/workerRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ export const make = <I, R, E, O>(
return Effect.flatMap(
Effect.forEach(data, (data) => {
if (options?.transfers) {
transfers.push(...options.transfers(data))
for (const option of options.transfers(data)) {
transfers.push(option)
}
}
return Effect.orDie(options.encodeOutput!(req[2], data))
}),
Expand Down
4 changes: 3 additions & 1 deletion packages/schema/src/AST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,9 @@ export const getNumberIndexedAccess = (ast: AST): AST => {
out.push(undefinedKeyword)
}
if (Option.isSome(ast.rest)) {
out.push(...ast.rest.value)
for (const e of ast.rest.value) {
out.push(e)
}
}
return createUnion(out)
}
Expand Down
Loading