Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.watch - on change returns incorrect file name #19170

Closed
captainrdubb opened this issue Mar 6, 2018 · 6 comments
Closed

fs.watch - on change returns incorrect file name #19170

captainrdubb opened this issue Mar 6, 2018 · 6 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.

Comments

@captainrdubb
Copy link

captainrdubb commented Mar 6, 2018

In another project, I ran into the following scenario:

The function os.tmpDir returns a path with the 8.3 short name for the user name, example "C:\Users\<FIRST SIX CHARS OF NAME>~1\AppData\Local\Temp".

The Windows api returns the changed file name including the long version of the user name, example,
"C:\Users\<actual 18 char user name>\AppData\Local\Temp".

Pretend we are monitoring, "C:\Users\<8.3 file name>\AppData\Local\Temp\Configuration", expecting the file "config.json" to change. We would then expect the FSWatcher change event to return "config.json" as the name of the changed file. Instead, it returns something like "figuration\config.json", depending on the difference in length between the 8.3 file name and the actual file name.

It seems that node is saving an index so that it can quickly determine the part of the uri that is the file name, but the uri passed to fs.watch is a different length than the uri returned by the Windows api.

@seishun
Copy link
Contributor

seishun commented Mar 7, 2018

It's not clear from your description how one can reproduce this issue. Can you provide a test case?

@captainrdubb
Copy link
Author

captainrdubb commented Mar 7, 2018

@seishun

var os = require('os');
var fs = require('fs');
var path = require('path');
var assert = require('assert');

var expectedFilename = 'testFile.txt';

var tmpDir = os.tmpdir();
var testDir = path.join(tmpDir, 'testCase');
var testFile = path.join(testDir, expectedFilename);

if (!fs.existsSync(testDir)) {
    fs.mkdirSync(testDir);
}

fs.watch(testDir, function (eventType, filename) {
    var areEqual = filename === expectedFilename;

    //This assert will fail if testDir contains an 8.3 short filename https://en.wikipedia.org/wiki/8.3_filename
    assert.deepStrictEqual(filename, expectedFilename, `"${filename}\" ${areEqual ? '=' : '!'}== \"${expectedFilename}\"`);
});


if (!fs.exists(testFile)) {
    fs.writeFileSync(testFile);
} else {
    fs.appendFileSync(testFile, 'data');
}

@joyeecheung joyeecheung added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. labels Mar 7, 2018
@bzoz
Copy link
Contributor

bzoz commented Mar 8, 2018

Its a libuv issue. The wrong relative filename is created in fs-event.c.

@captainrdubb
Copy link
Author

@bzoz Maybe we need the long_dirw, instead of passing handle->dirw
image

@bzoz
Copy link
Contributor

bzoz commented Mar 8, 2018

I think we should get long directory name here: https://github.com/libuv/libuv/blob/v1.x/src/win/fs-event.c#L200.

bzoz pushed a commit to libuv/libuv that referenced this issue Mar 19, 2018
`uv_relative_path` assumes `dir` is a prefix of `filename`, which is not
the case when `handle->dirw` is a short path.

Refs: nodejs/node#19170
PR-URL: #1769
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@bzoz
Copy link
Contributor

bzoz commented Mar 19, 2018

@seishun fixed this in libuv

@cjihrig cjihrig closed this as completed in ae2b5bc Apr 5, 2018
targos pushed a commit that referenced this issue Apr 6, 2018
Notable changes:
- uv_fs_copyfile() adds support for copy-on-write behavior.
- uv_relative_path() now uses the long directory name
  for handle->dirw.
- File operations on files > 2 GB on 32-bit platforms are
  working again.
- uv_fs_fchmod() on Windows works on files with the
  Archive flag cleared.

Fixes: #19170
Fixes: #19455
Fixes: #12803
PR-URL: #19758
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Aug 7, 2018
Notable changes:
- uv_fs_copyfile() adds support for copy-on-write behavior.
- uv_relative_path() now uses the long directory name
  for handle->dirw.
- File operations on files > 2 GB on 32-bit platforms are
  working again.
- uv_fs_fchmod() on Windows works on files with the
  Archive flag cleared.

Fixes: #19170
Fixes: #19455
Fixes: #12803
PR-URL: #19758
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Nov 5, 2018
Notable changes:
- uv_fs_copyfile() adds support for copy-on-write behavior.
- uv_relative_path() now uses the long directory name
  for handle->dirw.
- File operations on files > 2 GB on 32-bit platforms are
  working again.
- uv_fs_fchmod() on Windows works on files with the
  Archive flag cleared.

Fixes: nodejs#19170
Fixes: nodejs#19455
Fixes: nodejs#12803
PR-URL: nodejs#19758
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 11, 2018
Notable changes:
- uv_fs_copyfile() adds support for copy-on-write behavior.
- uv_relative_path() now uses the long directory name
  for handle->dirw.
- File operations on files > 2 GB on 32-bit platforms are
  working again.
- uv_fs_fchmod() on Windows works on files with the
  Archive flag cleared.

Backport-PR-URL: #24103
Fixes: #19170
Fixes: #19455
Fixes: #12803
PR-URL: #19758
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants