From 3c2a9361ffd797acd6c9f5a09ae01648aa0e9792 Mon Sep 17 00:00:00 2001 From: Nikolai Vavilov Date: Tue, 10 Jan 2017 14:26:23 +0200 Subject: [PATCH] fs: remove fs.read's string interface It is a maintenance burden that was removed from the docs in 2010 (c93e0aaf062081db3ec40ac45b3e2c979d5759d6) and runtime-deprecated in v6.0 (1124de2d76ad7118267d91a08485aa928a5f0865). PR-URL: https://github.com/nodejs/node/pull/9683 Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel Reviewed-By: Fedor Indutny Reviewed-By: Benjamin Gruenbaum --- lib/fs.js | 82 +------------------ .../test-fs-read-buffer-tostring-fail.js | 64 --------------- .../test-fs-read-buffer-zero-length.js | 19 ----- test/parallel/test-fs-read-buffer.js | 35 -------- test/parallel/test-fs-read-type.js | 21 +++++ test/parallel/test-fs-read-zero-length.js | 15 ++-- test/parallel/test-fs-read.js | 39 ++++++--- 7 files changed, 57 insertions(+), 218 deletions(-) delete mode 100644 test/parallel/test-fs-read-buffer-tostring-fail.js delete mode 100644 test/parallel/test-fs-read-buffer-zero-length.js delete mode 100644 test/parallel/test-fs-read-buffer.js create mode 100644 test/parallel/test-fs-read-type.js diff --git a/lib/fs.js b/lib/fs.js index f9223f5ca8..6362fab54a 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -555,40 +555,7 @@ fs.openSync = function(path, flags, mode) { return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode); }; -var readWarned = false; fs.read = function(fd, buffer, offset, length, position, callback) { - if (!isUint8Array(buffer)) { - // legacy string interface (fd, length, position, encoding, callback) - if (!readWarned) { - readWarned = true; - process.emitWarning( - 'fs.read\'s legacy String interface is deprecated. Use the Buffer ' + - 'API as mentioned in the documentation instead.', - 'DeprecationWarning'); - } - - const cb = arguments[4]; - const encoding = arguments[3]; - - assertEncoding(encoding); - - position = arguments[2]; - length = arguments[1]; - buffer = Buffer.allocUnsafe(length); - offset = 0; - - callback = function(err, bytesRead) { - if (!cb) return; - if (err) return cb(err); - - if (bytesRead > 0) { - tryToStringWithEnd(buffer, encoding, bytesRead, cb); - } else { - (cb)(err, '', bytesRead); - } - }; - } - if (length === 0) { return process.nextTick(function() { callback && callback(null, 0, buffer); @@ -606,57 +573,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) { binding.read(fd, buffer, offset, length, position, req); }; -function tryToStringWithEnd(buf, encoding, end, callback) { - var e; - try { - buf = buf.toString(encoding, 0, end); - } catch (err) { - e = err; - } - callback(e, buf, end); -} - -var readSyncWarned = false; fs.readSync = function(fd, buffer, offset, length, position) { - var legacy = false; - var encoding; - - if (!isUint8Array(buffer)) { - // legacy string interface (fd, length, position, encoding, callback) - if (!readSyncWarned) { - readSyncWarned = true; - process.emitWarning( - 'fs.readSync\'s legacy String interface is deprecated. Use the ' + - 'Buffer API as mentioned in the documentation instead.', - 'DeprecationWarning'); - } - legacy = true; - encoding = arguments[3]; - - assertEncoding(encoding); - - position = arguments[2]; - length = arguments[1]; - buffer = Buffer.allocUnsafe(length); - - offset = 0; - } - if (length === 0) { - if (legacy) { - return ['', 0]; - } else { - return 0; - } - } - - var r = binding.read(fd, buffer, offset, length, position); - if (!legacy) { - return r; + return 0; } - var str = (r > 0) ? buffer.toString(encoding, 0, r) : ''; - return [str, r]; + return binding.read(fd, buffer, offset, length, position); }; // usage: diff --git a/test/parallel/test-fs-read-buffer-tostring-fail.js b/test/parallel/test-fs-read-buffer-tostring-fail.js deleted file mode 100644 index cce33edf4e..0000000000 --- a/test/parallel/test-fs-read-buffer-tostring-fail.js +++ /dev/null @@ -1,64 +0,0 @@ -'use strict'; - -const common = require('../common'); - -if (!common.enoughTestMem) { - const skipMessage = 'intensive toString tests due to memory confinements'; - common.skip(skipMessage); - return; -} - -const assert = require('assert'); -const fs = require('fs'); -const path = require('path'); -const Buffer = require('buffer').Buffer; -const kStringMaxLength = process.binding('buffer').kStringMaxLength; - -var fd; - -common.refreshTmpDir(); - -const file = path.join(common.tmpDir, 'toobig2.txt'); -const stream = fs.createWriteStream(file, { - flags: 'a' -}); - -const size = kStringMaxLength / 200; -const a = Buffer.alloc(size, 'a'); - -for (var i = 0; i < 201; i++) { - stream.write(a); -} - -stream.end(); -stream.on('finish', common.mustCall(function() { - fd = fs.openSync(file, 'r'); - fs.read(fd, kStringMaxLength + 1, 0, 'utf8', common.mustCall(function(err) { - assert.ok(err instanceof Error); - assert.strictEqual('"toString()" failed', err.message); - })); -})); - -function destroy() { - try { - // Make sure we close fd and unlink the file - fs.closeSync(fd); - fs.unlinkSync(file); - } catch (err) { - // it may not exist - } -} - -process.on('exit', destroy); - -process.on('SIGINT', function() { - destroy(); - process.exit(); -}); - -// To make sure we don't leave a very large file -// on test machines in the event this test fails. -process.on('uncaughtException', function(err) { - destroy(); - throw err; -}); diff --git a/test/parallel/test-fs-read-buffer-zero-length.js b/test/parallel/test-fs-read-buffer-zero-length.js deleted file mode 100644 index 35b1f7b566..0000000000 --- a/test/parallel/test-fs-read-buffer-zero-length.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const path = require('path'); -const Buffer = require('buffer').Buffer; -const fs = require('fs'); -const filepath = path.join(common.fixturesDir, 'x.txt'); -const fd = fs.openSync(filepath, 'r'); -const bufferAsync = Buffer.alloc(0); -const bufferSync = Buffer.alloc(0); - -fs.read(fd, bufferAsync, 0, 0, 0, common.mustCall(function(err, bytesRead) { - assert.equal(bytesRead, 0); - assert.deepStrictEqual(bufferAsync, Buffer.alloc(0)); -})); - -const r = fs.readSync(fd, bufferSync, 0, 0, 0); -assert.deepStrictEqual(bufferSync, Buffer.alloc(0)); -assert.equal(r, 0); diff --git a/test/parallel/test-fs-read-buffer.js b/test/parallel/test-fs-read-buffer.js deleted file mode 100644 index 733be5ba0d..0000000000 --- a/test/parallel/test-fs-read-buffer.js +++ /dev/null @@ -1,35 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const path = require('path'); -const Buffer = require('buffer').Buffer; -const fs = require('fs'); -const filepath = path.join(common.fixturesDir, 'x.txt'); -const fd = fs.openSync(filepath, 'r'); - -const expected = Buffer.from('xyz\n'); - -function test(bufferAsync, bufferSync, expected) { - fs.read(fd, - bufferAsync, - 0, - expected.length, - 0, - common.mustCall((err, bytesRead) => { - assert.ifError(err); - assert.strictEqual(bytesRead, expected.length); - assert.deepStrictEqual(bufferAsync, Buffer.from(expected)); - })); - - const r = fs.readSync(fd, bufferSync, 0, expected.length, 0); - assert.deepStrictEqual(bufferSync, Buffer.from(expected)); - assert.strictEqual(r, expected.length); -} - -test(Buffer.allocUnsafe(expected.length), - Buffer.allocUnsafe(expected.length), - expected); - -test(new Uint8Array(expected.length), - new Uint8Array(expected.length), - Uint8Array.from(expected)); diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js new file mode 100644 index 0000000000..2b600b0487 --- /dev/null +++ b/test/parallel/test-fs-read-type.js @@ -0,0 +1,21 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const filepath = path.join(common.fixturesDir, 'x.txt'); +const fd = fs.openSync(filepath, 'r'); +const expected = 'xyz\n'; + +// Error must be thrown with string +assert.throws(() => { + fs.read(fd, + expected.length, + 0, + 'utf-8', + () => {}); +}, /Second argument needs to be a buffer/); + +assert.throws(() => { + fs.readSync(fd, expected.length, 0, 'utf-8'); +}, /Second argument needs to be a buffer/); diff --git a/test/parallel/test-fs-read-zero-length.js b/test/parallel/test-fs-read-zero-length.js index 9c4cde5236..35b1f7b566 100644 --- a/test/parallel/test-fs-read-zero-length.js +++ b/test/parallel/test-fs-read-zero-length.js @@ -2,17 +2,18 @@ const common = require('../common'); const assert = require('assert'); const path = require('path'); +const Buffer = require('buffer').Buffer; const fs = require('fs'); const filepath = path.join(common.fixturesDir, 'x.txt'); const fd = fs.openSync(filepath, 'r'); -const expected = ''; +const bufferAsync = Buffer.alloc(0); +const bufferSync = Buffer.alloc(0); -fs.read(fd, 0, 0, 'utf-8', common.mustCall(function(err, str, bytesRead) { - assert.ok(!err); - assert.equal(str, expected); +fs.read(fd, bufferAsync, 0, 0, 0, common.mustCall(function(err, bytesRead) { assert.equal(bytesRead, 0); + assert.deepStrictEqual(bufferAsync, Buffer.alloc(0)); })); -const r = fs.readSync(fd, 0, 0, 'utf-8'); -assert.equal(r[0], expected); -assert.equal(r[1], 0); +const r = fs.readSync(fd, bufferSync, 0, 0, 0); +assert.deepStrictEqual(bufferSync, Buffer.alloc(0)); +assert.equal(r, 0); diff --git a/test/parallel/test-fs-read.js b/test/parallel/test-fs-read.js index 41957c228e..733be5ba0d 100644 --- a/test/parallel/test-fs-read.js +++ b/test/parallel/test-fs-read.js @@ -2,21 +2,34 @@ const common = require('../common'); const assert = require('assert'); const path = require('path'); +const Buffer = require('buffer').Buffer; const fs = require('fs'); const filepath = path.join(common.fixturesDir, 'x.txt'); const fd = fs.openSync(filepath, 'r'); -const expected = 'xyz\n'; -fs.read(fd, - expected.length, - 0, - 'utf-8', - common.mustCall((err, str, bytesRead) => { - assert.ifError(err); - assert.strictEqual(str, expected); - assert.strictEqual(bytesRead, expected.length); - })); +const expected = Buffer.from('xyz\n'); -var r = fs.readSync(fd, expected.length, 0, 'utf-8'); -assert.strictEqual(r[0], expected); -assert.strictEqual(r[1], expected.length); +function test(bufferAsync, bufferSync, expected) { + fs.read(fd, + bufferAsync, + 0, + expected.length, + 0, + common.mustCall((err, bytesRead) => { + assert.ifError(err); + assert.strictEqual(bytesRead, expected.length); + assert.deepStrictEqual(bufferAsync, Buffer.from(expected)); + })); + + const r = fs.readSync(fd, bufferSync, 0, expected.length, 0); + assert.deepStrictEqual(bufferSync, Buffer.from(expected)); + assert.strictEqual(r, expected.length); +} + +test(Buffer.allocUnsafe(expected.length), + Buffer.allocUnsafe(expected.length), + expected); + +test(new Uint8Array(expected.length), + new Uint8Array(expected.length), + Uint8Array.from(expected));