From 7dc2c492e9816631aae0eb72c218e0389c5d6e5f Mon Sep 17 00:00:00 2001 From: Thomas Shinnick Date: Thu, 8 Sep 2011 23:16:10 -0500 Subject: [PATCH] fs: unguarded fs.watchFile cache statWatchers checking fixed Use hasOwnProperty to check filepath cache; previous code could fail if a filepath duplicated a chained property name. Fixes #1637. --- lib/fs.js | 10 +++--- test/simple/test-fs-watch-file.js | 60 +++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 5a7f2551ac..3ea945d861 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -617,6 +617,9 @@ StatWatcher.prototype.stop = function() { var statWatchers = {}; +function inStatWatchers(filename) { + return Object.prototype.hasOwnProperty.call(statWatchers, filename); +} fs.watchFile = function(filename) { @@ -639,11 +642,10 @@ fs.watchFile = function(filename) { if (options.persistent === undefined) options.persistent = true; if (options.interval === undefined) options.interval = 0; - if (statWatchers[filename]) { + if (inStatWatchers(filename)) { stat = statWatchers[filename]; } else { - statWatchers[filename] = new StatWatcher(); - stat = statWatchers[filename]; + stat = statWatchers[filename] = new StatWatcher(); stat.start(filename, options.persistent, options.interval); } stat.addListener('change', listener); @@ -652,7 +654,7 @@ fs.watchFile = function(filename) { fs.unwatchFile = function(filename) { var stat; - if (statWatchers[filename]) { + if (inStatWatchers(filename)) { stat = statWatchers[filename]; stat.stop(); statWatchers[filename] = undefined; diff --git a/test/simple/test-fs-watch-file.js b/test/simple/test-fs-watch-file.js index ceee976914..af91aeb167 100644 --- a/test/simple/test-fs-watch-file.js +++ b/test/simple/test-fs-watch-file.js @@ -27,13 +27,33 @@ var assert = require('assert'); var path = require('path'); var fs = require('fs'); +var watchSeenOne = 0; +var watchSeenTwo = 0; -var filename = path.join(common.fixturesDir, 'watch.txt'); -fs.writeFileSync(filename, "hello"); +var startDir = process.cwd(); +var testDir = common.fixturesDir; + +var filenameOne = 'watch.txt'; +var filepathOne = path.join(testDir, filenameOne); + +var filenameTwo = 'hasOwnProperty'; +var filepathTwo = filenameTwo; +var filepathTwoAbs = path.join(testDir, filenameTwo); + + +process.addListener('exit', function() { + fs.unlinkSync(filepathOne); + fs.unlinkSync(filepathTwoAbs); + assert.equal(1, watchSeenOne); + assert.equal(1, watchSeenTwo); +}); + + +fs.writeFileSync(filepathOne, "hello"); assert.throws( function() { - fs.watchFile(filename); + fs.watchFile(filepathOne); }, function(e) { return e.message === 'watchFile requires a listener function'; @@ -42,14 +62,40 @@ assert.throws( assert.doesNotThrow( function() { - fs.watchFile(filename, function(curr, prev) { - fs.unwatchFile(filename); - fs.unlinkSync(filename); + fs.watchFile(filepathOne, function(curr, prev) { + fs.unwatchFile(filepathOne); + ++watchSeenOne; }); } ); setTimeout(function() { - fs.writeFileSync(filename, "world"); + fs.writeFileSync(filepathOne, "world"); }, 1000); + +process.chdir(testDir); + +fs.writeFileSync(filepathTwoAbs, "howdy"); + +assert.throws( + function() { + fs.watchFile(filepathTwo); + }, + function(e) { + return e.message === 'watchFile requires a listener function'; + } +); + +assert.doesNotThrow( + function() { + fs.watchFile(filepathTwo, function(curr, prev) { + fs.unwatchFile(filepathTwo); + ++watchSeenTwo; + }); + } +); + +setTimeout(function() { + fs.writeFileSync(filepathTwoAbs, "pardner"); +}, 1000);