{"thread":{"id":"3863","subject":"[PATCH] Shell utilities: Guard against expr' magic tokens.","startedAt":"2006-04-13T22:01:24Z","lastAt":"2006-04-14T02:12:24Z","messageCount":3,"participants":["Mark Wooding","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"18627","messageId":"slrne3tihk.1dq.mdw@metalzone.distorted.org.uk","threadId":"3863","inReplyTo":null,"subject":"[PATCH] Shell utilities: Guard against expr' magic tokens.","fromName":"Mark Wooding","fromEmail":"mdw@distorted.org.uk","sentAt":"2006-04-13T22:01:24Z","receivedAt":"2006-04-13T22:01:24Z","isPatch":true,"sender":{"key":"mdw@distorted.org.uk","avatar":null},"body":"From: Mark Wooding <mdw@distorted.org.uk>\n\nSome words, e.g., `match', are special to expr(1), and cause strange\nparsing effects.  Track down all uses of expr and mangle the arguments\nso that this isn't a problem.\n\nSigned-off-by: Mark Wooding <mdw@distorted.org.uk>\n---\nAmusing one, this.  I hacked on one of my projects, messing with a\nsimple glob matching function.  Being uncreative, I called my topic\nbranch `match'.  When I was ready, I switched back to my master branch\nand said\n\n  $ git pull . match\n  Already up-to-date.\n\nOh.  I checked.  Nope, not up-to-date.  I tried \n\n  $ git merge fast HEAD match, and that\n\nand that did the right thing.  But I was puzzled.  I fired up the\ngit-bisect machinery and tried to find a good version to no avail.  And\nthen, comparing `sh -x' traces of git-fetch, I noticed what had gone\nwrong.\n\nThere's a line in git-parse-remote.sh, in canon_refs_list_for_fetch,\nwhich says\n\n  expr \"$ref\" : '.*:' >/dev/null || ref=\"${ref}:\"\n\nIn my case, $ref is `match', so this expands to\n\n  expr match : '.*:' >...\n\nUnfortunately, GNU expr has a magic keyword `match'.  So what this does\nis compare `:' to the regexp `.*:', which /succeeds/, even though POSIX\nexpr without the `match' keyword would do the right thing and fail.  So\n$ref never has a `:' appended, which makes the later parsing fail, and\nall sorts of strange things happen.\n\nThis patch puts magical extra characters in expr regexp calls\nthroughout the shell bits of GIT, to robustify them against this kind of\ncrapness.\n\nThere's a small chance I got something wrong while making this fix.  I\nwas fairly careful, though, and ran the test suite without any\nproblems.  I also checked Cogito, though that has no truck with expr.\n\n---\n\n git-cherry.sh         |    2 +-\n git-clone.sh          |    6 +++---\n git-commit.sh         |    4 ++--\n git-fetch.sh          |   18 +++++++++---------\n git-format-patch.sh   |    4 ++--\n git-merge-one-file.sh |    2 +-\n git-parse-remote.sh   |   20 ++++++++++----------\n git-rebase.sh         |    2 +-\n git-tag.sh            |    2 +-\n 9 files changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/git-cherry.sh b/git-cherry.sh\nindex 1a62320..f0e8831 100755\n--- a/git-cherry.sh\n+++ b/git-cherry.sh\n@@ -20,7 +20,7 @@ case \"$1\" in -v) verbose=t; shift ;; esa\n \n case \"$#,$1\" in\n 1,*..*)\n-    upstream=$(expr \"$1\" : '\\(.*\\)\\.\\.') ours=$(expr \"$1\" : '.*\\.\\.\\(.*\\)$')\n+    upstream=$(expr \"z$1\" : 'z\\(.*\\)\\.\\.') ours=$(expr \"z$1\" : '.*\\.\\.\\(.*\\)$')\n     set x \"$upstream\" \"$ours\"\n     shift ;;\n esac\ndiff --git a/git-clone.sh b/git-clone.sh\nindex c013e48..0805168 100755\n--- a/git-clone.sh\n+++ b/git-clone.sh\n@@ -38,12 +38,12 @@ Perhaps git-update-server-info needs to \n \t}\n \twhile read sha1 refname\n \tdo\n-\t\tname=`expr \"$refname\" : 'refs/\\(.*\\)'` &&\n+\t\tname=`expr \"z$refname\" : 'zrefs/\\(.*\\)'` &&\n \t\tcase \"$name\" in\n \t\t*^*)\tcontinue;;\n \t\tesac\n \t\tif test -n \"$use_separate_remote\" &&\n-\t\t   branch_name=`expr \"$name\" : 'heads/\\(.*\\)'`\n+\t\t   branch_name=`expr \"z$name\" : 'zheads/\\(.*\\)'`\n \t\tthen\n \t\t\ttname=\"remotes/$origin/$branch_name\"\n \t\telse\n@@ -346,7 +346,7 @@ then\n \t\t# new style repository with a symref HEAD).\n \t\t# Ideally we should skip the guesswork but for now\n \t\t# opt for minimum change.\n-\t\thead_sha1=`expr \"$head_sha1\" : 'ref: refs/heads/\\(.*\\)'`\n+\t\thead_sha1=`expr \"z$head_sha1\" : 'zref: refs/heads/\\(.*\\)'`\n \t\thead_sha1=`cat \"$GIT_DIR/$remote_top/$head_sha1\"`\n \t\t;;\n \tesac\ndiff --git a/git-commit.sh b/git-commit.sh\nindex bd3dc71..01c73bd 100755\n--- a/git-commit.sh\n+++ b/git-commit.sh\n@@ -549,8 +549,8 @@ fi >>\"$GIT_DIR\"/COMMIT_EDITMSG\n # Author\n if test '' != \"$force_author\"\n then\n-\tGIT_AUTHOR_NAME=`expr \"$force_author\" : '\\(.*[^ ]\\) *<.*'` &&\n-\tGIT_AUTHOR_EMAIL=`expr \"$force_author\" : '.*\\(<.*\\)'` &&\n+\tGIT_AUTHOR_NAME=`expr \"z$force_author\" : 'z\\(.*[^ ]\\) *<.*'` &&\n+\tGIT_AUTHOR_EMAIL=`expr \"z$force_author\" : '.*\\(<.*\\)'` &&\n \ttest '' != \"$GIT_AUTHOR_NAME\" &&\n \ttest '' != \"$GIT_AUTHOR_EMAIL\" ||\n \tdie \"malformatted --author parameter\"\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex 954901d..711650f 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -112,7 +112,7 @@ append_fetch_head () {\n     *)\n \tnote_=\"$remote_name of \" ;;\n     esac\n-    remote_1_=$(expr \"$remote_\" : '\\(.*\\)\\.git/*$') &&\n+    remote_1_=$(expr \"z$remote_\" : 'z\\(.*\\)\\.git/*$') &&\n \tremote_=\"$remote_1_\"\n     note_=\"$note_$remote_\"\n \n@@ -245,22 +245,22 @@ fetch_main () {\n \n       # These are relative path from $GIT_DIR, typically starting at refs/\n       # but may be HEAD\n-      if expr \"$ref\" : '\\.' >/dev/null\n+      if expr \"z$ref\" : 'z\\.' >/dev/null\n       then\n \t  not_for_merge=t\n-\t  ref=$(expr \"$ref\" : '\\.\\(.*\\)')\n+\t  ref=$(expr \"z$ref\" : 'z\\.\\(.*\\)')\n       else\n \t  not_for_merge=\n       fi\n-      if expr \"$ref\" : '\\+' >/dev/null\n+      if expr \"z$ref\" : 'z\\+' >/dev/null\n       then\n \t  single_force=t\n-\t  ref=$(expr \"$ref\" : '\\+\\(.*\\)')\n+\t  ref=$(expr \"z$ref\" : 'z\\+\\(.*\\)')\n       else\n \t  single_force=\n       fi\n-      remote_name=$(expr \"$ref\" : '\\([^:]*\\):')\n-      local_name=$(expr \"$ref\" : '[^:]*:\\(.*\\)')\n+      remote_name=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n+      local_name=$(expr \"z$ref\" : 'z[^:]*:\\(.*\\)')\n \n       rref=\"$rref$LF$remote_name\"\n \n@@ -276,7 +276,7 @@ fetch_main () {\n \t      print \"$u\";\n \t  ' \"$remote_name\")\n \t  head=$(curl -nsfL $curl_extra_args \"$remote/$remote_name_quoted\") &&\n-\t  expr \"$head\" : \"$_x40\\$\" >/dev/null ||\n+\t  expr \"z$head\" : \"z$_x40\\$\" >/dev/null ||\n \t\t  die \"Failed to fetch $remote_name from $remote\"\n \t  echo >&2 Fetching \"$remote_name from $remote\" using http\n \t  git-http-fetch -v -a \"$head\" \"$remote/\" || exit\n@@ -362,7 +362,7 @@ fetch_main () {\n \t\t  break ;;\n \t      esac\n \t  done\n-\t  local_name=$(expr \"$found\" : '[^:]*:\\(.*\\)')\n+\t  local_name=$(expr \"z$found\" : 'z[^:]*:\\(.*\\)')\n \t  append_fetch_head \"$sha1\" \"$remote\" \\\n \t\t  \"$remote_name\" \"$remote_nick\" \"$local_name\" \"$not_for_merge\"\n       done\ndiff --git a/git-format-patch.sh b/git-format-patch.sh\nindex 2ebf7e8..c7133bc 100755\n--- a/git-format-patch.sh\n+++ b/git-format-patch.sh\n@@ -126,8 +126,8 @@ for revpair\n do\n \tcase \"$revpair\" in\n \t?*..?*)\n-\t\trev1=`expr \"$revpair\" : '\\(.*\\)\\.\\.'`\n-\t\trev2=`expr \"$revpair\" : '.*\\.\\.\\(.*\\)'`\n+\t\trev1=`expr \"z$revpair\" : 'z\\(.*\\)\\.\\.'`\n+\t\trev2=`expr \"z$revpair\" : 'z.*\\.\\.\\(.*\\)'`\n \t\t;;\n \t*)\n \t\trev1=\"$revpair^\"\ndiff --git a/git-merge-one-file.sh b/git-merge-one-file.sh\nindex 5349a1c..5619409 100755\n--- a/git-merge-one-file.sh\n+++ b/git-merge-one-file.sh\n@@ -26,7 +26,7 @@ #\n \tfi\n \tif test -f \"$4\"; then\n \t\trm -f -- \"$4\" &&\n-\t\trmdir -p \"$(expr \"$4\" : '\\(.*\\)/')\" 2>/dev/null || :\n+\t\trmdir -p \"$(expr \"z$4\" : 'z\\(.*\\)/')\" 2>/dev/null || :\n \tfi &&\n \t\texec git-update-index --remove -- \"$4\"\n \t;;\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 63f2281..65c66d5 100755\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -8,8 +8,8 @@ get_data_source () {\n \tcase \"$1\" in\n \t*/*)\n \t\t# Not so fast.\tThis could be the partial URL shorthand...\n-\t\ttoken=$(expr \"$1\" : '\\([^/]*\\)/')\n-\t\tremainder=$(expr \"$1\" : '[^/]*/\\(.*\\)')\n+\t\ttoken=$(expr \"z$1\" : 'z\\([^/]*\\)/')\n+\t\tremainder=$(expr \"z$1\" : 'z[^/]*/\\(.*\\)')\n \t\tif test -f \"$GIT_DIR/branches/$token\"\n \t\tthen\n \t\t\techo branches-partial\n@@ -43,8 +43,8 @@ get_remote_url () {\n \tbranches)\n \t\tsed -e 's/#.*//' \"$GIT_DIR/branches/$1\" ;;\n \tbranches-partial)\n-\t\ttoken=$(expr \"$1\" : '\\([^/]*\\)/')\n-\t\tremainder=$(expr \"$1\" : '[^/]*/\\(.*\\)')\n+\t\ttoken=$(expr \"z$1\" : 'z\\([^/]*\\)/')\n+\t\tremainder=$(expr \"z$1\" : 'z[^/]*/\\(.*\\)')\n \t\turl=$(sed -e 's/#.*//' \"$GIT_DIR/branches/$token\")\n \t\techo \"$url/$remainder\"\n \t\t;;\n@@ -77,13 +77,13 @@ canon_refs_list_for_fetch () {\n \t\tforce=\n \t\tcase \"$ref\" in\n \t\t+*)\n-\t\t\tref=$(expr \"$ref\" : '\\+\\(.*\\)')\n+\t\t\tref=$(expr \"z$ref\" : 'z\\+\\(.*\\)')\n \t\t\tforce=+\n \t\t\t;;\n \t\tesac\n-\t\texpr \"$ref\" : '.*:' >/dev/null || ref=\"${ref}:\"\n-\t\tremote=$(expr \"$ref\" : '\\([^:]*\\):')\n-\t\tlocal=$(expr \"$ref\" : '[^:]*:\\(.*\\)')\n+\t\texpr \"z$ref\" : 'z.*:' >/dev/null || ref=\"${ref}:\"\n+\t\tremote=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n+\t\tlocal=$(expr \"z$ref\" : 'z[^:]*:\\(.*\\)')\n \t\tcase \"$remote\" in\n \t\t'') remote=HEAD ;;\n \t\trefs/heads/* | refs/tags/* | refs/remotes/*) ;;\n@@ -97,7 +97,7 @@ canon_refs_list_for_fetch () {\n \t\t*) local=\"refs/heads/$local\" ;;\n \t\tesac\n \n-\t\tif local_ref_name=$(expr \"$local\" : 'refs/\\(.*\\)')\n+\t\tif local_ref_name=$(expr \"z$local\" : 'zrefs/\\(.*\\)')\n \t\tthen\n \t\t   git-check-ref-format \"$local_ref_name\" ||\n \t\t   die \"* refusing to create funny ref '$local_ref_name' locally\"\n@@ -171,7 +171,7 @@ get_remote_refs_for_fetch () {\n \n resolve_alternates () {\n \t# original URL (xxx.git)\n-\ttop_=`expr \"$1\" : '\\([^:]*:/*[^/]*\\)/'`\n+\ttop_=`expr \"z$1\" : 'z\\([^:]*:/*[^/]*\\)/'`\n \twhile read path\n \tdo\n \t\tcase \"$path\" in\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 5956f06..86dfe9c 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -94,7 +94,7 @@ case \"$#\" in\n \t;;\n *)\n \tbranch_name=`git symbolic-ref HEAD` || die \"No current branch\"\n-\tbranch_name=`expr \"$branch_name\" : 'refs/heads/\\(.*\\)'`\n+\tbranch_name=`expr \"z$branch_name\" : 'zrefs/heads/\\(.*\\)'`\n \t;;\n esac\n branch=$(git-rev-parse --verify \"${branch_name}^0\") || exit\ndiff --git a/git-tag.sh b/git-tag.sh\nindex 76e51ed..dc6aa95 100755\n--- a/git-tag.sh\n+++ b/git-tag.sh\n@@ -75,7 +75,7 @@ git-check-ref-format \"tags/$name\" ||\n object=$(git-rev-parse --verify --default HEAD \"$@\") || exit 1\n type=$(git-cat-file -t $object) || exit 1\n tagger=$(git-var GIT_COMMITTER_IDENT) || exit 1\n-: ${username:=$(expr \"$tagger\" : '\\(.*>\\)')}\n+: ${username:=$(expr \"z$tagger\" : 'z\\(.*>\\)')}\n \n trap 'rm -f \"$GIT_DIR\"/TAG_TMP* \"$GIT_DIR\"/TAG_FINALMSG \"$GIT_DIR\"/TAG_EDITMSG' 0\n \n\n\n\n-- [mdw]\n"},{"id":"18631","messageId":"7vodz5vyd6.fsf@assigned-by-dhcp.cox.net","threadId":"3863","inReplyTo":"slrne3tihk.1dq.mdw@metalzone.distorted.org.uk","subject":"Re: [PATCH] Shell utilities: Guard against expr' magic tokens.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-04-13T23:39:49Z","receivedAt":"2006-04-13T23:39:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Wooding <mdw@distorted.org.uk> writes:\n\n> From: Mark Wooding <mdw@distorted.org.uk>\n>\n> Some words, e.g., `match', are special to expr(1), and cause strange\n> parsing effects.  Track down all uses of expr and mangle the arguments\n> so that this isn't a problem.\n\nGaaaaaaaaaah.  \n\n    http://www.opengroup.org/onlinepubs/009695399/utilities/expr.html\n\nsays use of length, substr, index, match as string arguments\nproduces unspecified results, so obviously the program was\nwrong.\n\nThanks.\n"},{"id":"18632","messageId":"7v8xq8x5vb.fsf@assigned-by-dhcp.cox.net","threadId":"3863","inReplyTo":"slrne3tihk.1dq.mdw@metalzone.distorted.org.uk","subject":"[PATCH] Fix-up previous expr changes.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-04-14T02:12:24Z","receivedAt":"2006-04-14T02:12:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The regexp on the right hand side of expr : operator somehow was\nbroken.\n\n\texpr 'z+pu:refs/tags/ko-pu' : 'z\\+\\(.*\\)'\n\ndoes not strip '+'; write 'z+\\(.*\\)' instead.\n\nWe probably should switch to shell based substring post 1.3.0;\nthat's not bashism but just POSIX anyway.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n * Funny thing is that before the z prefixing, the code was\n   already broken (we said expr \"$ref\" : '\\+\\(.*\\)'), but\n   somehow it worked.  It could be a bug in expr.\n\n\t# already buggy but did not trigger somehow.\n        : siamese; expr '+pu:ko-pu' : '\\+\\(.*\\)'\n        pu:ko-pu\n        # z prefix exposed the breakage.\n        : siamese; expr 'z+pu:ko-pu' : 'z\\+\\(.*\\)'\n        +pu:ko-pu\n        # the fix-up this patch is about.\n        : siamese; expr 'z+pu:ko-pu' : 'z+\\(.*\\)'\n        pu:ko-pu\n        # this is the way it should have been written from the start.\n        : siamese; expr '+pu:ko-pu' : '+\\(.*\\)'\n        pu:ko-pu\n        # maybe I am using broken expr...\n        : siamese; type expr\n        expr is hashed (/usr/bin/expr)\n        : siamese; /usr/bin/expr --version |head -n2\n        expr (GNU coreutils) 5.94\n        Copyright (C) 2006 Free Software Foundation, Inc.\n\n git-fetch.sh        |    4 ++--\n git-parse-remote.sh |    2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndfdcb558ecf93c0e09b8dab89cff4839e8c95e36\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex 711650f..83143f8 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -252,10 +252,10 @@ fetch_main () {\n       else\n \t  not_for_merge=\n       fi\n-      if expr \"z$ref\" : 'z\\+' >/dev/null\n+      if expr \"z$ref\" : 'z+' >/dev/null\n       then\n \t  single_force=t\n-\t  ref=$(expr \"z$ref\" : 'z\\+\\(.*\\)')\n+\t  ref=$(expr \"z$ref\" : 'z+\\(.*\\)')\n       else\n \t  single_force=\n       fi\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 65c66d5..c9b899e 100755\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -77,7 +77,7 @@ canon_refs_list_for_fetch () {\n \t\tforce=\n \t\tcase \"$ref\" in\n \t\t+*)\n-\t\t\tref=$(expr \"z$ref\" : 'z\\+\\(.*\\)')\n+\t\t\tref=$(expr \"z$ref\" : 'z+\\(.*\\)')\n \t\t\tforce=+\n \t\t\t;;\n \t\tesac\n-- \n1.3.0.rc3.gce03\n"}]}