nodejs/node

Inconsistent behavior of path.basename(path, ext)

Open

#21,358 opened on Jun 15, 2018

View on GitHub
 (8 comments) (0 reactions) (0 assignees)JavaScript (35,535 forks)batch import
discusshelp wantedpath

Repository metrics

Stars
 (117,218 stars)
PR merge metrics
 (Avg merge 18d 17h) (219 merged PRs in 30d)

Description

I believe this was introduced somewhere in https://github.com/nodejs/node/pull/5123, which changed the behavior of the ext argument.

This is observed on all supported branches and was even recently backported to 4.x.

Documentation: https://nodejs.org/api/path.html#path_path_basename_path_ext

Observe the input and try to predict the output:

> ['a', 'a/', 'a//'].map(x => path.posix.basename(x))
[ 'a', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'b'))
[ 'a', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'a'))
[ '', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'a/'))
[ 'a', '', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'a//'))
[ 'a', 'a', '' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'aa'))
[ 'a', 'a/', 'a//' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'bb'))
[ 'a', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'aaa'))
[ 'a', 'a', 'a//' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'aaaa'))
[ 'a', 'a', 'a' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x))
[ 'dd', 'dd', 'dd', 'dd' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x, 'd'))
[ 'd', 'd', 'd', 'd' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x, 'dd'))
[ '', 'dd', 'dd', 'dd' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x, 'ddd'))
[ 'dd', 'dd', 'dd', 'dd/' ]

There are more, but all the inconsistencies with the previous behavior involve at least one of those:

  1. Either the path ends with /,
  2. Or ext includes /,
  3. Or ext equals to the actual resolved basename (i.e. path.endsWith('/' + ext)).

More specifically, the following check covers all the cases inconsistent behavior to my knowledge: path.endsWith('/') || ext.includes('/') || path.endsWith('/' + ext) (note that it also includes cases of consistent behavior).


Reminder: before #5123, this was how ext behave:

if (ext && f.substr(-1 * ext.length) === ext) {
    f = f.substr(0, f.length - ext.length);
}

I.e. it just sliced off the suffix (after doing everything else).

Contributor guide