require('path').relative(to, from) fails on windows azure #4073

Closed
lancejpollard opened this Issue Oct 2, 2012 · 8 comments

Projects

None yet

5 participants

@lancejpollard

Note, I am using, and Windows Azure is using, Node 0.8.2, but I also tested this by copy/pasting the path.js file from Node master on GitHub and it still has the same issue.

The issue is this:

var basePath = "\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales"
var nestedPath = "\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales\\en.coffee"
require('path').relative(basePath, nestedPath) //=> "shared"
// it should be "en.coffee"

The issue is with the first characters in the path....

Here is the copied 0.8.2 implementation:

exports.relative = function (from, to) {
  from = exports.resolve(from).substr(1);
  to = exports.resolve(to).substr(1);

  function trim(arr) {
    var start = 0;
    for (; start < arr.length; start++) {
      if (arr[start] !== '') break;
    }

    var end = arr.length - 1;
    for (; end >= 0; end--) {
      if (arr[end] !== '') break;
    }

    if (start > end) return [];
    return arr.slice(start, end - start + 1);
  }

  var fromParts = trim(from.split('/'));
  var toParts = trim(to.split('/'));

  var length = Math.min(fromParts.length, toParts.length);
  var samePartsLength = length;
  for (var i = 0; i < length; i++) {
    if (fromParts[i] !== toParts[i]) {
      samePartsLength = i;
      break;
    }
  }

  var outputParts = [];
  for (var i = samePartsLength; i < fromParts.length; i++) {
    outputParts.push('..');
  }

  outputParts = outputParts.concat(toParts.slice(samePartsLength));

  return outputParts.join('/');
}

The lines exports.resolve(from).substr(1) and such need to account for C:\\ and \\ on windows (which is 4, and 2). Also, they are splitting by / which fails on windows. So it should be this:

exports.relative = function (from, to) {
  // this should be even more robust
  var startLength = require('os').platform() == 'win32' ? 2 : 1;
  from = exports.resolve(from).substr(startLength);
  to = exports.resolve(to).substr(startLength);

  function trim(arr) {
    var start = 0;
    for (; start < arr.length; start++) {
      if (arr[start] !== '') break;
    }

    var end = arr.length - 1;
    for (; end >= 0; end--) {
      if (arr[end] !== '') break;
    }

    if (start > end) return [];
    return arr.slice(start, end - start + 1);
  }

  var fromParts = trim(from.split(exports.sep));
  var toParts = trim(to.split(exports.sep));

  var length = Math.min(fromParts.length, toParts.length);
  var samePartsLength = length;
  for (var i = 0; i < length; i++) {
    if (fromParts[i] !== toParts[i]) {
      samePartsLength = i;
      break;
    }
  }

  var outputParts = [];
  for (var i = samePartsLength; i < fromParts.length; i++) {
    outputParts.push('..');
  }

  outputParts = outputParts.concat(toParts.slice(samePartsLength));

  return outputParts.join(exports.sep);
}

The latest node on github also fails. Even though the code is much different on master, it still produces "shared" when it should produce "en.coffee" in the example at the beginning.

Any ideas?

Using the latest path.js code, some other potentially useful info:

> var r = splitDeviceRe.exec(nestedPath)
[ '\\\\10.207.104.177\\volume-22-default\\<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/en.coffee',
  '\\\\10.207.104.177\\volume-22-default',
  '\\',
  '<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/en.coffee',
  index: 0,
  input: '\\\\10.207.104.177\\volume-22-default\\<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/en.coffee' ]

so:

device == "\\\\10.207.104.177\\volume-22-default\\"
tail == "<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/en.coffee"

and

> splitTailRe.exec(r[3])
[ '<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/en.coffee',
  '<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/',
  'en.coffee',
  '.coffee',
  index: 0,
  input: '<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/en.coffee' ]

so:

dir == '<some-hash>\\<another-hash>\\site\\wwwroot\\app\\config\\shared\\locales/'
basename == 'en.coffee'
ext == '.coffee'

@viatropos Your note about backslashes are correct. Not sure about < and > in example. Code should be:

var basePath = '\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales';
var nestedPath = '\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales\\en.coffee';
require('path').relative(basePath, nestedPath); // 'en.coffee'

Unfortunately I'm linux user and after replacing backslashes to slashes works for me

Changing the trim function nested in the windows portion of path.relative in the latest code seems to work:

    function trim(arr) {
      var start = 0;
      for (; start < arr.length; start++) {
        if (arr[start] !== '') break;
      }

      var end = arr.length - 1;
      for (; end >= 0; end--) {
        if (arr[end] !== '') break;
      }

      if (start > end) return [];
      return arr.slice(start, end + 1);
    }

https://github.com/joyent/node/blob/41e53e557992a7d552a8e23de035f9463da25c99/lib/path.js#L221

@langpavel I updated the example, but those <some-hash> snippets were just me removing stuff that looked like 41e53e557992a7d552a8e23de035f9463da25c99 (since I'm not sure if it's a secret key or something on windows azure).

It seems to be resolved by changing the last line in that trim function above from:

return arr.slice(start, end - start + 1);

to

return arr.slice(start, end + 1);

because in the first case, given these paths:

var basePath = "\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales"
var nestedPath = "\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales\\en.coffee"

splitting them produces

> nestedPath.split('\\')
[ '',
  '',
  '10.207.104.177',
  'volume-22-default',
  'some-hash',
  'another-hash',
  'site',
  'wwwroot',
  'app',
  'config',
  'shared',
  'locales',
  'en.coffee' ]
> nestedPath.split('\\').length
13

So the original trim code will do

nestedPath.split('\\').slice(2, 13 - 2 + 1)
nestedPath.split('\\').slice(2, 12)

which gives slices off the last en.coffee for no reason.

The modified trim fixes that.

Any thoughts?

Test code to mess around with it:

process.platform = 'win32';
var basePath = "\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales"
var nestedPath = "\\\\10.207.104.177\\volume-22-default\\some-hash\\another-hash\\site\\wwwroot\\app\\config\\shared\\locales\\en.coffee"
var path = require('./tmp/path');
path.relative(basePath, nestedPath);
Member

It would be nice to add some path.relative tests for UNC paths first. They should go here: https://github.com/joyent/node/blob/109f8e2773bbdce8faeb45e616cd194129bab3a6/test/simple/test-path.js#L263

Owner
jasnell commented May 18, 2015

@orangemocha ... thoughts on this one?

@orangemocha orangemocha self-assigned this May 20, 2015
@orangemocha orangemocha added the windows label May 20, 2015
Member

This was fixed in 37c2a52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment