{"thread":{"id":"5695","subject":"[PATCH 0/5] gitweb: A few code cleanup patches","startedAt":"2006-09-25T23:53:08Z","lastAt":"2006-09-26T07:55:02Z","messageCount":10,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"27641","messageId":"200609260153.08503.jnareb@gmail.com","threadId":"5695","inReplyTo":null,"subject":"[PATCH 0/5] gitweb: A few code cleanup patches","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T23:53:08Z","receivedAt":"2006-09-25T23:53:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This series of patches, based on 8391548e5e78677eb81f97334d998418802ea194\nadds some code cleanups (including validating input and escaping output).\nMost deals with changes introduced in\n [PATCH] gitweb: Consolidate escaping/validation of query string\n commit a2f3db2f5de2a3667b0e038aa65e3e097e642e7d (by Petr Baudis)\n Message-ID: <20060923221841.18063.56589.stgit@rover>\n\nShortlog:\n [PATCH 1/5] gitweb: Strip trailing slashes from $path in git_get_hash_by_path\n [PATCH 2/5] gitweb: Use \"return\" instead of \"return undef\" for some subs\n [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname\n [PATCH 4/5] gitweb: Add git_url subroutine, and use it to quote full URLs\n [PATCH 5/5] gitweb: Quote filename in HTTP Content-Disposition: header\n\nDiffstat:\n gitweb/gitweb.perl | 100 +++++++++++++++++++++++++++++++++++++---------------\n 1 files changed, 72 insertions(+), 28 deletions(-)\n\n-- \nJakub Narebski\nShadeHawk on #git\nWarsaw, Poland\n"},{"id":"27642","messageId":"200609260154.24488.jnareb@gmail.com","threadId":"5695","inReplyTo":"200609260153.08503.jnareb@gmail.com","subject":"[PATCH 1/5] gitweb: Strip trailing slashes from $path in git_get_hash_by_path","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T23:54:24Z","receivedAt":"2006-09-25T23:54:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"It also removes unused local variable $tree \n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 66be619..2ad7eed 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -710,7 +710,7 @@ sub git_get_hash_by_path {\n \tmy $path = shift || return undef;\n \tmy $type = shift;\n \n-\tmy $tree = $base;\n+\t$path =~ s,/+$,,;\n \n \topen my $fd, \"-|\", git_cmd(), \"ls-tree\", $base, \"--\", $path\n \t\tor die_error(undef, \"Open git-ls-tree failed\");\n-- \n1.4.2.1\n"},{"id":"27645","messageId":"200609260156.17939.jnareb@gmail.com","threadId":"5695","inReplyTo":"200609260153.08503.jnareb@gmail.com","subject":"[PATCH 2/5] gitweb: Use \"return\" instead of \"return undef\" for some subs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T23:56:17Z","receivedAt":"2006-09-25T23:56:17Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use \"return\" instead of \"return undef\" when subroutine can return, or\nalways return, non-scalar (list) value.\n\nOther places are left as is.\n\nSigned-off-by: Jakub Narebski \n---\nThis is inspired by \"gitweb: Fix @git_base_url_list usage\" patch\n(74d6166751ddcf08029ffc90a14158a86f80cd40) by Petr Baudis.\n\n gitweb/gitweb.perl | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 2ad7eed..6458d7b 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -106,7 +106,7 @@ our %feature = (\n \n sub gitweb_check_feature {\n \tmy ($name) = @_;\n-\treturn undef unless exists $feature{$name};\n+\treturn unless exists $feature{$name};\n \tmy ($sub, $override, @defaults) = (\n \t\t$feature{$name}{'sub'},\n \t\t$feature{$name}{'override'},\n@@ -781,7 +781,7 @@ sub git_get_projects_list {\n \t\t# 'git%2Fgit.git Linus+Torvalds'\n \t\t# 'libs%2Fklibc%2Fklibc.git H.+Peter+Anvin'\n \t\t# 'linux%2Fhotplug%2Fudev.git Greg+Kroah-Hartman'\n-\t\topen my ($fd), $projects_list or return undef;\n+\t\topen my ($fd), $projects_list or return;\n \t\twhile (my $line = <$fd>) {\n \t\t\tchomp $line;\n \t\t\tmy ($path, $owner) = split ' ', $line;\n-- \n1.4.2.1\n"},{"id":"27646","messageId":"200609260157.03035.jnareb@gmail.com","threadId":"5695","inReplyTo":"200609260153.08503.jnareb@gmail.com","subject":"[PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T23:57:02Z","receivedAt":"2006-09-25T23:57:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Split validate_input subroutine into validate_pathname which is used\nfor $project, $file_name and $file_parent parameters, and\nvalidate_refname which is used for $hash, $hash_base, $hash_parent and\n$hash_parent_base parameters. Reintroduce validation of $file_name\nand $file_parent parameters, removed in a2f3db2f5de2a3667b0e038aa65e3\n\nvalidate_pathname in addition to what validate_input did checks also\nfor doubled slashes and NUL character. It does not check if input is\ntextual hash, and does not check if all characters are from the\nfollowing set: [a-zA-Z0-9_\\x80-\\xff\\ \\t\\.\\/\\-\\+\\#\\~\\%].\n\nvalidate_refname first check if the input is textual hash, then checks\nif it is valid pathname, then checks for invalid characters (according\nto git-check-ref-format manpage). It does not check if all charactes\nare from the [a-zA-Z0-9_\\x80-\\xff\\ \\t\\.\\/\\-\\+\\#\\~\\%] set.\n\nWe do not reintroduce validating pathnames we got from git.\n\nSigned-off-by: Jakub Narebski \n---\nAt last.\n\n gitweb/gitweb.perl | 66 +++++++++++++++++++++++++++++++++++++++-------------\n 1 files changed, 50 insertions(+), 16 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 6458d7b..7fce9a6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -200,9 +200,10 @@ if (defined $action) {\n \t}\n }\n \n+# parameters which are pathnames\n our $project = $cgi->param('p');\n if (defined $project) {\n-\tif (!validate_input($project) ||\n+\tif (!validate_pathname($project) ||\n \t !(-d \"$projectroot/$project\") ||\n \t !(-e \"$projectroot/$project/HEAD\") ||\n \t ($export_ok && !(-e \"$projectroot/$project/$export_ok\")) ||\n@@ -212,38 +213,50 @@ if (defined $project) {\n \t}\n }\n \n-# We have to handle those containing any characters:\n our $file_name = $cgi->param('f');\n+if (defined $file_name) {\n+\tif (!validate_pathname($file_name)) {\n+\t\tdie_error(undef, \"Invalid file parameter\");\n+\t}\n+}\n+\n our $file_parent = $cgi->param('fp');\n+if (defined $file_parent) {\n+\tif (!validate_pathname($file_parent)) {\n+\t\tdie_error(undef, \"Invalid file parent parameter\");\n+\t}\n+}\n \n+# parameters which are refnames\n our $hash = $cgi->param('h');\n if (defined $hash) {\n-\tif (!validate_input($hash)) {\n+\tif (!validate_refname($hash)) {\n \t\tdie_error(undef, \"Invalid hash parameter\");\n \t}\n }\n \n our $hash_parent = $cgi->param('hp');\n if (defined $hash_parent) {\n-\tif (!validate_input($hash_parent)) {\n+\tif (!validate_refname($hash_parent)) {\n \t\tdie_error(undef, \"Invalid hash parent parameter\");\n \t}\n }\n \n our $hash_base = $cgi->param('hb');\n if (defined $hash_base) {\n-\tif (!validate_input($hash_base)) {\n+\tif (!validate_refname($hash_base)) {\n \t\tdie_error(undef, \"Invalid hash base parameter\");\n \t}\n }\n \n our $hash_parent_base = $cgi->param('hpb');\n if (defined $hash_parent_base) {\n-\tif (!validate_input($hash_parent_base)) {\n+\tif (!validate_refname($hash_parent_base)) {\n \t\tdie_error(undef, \"Invalid hash parent base parameter\");\n \t}\n }\n \n+# other parameters\n our $page = $cgi->param('pg');\n if (defined $page) {\n \tif ($page =~ m/[^0-9]/) {\n@@ -273,7 +286,7 @@ sub evaluate_path_info {\n \t\t$project =~ s,/*[^/]*$,,;\n \t}\n \t# validate project\n-\t$project = validate_input($project);\n+\t$project = validate_pathname($project);\n \tif (!$project ||\n \t ($export_ok && !-e \"$projectroot/$project/$export_ok\") ||\n \t ($strict_export && !project_in_list($project))) {\n@@ -294,12 +307,12 @@ sub evaluate_path_info {\n \t\t} else {\n \t\t\t$action ||= \"blob_plain\";\n \t\t}\n-\t\t$hash_base ||= validate_input($refname);\n-\t\t$file_name ||= $pathname;\n+\t\t$hash_base ||= validate_refname($refname);\n+\t\t$file_name ||= validate_pathname($pathname);\n \t} elsif (defined $refname) {\n \t\t# we got \"project.git/branch\"\n \t\t$action ||= \"shortlog\";\n-\t\t$hash ||= validate_input($refname);\n+\t\t$hash ||= validate_refname($refname);\n \t}\n }\n evaluate_path_info();\n@@ -387,16 +400,37 @@ sub href(%) {\n ## ======================================================================\n ## validation, quoting/unquoting and escaping\n \n-sub validate_input {\n-\tmy $input = shift;\n+sub validate_pathname {\n+\tmy $input = shift || return undef;\n \n-\tif ($input =~ m/^[0-9a-fA-F]{40}$/) {\n-\t\treturn $input;\n+\t# no '.' or '..' as elements of path, i.e. no '.' nor '..'\n+\t# at the beginning, at the end, and between slashes.\n+\tif ($input =~ m!(^|/)(|\\.|\\.\\.)(/|$)!) {\n+\t\treturn undef;\n \t}\n-\tif ($input =~ m/(^|\\/)(|\\.|\\.\\.)($|\\/)/) {\n+\t# no doubled slashes\n+\tif ($input =~ m!//!) {\n \t\treturn undef;\n \t}\n-\tif ($input =~ m/[^a-zA-Z0-9_\\x80-\\xff\\ \\t\\.\\/\\-\\+\\#\\~\\%]/) {\n+\t# no null characters\n+\tif ($input =~ m!\\0!) {\n+\t\treturn undef;\n+\t}\n+\treturn $input;\n+}\n+\n+sub validate_refname {\n+\tmy $input = shift || return undef;\n+\n+\t# textual hashes are O.K.\n+\tif ($input =~ m/^[0-9a-fA-F]{40}$/) {\n+\t\treturn $input;\n+\t}\n+\t# it must be correct pathname\n+\t$input = validate_pathname($input)\n+\t\tor return undef;\n+\t# restrictions on ref name according to git-check-ref-format\n+\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n \t\treturn undef;\n \t}\n \treturn $input;\n-- \n1.4.2.1\n"},{"id":"27644","messageId":"200609260158.42247.jnareb@gmail.com","threadId":"5695","inReplyTo":"200609260153.08503.jnareb@gmail.com","subject":"[PATCH 4/5] gitweb: Add git_url subroutine, and use it to quote full URLs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T23:58:41Z","receivedAt":"2006-09-25T23:58:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add git_url subroutine, which does what git_param did before commit\na2f3db2f5de2a3667b0e038aa65e3e097e642e7d, and is used to quote full\nURLs, currently only $home_link.\n\nSigned-off-by: Jakub Narebski \n---\nIt will also be used in patch inspired by\n \"[RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage\"\n Message-ID: <7virjes7dq.fsf@assigned-by-dhcp.cox.net>\n http://permalink.gmane.org/gmane.comp.version-control.git/27619\n\n gitweb/gitweb.perl | 11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7fce9a6..b51e061 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -446,6 +446,15 @@ sub esc_param {\n \treturn $str;\n }\n \n+# quote unsafe chars in whole URL, so some charactrs cannot be quoted\n+sub esc_url {\n+\tmy $str = shift;\n+\t$str =~ s/([^A-Za-z0-9\\-_.~();\\/;?:@&=])/sprintf(\"%%%02X\", ord($1))/eg;\n+\t$str =~ s/\\+/%2B/g;\n+\t$str =~ s/ /\\+/g;\n+\treturn $str;\n+}\n+\n # replace invalid utf8 character with SUBSTITUTION sequence\n sub esc_html {\n \tmy $str = shift;\n@@ -1362,7 +1371,7 @@ EOF\n \t \"\" .\n \t \"\\\"git\\\"\" .\n \t \"\\n\";\n-\tprint $cgi->a({-href => esc_param($home_link)}, $home_link_str) . \" / \";\n+\tprint $cgi->a({-href => esc_url($home_link)}, $home_link_str) . \" / \";\n \tif (defined $project) {\n \t\tprint $cgi->a({-href => href(action=>\"summary\")}, esc_html($project));\n \t\tif (defined $action) {\n-- \n1.4.2.1\n"},{"id":"27643","messageId":"200609260159.43684.jnareb@gmail.com","threadId":"5695","inReplyTo":"200609260153.08503.jnareb@gmail.com","subject":"[PATCH 5/5] gitweb: Quote filename in HTTP Content-Disposition: header","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T23:59:43Z","receivedAt":"2006-09-25T23:59:43Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Finish work started by a2f3db2f5de2a3667b0e038aa65e3e097e642e7d commit\n(although not documented in commit message) of quoting using quotemeta\nthe filename in HTTP -content_disposition header. Uniquify output.\n\nJust in case filename contains end of line character.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 17 +++++++++--------\n 1 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b51e061..4dd7a5d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2326,7 +2326,7 @@ sub git_project_index {\n \tprint $cgi->header(\n \t\t-type => 'text/plain',\n \t\t-charset => 'utf-8',\n-\t\t-content_disposition => qq(inline; filename=\"index.aux\"));\n+\t\t-content_disposition => 'inline; filename=\"index.aux\"');\n \n \tforeach my $pr (@projects) {\n \t\tif (!exists $pr->{'owner'}) {\n@@ -2672,7 +2672,7 @@ sub git_blob_plain {\n \tprint $cgi->header(\n \t\t-type => \"$type\",\n \t\t-expires=>$expires,\n-\t\t-content_disposition => \"inline; filename=\\\"$save_as\\\"\");\n+\t\t-content_disposition => 'inline; filename=\"' . quotemeta($save_as) . '\"');\n \tundef $/;\n \tbinmode STDOUT, ':raw';\n \tprint <$fd>;\n@@ -2846,10 +2846,11 @@ sub git_snapshot {\n \n \tmy $filename = basename($project) . \"-$hash.tar.$suffix\";\n \n-\tprint $cgi->header(-type => 'application/x-tar',\n-\t -content_encoding => $ctype,\n-\t -content_disposition => \"inline; filename=\\\"$filename\\\"\",\n-\t -status => '200 OK');\n+\tprint $cgi->header(\n+\t\t-type => 'application/x-tar',\n+\t\t-content_encoding => $ctype,\n+\t\t-content_disposition => 'inline; filename=\"' . quotemeta($filename) . '\"',\n+\t\t-status => '200 OK');\n \n \tmy $git_command = git_cmd_str();\n \topen my $fd, \"-|\", \"$git_command tar-tree $hash \\'$project\\' | $command\" or\n@@ -3159,7 +3160,7 @@ sub git_blobdiff {\n \t\t\t-type => 'text/plain',\n \t\t\t-charset => 'utf-8',\n \t\t\t-expires => $expires,\n-\t\t\t-content_disposition => qq(inline; filename=\") . quotemeta($file_name) . qq(.patch\"));\n+\t\t\t-content_disposition => 'inline; filename=\"' . quotemeta($file_name) . '.patch\"');\n \n \t\tprint \"X-Git-Url: \" . $cgi->self_url() . \"\\n\\n\";\n \n@@ -3262,7 +3263,7 @@ sub git_commitdiff {\n \t\t\t-type => 'text/plain',\n \t\t\t-charset => 'utf-8',\n \t\t\t-expires => $expires,\n-\t\t\t-content_disposition => qq(inline; filename=\"$filename\"));\n+\t\t\t-content_disposition => 'inline; filename=\"' . quotemeta($filename) . '\"');\n \t\tmy %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});\n \t\tprint < writes:\n\n> Finish work started by a2f3db2f5de2a3667b0e038aa65e3e097e642e7d commit\n> (although not documented in commit message) of quoting using quotemeta\n> the filename in HTTP -content_disposition header. Uniquify output.\n>\n> Just in case filename contains end of line character.\n\nWhat do you mean by \"uniquify output\"?\n"},{"id":"27650","messageId":"7v1wpzw9l9.fsf@assigned-by-dhcp.cox.net","threadId":"5695","inReplyTo":"200609260157.03035.jnareb@gmail.com","subject":"Re: [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-26T04:11:30Z","receivedAt":"2006-09-26T04:11:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> @@ -387,16 +400,37 @@ sub href(%) {\n> ## ======================================================================\n> ## validation, quoting/unquoting and escaping\n> \n> -sub validate_input {\n> -\tmy $input = shift;\n> +sub validate_pathname {\n> +\tmy $input = shift || return undef;\n> \n> -\tif ($input =~ m/^[0-9a-fA-F]{40}$/) {\n> -\t\treturn $input;\n> +\t# no '.' or '..' as elements of path, i.e. no '.' nor '..'\n> +\t# at the beginning, at the end, and between slashes.\n> +\tif ($input =~ m!(^|/)(|\\.|\\.\\.)(/|$)!) {\n> +\t\treturn undef;\n> \t}\n> -\tif ($input =~ m/(^|\\/)(|\\.|\\.\\.)($|\\/)/) {\n> +\t# no doubled slashes\n> +\tif ($input =~ m!//!) {\n> \t\treturn undef;\n> \t}\n\nI do not think you need the second check for double-slash. The\npattern you borrowed from the original:\n\n\t/(^|\\/)(|\\.|\\.\\.)($|\\/)/)\n\ncleverly matches an empty string with $2, so you already match\ndouble-slash with $1 = '/' $2 = '' $3 = '/', don't you?\n\n> +\t# it must be correct pathname\n> +\t$input = validate_pathname($input)\n> +\t\tor return undef;\n> +\t# restrictions on ref name according to git-check-ref-format\n> +\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n> \t\treturn undef;\n> \t}\n\nWhy would you need validate_pathname here?\n"},{"id":"27666","messageId":"200609260951.31806.jnareb@gmail.com","threadId":"5695","inReplyTo":"7v7izrw9la.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 5/5] gitweb: Quote filename in HTTP Content-Disposition: header","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-26T07:51:31Z","receivedAt":"2006-09-26T07:51:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>> Finish work started by a2f3db2f5de2a3667b0e038aa65e3e097e642e7d commit\n>> (although not documented in commit message) of quoting using quotemeta\n>> the filename in HTTP -content_disposition header. Uniquify output.\n>>\n>> Just in case filename contains end of line character.\n> \n> What do you mean by \"uniquify output\"?\n\nIt means alsways use 'inline; filename=\"' . (quotemeta ) . '\"',\nand not sometimes qq(...), sometimes \"...\\\"...\\\"\", and sometimes '...'.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"27668","messageId":"200609260955.02869.jnareb@gmail.com","threadId":"5695","inReplyTo":"7v1wpzw9l9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/5] gitweb: Split validate_input into validate_pathname and validate_refname","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-26T07:55:02Z","receivedAt":"2006-09-26T07:55:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia wtorek 26. września 2006 06:11, Junio C Hamano napisał:\n> Jakub Narebski writes:\n> \n> > @@ -387,16 +400,37 @@ sub href(%) {\n> > ## \n======================================================================\n> > ## validation, quoting/unquoting and escaping\n> > \n> > -sub validate_input {\n> > -\tmy $input = shift;\n> > +sub validate_pathname {\n> > +\tmy $input = shift || return undef;\n> > \n> > -\tif ($input =~ m/^[0-9a-fA-F]{40}$/) {\n> > -\t\treturn $input;\n> > +\t# no '.' or '..' as elements of path, i.e. no '.' nor '..'\n> > +\t# at the beginning, at the end, and between slashes.\n> > +\tif ($input =~ m!(^|/)(|\\.|\\.\\.)(/|$)!) {\n> > +\t\treturn undef;\n> > \t}\n> > -\tif ($input =~ m/(^|\\/)(|\\.|\\.\\.)($|\\/)/) {\n> > +\t# no doubled slashes\n> > +\tif ($input =~ m!//!) {\n> > \t\treturn undef;\n> > \t}\n> \n> I do not think you need the second check for double-slash. The\n> pattern you borrowed from the original:\n> \n> \t/(^|\\/)(|\\.|\\.\\.)($|\\/)/)\n> \n> cleverly matches an empty string with $2, so you already match\n> double-slash with $1 = '/' $2 = '' $3 = '/', don't you?\n\nDo I need to resend patch, then, to remove this unnecessary check?\n\n> > +\t# it must be correct pathname\n> > +\t$input = validate_pathname($input)\n> > +\t\tor return undef;\n> > +\t# restrictions on ref name according to git-check-ref-format\n> > +\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n> > \t\treturn undef;\n> > \t}\n> \n> Why would you need validate_pathname here?\n\nrefname _must_ be a valid pathname, no? It means for example that it\ncannot have double slashes, not NUL (the only thinkg not covered by\ngit-check-ref-format restrictions). Well, we could add that to regexp\ninstead...\n\n-- \nJakub Narebski\nPoland\n"}]}