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

fs: make sure that file is truncated in writeFile #23181

Conversation

thefourtheye
Copy link
Contributor

@thefourtheye thefourtheye commented Sep 30, 2018

Simple Reproduction of the Bug

'use strict';

const assert = require('assert');
const fs = require('fs');

const fileName = 'test.txt';
fs.writeFileSync(fileName, '123');
fs.writeFileSync(fileName, '0');
const result1 = fs.readFileSync(fileName, 'utf8');
fs.unlinkSync(fileName);

const fd = fs.openSync(fileName, 'w');
fs.writeFileSync(fd, '123');
fs.writeFileSync(fd, '0');
fs.closeSync(fd);
const result2 = fs.readFileSync(fileName, 'utf8');
fs.unlinkSync(fileName);

assert.deepStrictEqual(result2, result1);
// + expected - actual
//
//- '023'
//+ '0'

Fixes: #22554


cc @vsemozhetbyt @nodejs/fs

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Simple Reproduction of the Bug

```js
'use strict';

const assert = require('assert');
const fs = require('fs');

const fileName = 'test.txt';
fs.writeFileSync(fileName, '123');
fs.writeFileSync(fileName, '0');
const result1 = fs.readFileSync(fileName, 'utf8');
fs.unlinkSync(fileName);

const fd = fs.openSync(fileName, 'w');
fs.writeFileSync(fd, '123');
fs.writeFileSync(fd, '0');
fs.closeSync(fd);
const result2 = fs.readFileSync(fileName, 'utf8');
fs.unlinkSync(fileName);

assert.deepStrictEqual(result2, result1);
// + expected - actual
//
//- '023'
//+ '0'

```

Fixes: nodejs#22554
@thefourtheye thefourtheye added fs Issues and PRs related to the fs subsystem / file system. blocked PRs that are blocked by other issues or PRs. labels Sep 30, 2018
@thefourtheye
Copy link
Contributor Author

This PR is blocked by #23136

@thefourtheye thefourtheye added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 30, 2018
@vsemozhetbyt vsemozhetbyt mentioned this pull request Oct 2, 2018
4 tasks
@richardlau richardlau added this to the 11.0.0 milestone Oct 2, 2018
@thefourtheye
Copy link
Contributor Author

Bump @nodejs/fs

@jasnell
Copy link
Member

jasnell commented Oct 17, 2018

This is highly unlikely to make the 11.0.0 cutoff since the blocking PR has not yet landed.

@jasnell jasnell removed this from the 11.0.0 milestone Oct 23, 2018
@thefourtheye
Copy link
Contributor Author

As per the decision in #23433, the behaviour of writeFile is changed to be consistent with readFile, by making both of them start from the current position they are used with file descriptors. So closing this.

@thefourtheye thefourtheye deleted the fix-for-22554-fs-writefile-not-truncating branch December 17, 2018 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. fs Issues and PRs related to the fs subsystem / file system. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fs: .writeFile(filehandle, ...) behavior differs from the documented one in all its variants
4 participants