-
Notifications
You must be signed in to change notification settings - Fork 5.6k
fix(ext/fs): truncate files when a ReadableStream is passed to writeFile #23330
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
Conversation
This will need some discussion from the core team. @char could you add a unit test for this particular case (or update existing one)? |
I've added a test which ensures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enablement condition is wrong. There should also be a test that appends correctly when using the append
option (this is currently not working in this PR).
Mirrors op_fs_write_file_async behavior
}); | ||
await Deno.writeFile(filename, stream); | ||
assertEquals(Deno.readFileSync(filename), new Uint8Array([1, 2])); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}, | |
// append false should also overwrite | |
await Deno.writeFile(filename, new Response(new Uint8Array([1, 2, 3])).body!, {append: false}); | |
assertEquals(Deno.readFileSync(filename), new Uint8Array([1, 2, 3])); | |
// append set should append | |
await Deno.writeFile(filename, new Response(new Uint8Array([4, 5])).body!, {append: true}); | |
assertEquals(Deno.readFileSync(filename), new Uint8Array([1, 2, 3, 4, 5])); | |
}, |
controller.enqueue(new Uint8Array([2])); | ||
controller.close(); | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}); | |
}); | |
// append not set should overwrite |
@lucacasonato are you fine with these changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Closes #19697. This fixes a bug where the writeFile API can create partially-overwritten files which may lead to invalid / corrupt files or data leakage. It also aligns the behavior of writing a ReadableStream and writing a Uint8Array to the disk.