get_maintainer.pl: unexpected behaviour for path/to//file

get_maintainer.pl: unexpected behaviour for path/to//file

[Emil Velikov]

Hi Joe,

Recently I've noticed that get_maintainer behaves differently if there
is a double, sequential, forward slash in the path.

AFAICT there should be no distinction between the two. Or at least many
existing applications and scripts consider them one and the same.

I've tried fixing this, although my perl isn't quite up-to scratch.
Is this some weird bug or some intended feature?

Thanks
Emil

Example:

./scripts/get_maintainer -F drivers/gpu/drm//lima

David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS,commit_signer:3/42=7%)
Qiang Yu <yuq825@gmail.com> (commit_signer:36/42=86%,authored:24/42=57%)
Vasily Khoruzhick <anarsoul@gmail.com> (commit_signer:26/42=62%)
Krzysztof Kozlowski <krzk@kernel.org> (commit_signer:5/42=12%,authored:5/42=12%)
Emil Velikov <emil.velikov@collabora.com> (commit_signer:4/42=10%)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
linux-kernel@vger.kernel.org (open list)

./scripts/get_maintainer -F drivers/gpu/drm/lima

Qiang Yu <yuq825@gmail.com> (maintainer:DRM DRIVERS FOR LIMA)
David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS FOR LIMA)
lima@lists.freedesktop.org (moderated list:DRM DRIVERS FOR LIMA)
linux-kernel@vger.kernel.org (open list)

Re: get_maintainer.pl: unexpected behaviour for path/to//file

[Joe Perches]

On Fri, 2020-05-15 at 11:52 +0100, Emil Velikov wrote:
> Hi Joe,
> 
> Recently I've noticed that get_maintainer behaves differently if there
> is a double, sequential, forward slash in the path.
> 
> AFAICT there should be no distinction between the two. Or at least many
> existing applications and scripts consider them one and the same.
> 
> I've tried fixing this, although my perl isn't quite up-to scratch.
> Is this some weird bug or some intended feature?

Not really an intended feature.
The code counts slashes for directory depth.

I suppose it might be simpler to do this:
---
 scripts/get_maintainer.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 6d973f3685f9..eaaf9373dbcf 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -538,6 +538,7 @@ foreach my $file (@ARGV) {
 	} elsif (!(-f $file)) {
 	    die "$P: file '${file}' not found\n";
 	}
+	$file =~ s@//@/@g;	# compress file double slashes
     }
     if ($from_filename || ($file ne "&STDIN" && vcs_file_exists($file))) {
 	$file =~ s/^\Q${cur_path}\E//;	#strip any absolute path

Re: get_maintainer.pl: unexpected behaviour for path/to//file

[Joe Perches]

On Fri, 2020-05-15 at 05:31 -0700, Joe Perches wrote:
> On Fri, 2020-05-15 at 11:52 +0100, Emil Velikov wrote:
> > Hi Joe,
> > 
> > Recently I've noticed that get_maintainer behaves differently if there
> > is a double, sequential, forward slash in the path.
> > 
> > AFAICT there should be no distinction between the two. Or at least many
> > existing applications and scripts consider them one and the same.
> > 
> > I've tried fixing this, although my perl isn't quite up-to scratch.
> > Is this some weird bug or some intended feature?
> 
> Not really an intended feature.
> The code counts slashes for directory depth.
> 
> I suppose it might be simpler to do this:

Or perhaps a better alternative is:
---
 scripts/get_maintainer.pl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 6d973f3685f9..484d2fbf5921 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -19,6 +19,7 @@ my $V = '0.26';
 use Getopt::Long qw(:config no_auto_abbrev);
 use Cwd;
 use File::Find;
+use File::Spec::Functions;
 
 my $cur_path = fastgetcwd() . '/';
 my $lk_path = "./";
@@ -532,6 +533,7 @@ if (!@ARGV) {
 
 foreach my $file (@ARGV) {
     if ($file ne "&STDIN") {
+	$file = canonpath($file);
 	##if $file is a directory and it lacks a trailing slash, add one
 	if ((-d $file)) {
 	    $file =~ s@([^/])$@$1/@;

[PATCH] get_maintainer: Fix unexpected behavior for path/to//file (double slashes)

[Joe Perches]

get_maintainer behaves differently if there is a
double sequential forward slash in a filename because
the total number of slashes in a filename is used to
match MAINTAINERS file patterns.

For example:

# (with double slash)
$ ./scripts/get_maintainer.pl -f drivers/gpu/drm//lima
David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS,commit_signer:3/42=7%)
Qiang Yu <yuq825@gmail.com> (commit_signer:36/42=86%,authored:24/42=57%)
Vasily Khoruzhick <anarsoul@gmail.com> (commit_signer:26/42=62%)
Krzysztof Kozlowski <krzk@kernel.org> (commit_signer:5/42=12%,authored:5/42=12%)
Emil Velikov <emil.velikov@collabora.com> (commit_signer:4/42=10%)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
linux-kernel@vger.kernel.org (open list)

# (without double slash)
$ ./scripts/get_maintainer.pl -f drivers/gpu/drm/lima
Qiang Yu <yuq825@gmail.com> (maintainer:DRM DRIVERS FOR LIMA)
David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS FOR LIMA)
lima@lists.freedesktop.org (moderated list:DRM DRIVERS FOR LIMA)
linux-kernel@vger.kernel.org (open list)

So reduce consecutive double slashes to a single slash
by using File::Spec->canonpath().

from: https://perldoc.perl.org/File/Spec/Unix.html

canonpath()

No physical check on the filesystem, but a logical cleanup of a
path. On UNIX eliminates successive slashes and successive "/.".

Reported-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/get_maintainer.pl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 6d973f3685f9..484d2fbf5921 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -19,6 +19,7 @@ my $V = '0.26';
 use Getopt::Long qw(:config no_auto_abbrev);
 use Cwd;
 use File::Find;
+use File::Spec::Functions;
 
 my $cur_path = fastgetcwd() . '/';
 my $lk_path = "./";
@@ -532,6 +533,7 @@ if (!@ARGV) {
 
 foreach my $file (@ARGV) {
     if ($file ne "&STDIN") {
+	$file = canonpath($file);
 	##if $file is a directory and it lacks a trailing slash, add one
 	if ((-d $file)) {
 	    $file =~ s@([^/])$@$1/@;

Re: get_maintainer.pl: unexpected behaviour for path/to//file

[Emil Velikov]

On Fri, 15 May 2020 at 18:22, Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2020-05-15 at 05:31 -0700, Joe Perches wrote:
> > On Fri, 2020-05-15 at 11:52 +0100, Emil Velikov wrote:
> > > Hi Joe,
> > >
> > > Recently I've noticed that get_maintainer behaves differently if there
> > > is a double, sequential, forward slash in the path.
> > >
> > > AFAICT there should be no distinction between the two. Or at least many
> > > existing applications and scripts consider them one and the same.
> > >
> > > I've tried fixing this, although my perl isn't quite up-to scratch.
> > > Is this some weird bug or some intended feature?
> >
> > Not really an intended feature.
> > The code counts slashes for directory depth.
> >
> > I suppose it might be simpler to do this:
>
> Or perhaps a better alternative is:
> ---
>  scripts/get_maintainer.pl | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 6d973f3685f9..484d2fbf5921 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -19,6 +19,7 @@ my $V = '0.26';
>  use Getopt::Long qw(:config no_auto_abbrev);
>  use Cwd;
>  use File::Find;
> +use File::Spec::Functions;
>
>  my $cur_path = fastgetcwd() . '/';
>  my $lk_path = "./";
> @@ -532,6 +533,7 @@ if (!@ARGV) {
>
>  foreach my $file (@ARGV) {
>      if ($file ne "&STDIN") {
> +       $file = canonpath($file);

This seems like the better option since it also handles path traversal.
I would expect that people don't use it, yet who knows.

Thanks for the prompt fix.
-Emil