From 10f4734645f29a14f3f16773dd6fb3d8fbfc9002 Mon Sep 17 00:00:00 2001 From: Nikolai Vavilov Date: Fri, 18 Nov 2016 20:27:17 +0100 Subject: [PATCH 1/3] 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). --- lib/fs.js | 82 +------------------ .../test-fs-read-buffer-tostring-fail.js | 64 --------------- test/parallel/test-fs-read-zero-length.js | 18 ---- test/parallel/test-fs-read.js | 22 ----- 4 files changed, 2 insertions(+), 184 deletions(-) delete mode 100644 test/parallel/test-fs-read-buffer-tostring-fail.js delete mode 100644 test/parallel/test-fs-read-zero-length.js delete mode 100644 test/parallel/test-fs-read.js diff --git a/lib/fs.js b/lib/fs.js index f9223f5ca8484f..6362fab54a1c98 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 cce33edf4e6947..00000000000000 --- 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-zero-length.js b/test/parallel/test-fs-read-zero-length.js deleted file mode 100644 index 9c4cde52362ccc..00000000000000 --- a/test/parallel/test-fs-read-zero-length.js +++ /dev/null @@ -1,18 +0,0 @@ -'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 = ''; - -fs.read(fd, 0, 0, 'utf-8', common.mustCall(function(err, str, bytesRead) { - assert.ok(!err); - assert.equal(str, expected); - assert.equal(bytesRead, 0); -})); - -const r = fs.readSync(fd, 0, 0, 'utf-8'); -assert.equal(r[0], expected); -assert.equal(r[1], 0); diff --git a/test/parallel/test-fs-read.js b/test/parallel/test-fs-read.js deleted file mode 100644 index 41957c228ea944..00000000000000 --- a/test/parallel/test-fs-read.js +++ /dev/null @@ -1,22 +0,0 @@ -'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'; - -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); - })); - -var r = fs.readSync(fd, expected.length, 0, 'utf-8'); -assert.strictEqual(r[0], expected); -assert.strictEqual(r[1], expected.length); From 61fcb21a248e363a45a0870b8a48c72355fdba95 Mon Sep 17 00:00:00 2001 From: Nikolai Vavilov Date: Fri, 18 Nov 2016 23:00:35 +0200 Subject: [PATCH 2/3] doc: remove redundant "buffer" from test names --- ...-fs-read-buffer-zero-length.js => test-fs-read-zero-length.js} | 0 test/parallel/{test-fs-read-buffer.js => test-fs-read.js} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/parallel/{test-fs-read-buffer-zero-length.js => test-fs-read-zero-length.js} (100%) rename test/parallel/{test-fs-read-buffer.js => test-fs-read.js} (100%) diff --git a/test/parallel/test-fs-read-buffer-zero-length.js b/test/parallel/test-fs-read-zero-length.js similarity index 100% rename from test/parallel/test-fs-read-buffer-zero-length.js rename to test/parallel/test-fs-read-zero-length.js diff --git a/test/parallel/test-fs-read-buffer.js b/test/parallel/test-fs-read.js similarity index 100% rename from test/parallel/test-fs-read-buffer.js rename to test/parallel/test-fs-read.js From 1f4bd5298d05ef143b8c9bdd03cd0e2a0ee38aa3 Mon Sep 17 00:00:00 2001 From: Nikolai Vavilov Date: Tue, 6 Dec 2016 13:54:22 +0200 Subject: [PATCH 3/3] add test --- test/parallel/test-fs-read-type.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 test/parallel/test-fs-read-type.js diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js new file mode 100644 index 00000000000000..2b600b048758de --- /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/);