2020-05-15 10:57:56

by Emil Velikov

[permalink] [raw]
Subject: get_maintainer.pl: unexpected behaviour for path/to//file

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 <[email protected]> (maintainer:DRM DRIVERS)
Daniel Vetter <[email protected]> (maintainer:DRM DRIVERS,commit_signer:3/42=7%)
Qiang Yu <[email protected]> (commit_signer:36/42=86%,authored:24/42=57%)
Vasily Khoruzhick <[email protected]> (commit_signer:26/42=62%)
Krzysztof Kozlowski <[email protected]> (commit_signer:5/42=12%,authored:5/42=12%)
Emil Velikov <[email protected]> (commit_signer:4/42=10%)
[email protected] (open list:DRM DRIVERS)
[email protected] (open list)

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

Qiang Yu <[email protected]> (maintainer:DRM DRIVERS FOR LIMA)
David Airlie <[email protected]> (maintainer:DRM DRIVERS)
Daniel Vetter <[email protected]> (maintainer:DRM DRIVERS)
[email protected] (open list:DRM DRIVERS FOR LIMA)
[email protected] (moderated list:DRM DRIVERS FOR LIMA)
[email protected] (open list)


2020-05-15 12:35:32

by Joe Perches

[permalink] [raw]
Subject: Re: get_maintainer.pl: unexpected behaviour for path/to//file

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

2020-05-15 17:24:51

by Joe Perches

[permalink] [raw]
Subject: Re: get_maintainer.pl: unexpected behaviour for path/to//file

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/@;


2020-05-15 19:12:27

by Joe Perches

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

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 <[email protected]> (maintainer:DRM DRIVERS)
Daniel Vetter <[email protected]> (maintainer:DRM DRIVERS,commit_signer:3/42=7%)
Qiang Yu <[email protected]> (commit_signer:36/42=86%,authored:24/42=57%)
Vasily Khoruzhick <[email protected]> (commit_signer:26/42=62%)
Krzysztof Kozlowski <[email protected]> (commit_signer:5/42=12%,authored:5/42=12%)
Emil Velikov <[email protected]> (commit_signer:4/42=10%)
[email protected] (open list:DRM DRIVERS)
[email protected] (open list)

# (without double slash)
$ ./scripts/get_maintainer.pl -f drivers/gpu/drm/lima
Qiang Yu <[email protected]> (maintainer:DRM DRIVERS FOR LIMA)
David Airlie <[email protected]> (maintainer:DRM DRIVERS)
Daniel Vetter <[email protected]> (maintainer:DRM DRIVERS)
[email protected] (open list:DRM DRIVERS FOR LIMA)
[email protected] (moderated list:DRM DRIVERS FOR LIMA)
[email protected] (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 <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
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/@;


2020-05-17 15:53:52

by Emil Velikov

[permalink] [raw]
Subject: Re: get_maintainer.pl: unexpected behaviour for path/to//file

On Fri, 15 May 2020 at 18:22, Joe Perches <[email protected]> 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