nodejs/node

Inconsistent behavior of path.basename(path, ext)

Open

#21 358 ouverte le 15 juin 2018

Voir sur GitHub
 (8 commentaires) (0 réactions) (0 assignés)JavaScript (35 535 forks)batch import
discusshelp wantedpath

Métriques du dépôt

Stars
 (117 218 stars)
Métriques de merge PR
 (Merge moyen 18j 17h) (219 PRs mergées en 30 j)

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).

Guide contributeur