Skip to content
Browse files

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.
  • Loading branch information...
1 parent 087d210 commit 3ecd574d7ee41e2ce903f62c3a1d6780d39a1bb9 @tshinnic committed Sep 8, 2011
Showing with 61 additions and 11 deletions.
  1. +6 −4 lib/fs.js
  2. +55 −7 test/simple/test-fs-watch-file.js
View
10 lib/fs.js
@@ -614,6 +614,9 @@ StatWatcher.prototype.stop = function() {
var statWatchers = {};
+function inStatWatchers(filename) {
+ return Object.prototype.hasOwnProperty.call(statWatchers, filename);
+}
fs.watchFile = function(filename) {
@@ -636,11 +639,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);
@@ -649,7 +651,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;
View
62 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,42 @@ 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);
+
+

0 comments on commit 3ecd574

Please sign in to comment.
Something went wrong with that request. Please try again.