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.
nodejs · Sep 12, 2011 · 7dc2c49 · 7dc2c49
1 parent e58c036
commit 7dc2c49
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 11 deletions.
diff --git 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
@@ -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);