{"thread":{"id":"55358","subject":"[PATCH] gitweb: redacted e-mail addresses feature.","startedAt":"2021-03-21T00:02:50Z","lastAt":"2021-04-08T22:54:28Z","messageCount":41,"participants":["Georgios Kontaxis via GitGitGadget","Ævar Arnfjörð Bjarmason","brian m. carlson","Georgios Kontaxis","Junio C Hamano","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"419794","messageId":"pull.910.git.1616283780358.gitgitgadget@gmail.com","threadId":"55358","inReplyTo":null,"subject":"[PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T23:42:59Z","receivedAt":"2021-03-21T00:02:50Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"From: Georgios Kontaxis <geko1702+commits@99rst.org>\n\nGitweb extracts content from the Git log and makes it accessible\nover HTTP. As a result, e-mail addresses found in commits are\nexposed to web crawlers. This may result in unsolicited messages.\nThis is a feature for redacting e-mail addresses from the generated\nHTML content.\n\nThis feature does not prevent someone from downloading the\nunredacted commit log and extracting information from it.\nIt aims to hinder the low-effort bulk collection of e-mail\naddresses by web crawlers.\n\nSigned-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n---\n    gitweb: Redacted e-mail addresses feature.\n    \n    Gitweb extracts content from the Git log and makes it accessible over\n    HTTP. As a result, e-mail addresses found in commits are exposed to web\n    crawlers. This may result in unsolicited messages. This is a feature for\n    redacting e-mail addresses from the generated HTML content.\n    \n    This feature does not prevent someone from downloading the unredacted\n    commit log and extracting information from it. It aims to hinder the\n    low-effort bulk collection of e-mail addresses by web crawlers.\n    \n    Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/910\n\n Documentation/gitweb.conf.txt | 12 ++++++++++++\n gitweb/gitweb.perl            | 36 ++++++++++++++++++++++++++++++++---\n 2 files changed, 45 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba98b..10653d8670a8 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -896,6 +896,18 @@ same as of the snippet above:\n It is an error to specify a ref that does not pass \"git check-ref-format\"\n scrutiny. Duplicated values are filtered.\n \n+email_privacy::\n+    Redact e-mail addresses from the generated HTML, etc. content.\n+    This hides e-mail addresses found in the commit log from web crawlers.\n+    Enabled by default.\n++\n+It is highly recommended to keep this feature enabled unless web crawlers\n+are hindered in some other way. You can disable this feature as shown below:\n++\n+---------------------------------------------------------------------------\n+$feature{'email_privacy'}{'default'} = [0];\n+---------------------------------------------------------------------------\n+\n \n EXAMPLES\n --------\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782eccb..9d21c2583e18 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -569,6 +569,15 @@ sub evaluate_uri {\n \t\t'sub' => \\&feature_extra_branch_refs,\n \t\t'override' => 0,\n \t\t'default' => []},\n+\n+    # Redact e-mail addresses.\n+\n+    # To disable system wide have in $GITWEB_CONFIG\n+    # $feature{'email_privacy'}{'default'} = [0];\n+\t'email_privacy' => {\n+\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [1]},\n );\n \n sub gitweb_get_feature {\n@@ -3471,6 +3480,10 @@ sub parse_tag {\n \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$tag{'author_name'}  = $1;\n \t\t\t\t$tag{'author_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$tag{'author_email'} = \"private\";\n+\t\t\t\t\t$tag{'author'} =~ s/<([^>]+)>/<private>/;\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$tag{'author_name'} = $tag{'author'};\n \t\t\t}\n@@ -3519,6 +3532,10 @@ sub parse_commit_text {\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'author_name'}  = $1;\n \t\t\t\t$co{'author_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$co{'author_email'} = \"private\";\n+\t\t\t\t\t$co{'author'} =~ s/<([^>]+)>/<private>/;\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n@@ -3529,6 +3546,10 @@ sub parse_commit_text {\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'committer_name'}  = $1;\n \t\t\t\t$co{'committer_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$co{'committer_email'} = \"private\";\n+\t\t\t\t\t$co{'committer'} =~ s/<([^>]+)>/<private>/;\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\t}\n@@ -3568,9 +3589,13 @@ sub parse_commit_text {\n \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n \t}\n-\t# remove added spaces\n+\t# remove added spaces, redact e-mail addresses if applicable.\n \tforeach my $line (@commit_lines) {\n \t\t$line =~ s/^    //;\n+\t\tif (gitweb_check_feature('email_privacy') &&\n+\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n+\t\t\t$line =~ s/<([^>]+)>/<private>/;\n+\t\t}\n \t}\n \t$co{'comment'} = \\@commit_lines;\n \n@@ -8060,8 +8085,13 @@ sub git_commitdiff {\n \t\tclose $fd\n \t\t\tor print \"Reading git-diff-tree failed\\n\";\n \t} elsif ($format eq 'patch') {\n-\t\tlocal $/ = undef;\n-\t\tprint <$fd>;\n+\t\twhile (my $line = <$fd>) {\n+\t\t\tif (gitweb_check_feature('email_privacy') &&\n+\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n+\t\t\t\t$line =~ s/<([^>]+)>/<private>/;\n+\t\t\t}\n+\t\t\tprint $line;\n+\t\t}\n \t\tclose $fd\n \t\t\tor print \"Reading git-format-patch failed\\n\";\n \t}\n\nbase-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n-- \ngitgitgadget\n"},{"id":"419825","messageId":"8735wpz699.fsf@evledraar.gmail.com","threadId":"55358","inReplyTo":"pull.910.git.1616283780358.gitgitgadget@gmail.com","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-21T00:42:58Z","receivedAt":"2021-03-21T01:05:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:\n\n> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>\n> Gitweb extracts content from the Git log and makes it accessible\n> over HTTP. As a result, e-mail addresses found in commits are\n> exposed to web crawlers. This may result in unsolicited messages.\n> This is a feature for redacting e-mail addresses from the generated\n> HTML content.\n>\n> This feature does not prevent someone from downloading the\n> unredacted commit log and extracting information from it.\n> It aims to hinder the low-effort bulk collection of e-mail\n> addresses by web crawlers.\n\nSo web crawlers that aren't going to obey robots.txt?\n\nI'm not opposed to this feature, but a glance at gitweb's documentation\nseems to show that we don't discuss how to set robots.txt up for it at\nall.\n\nPerhaps having that in the docs or otherwise in the default setup would\nget us most of the win of this feature?\n\n> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n> ---\n\nOdd:\n\n>     gitweb: Redacted e-mail addresses feature.\n>     \n>     Gitweb extracts content from the Git log and makes it accessible over\n>     HTTP. As a result, e-mail addresses found in commits are exposed to web\n>     crawlers. This may result in unsolicited messages. This is a feature for\n>     redacting e-mail addresses from the generated HTML content.\n>     \n>     This feature does not prevent someone from downloading the unredacted\n>     commit log and extracting information from it. It aims to hinder the\n>     low-effort bulk collection of e-mail addresses by web crawlers.\n>     \n>     Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nTo have this duplication of the patch here below \"---\", some GGG feature\ngone awry?\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/910\n>\n>  Documentation/gitweb.conf.txt | 12 ++++++++++++\n>  gitweb/gitweb.perl            | 36 ++++++++++++++++++++++++++++++++---\n>  2 files changed, 45 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index 7963a79ba98b..10653d8670a8 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -896,6 +896,18 @@ same as of the snippet above:\n>  It is an error to specify a ref that does not pass \"git check-ref-format\"\n>  scrutiny. Duplicated values are filtered.\n>  \n> +email_privacy::\n> +    Redact e-mail addresses from the generated HTML, etc. content.\n> +    This hides e-mail addresses found in the commit log from web crawlers.\n> +    Enabled by default.\n> ++\n> +It is highly recommended to keep this feature enabled unless web crawlers\n> +are hindered in some other way. You can disable this feature as shown below:\n> ++\n> +---------------------------------------------------------------------------\n> +$feature{'email_privacy'}{'default'} = [0];\n> +---------------------------------------------------------------------------\n\nI think there's plenty of gitweb users that are going to be relying on\nthe current behavior, so doesn't it make more sense for this to be\nopt-in rather than opt-out?\n\n>  \n>  EXAMPLES\n>  --------\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0959a782eccb..9d21c2583e18 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -569,6 +569,15 @@ sub evaluate_uri {\n>  \t\t'sub' => \\&feature_extra_branch_refs,\n>  \t\t'override' => 0,\n>  \t\t'default' => []},\n> +\n> +    # Redact e-mail addresses.\n> +\n> +    # To disable system wide have in $GITWEB_CONFIG\n> +    # $feature{'email_privacy'}{'default'} = [0];\n> +\t'email_privacy' => {\n> +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n> +\t\t'override' => 0,\n> +\t\t'default' => [1]},\n>  );\n> [...]\n>  sub gitweb_get_feature {\n> @@ -3471,6 +3480,10 @@ sub parse_tag {\n>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>  \t\t\t\t$tag{'author_name'}  = $1;\n>  \t\t\t\t$tag{'author_email'} = $2;\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n> +\t\t\t\t\t$tag{'author'} =~ s/<([^>]+)>/<private>/;\n> +\t\t\t\t}\n>  \t\t\t} else {\n>  \t\t\t\t$tag{'author_name'} = $tag{'author'};\n>  \t\t\t}\n> @@ -3519,6 +3532,10 @@ sub parse_commit_text {\n>  \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>  \t\t\t\t$co{'author_name'}  = $1;\n>  \t\t\t\t$co{'author_email'} = $2;\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$co{'author_email'} = \"private\";\n> +\t\t\t\t\t$co{'author'} =~ s/<([^>]+)>/<private>/;\n> +\t\t\t\t}\n>  \t\t\t} else {\n>  \t\t\t\t$co{'author_name'} = $co{'author'};\n>  \t\t\t}\n> @@ -3529,6 +3546,10 @@ sub parse_commit_text {\n>  \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n>  \t\t\t\t$co{'committer_name'}  = $1;\n>  \t\t\t\t$co{'committer_email'} = $2;\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$co{'committer_email'} = \"private\";\n> +\t\t\t\t\t$co{'committer'} =~ s/<([^>]+)>/<private>/;\n> +\t\t\t\t}\n>  \t\t\t} else {\n>  \t\t\t\t$co{'committer_name'} = $co{'committer'};\n>  \t\t\t}\n> @@ -3568,9 +3589,13 @@ sub parse_commit_text {\n>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>  \t}\n> -\t# remove added spaces\n> +\t# remove added spaces, redact e-mail addresses if applicable.\n>  \tforeach my $line (@commit_lines) {\n>  \t\t$line =~ s/^    //;\n> +\t\tif (gitweb_check_feature('email_privacy') &&\n> +\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n> +\t\t\t$line =~ s/<([^>]+)>/<private>/;\n> +\t\t}\n>  \t}\n>  \t$co{'comment'} = \\@commit_lines;\n\nAll of these hunks (and the below) should use some new function that\ndoes this feature check + sanitizing instead of copy/pasting mostly the\nsame code N times. e.g.:\n    \n    sub maybe_hide_email {\n        my $email = shift;\n        return $email unless gitweb_check_feature('email_privacy');\n        return hide_email($email);\n    }\n\nthen:\n\n    $tag{author_email} = maybe_hide_email($2);\n\nAlso it looks like this isn't a new issue, but does this need to\nimplement its own E-Mail parser? We ship with Mail::Address for\ngit-send-email, can gitweb (and the elided hide_email() function above)\nuse that too?\n\n\n> @@ -8060,8 +8085,13 @@ sub git_commitdiff {\n>  \t\tclose $fd\n>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>  \t} elsif ($format eq 'patch') {\n> -\t\tlocal $/ = undef;\n> -\t\tprint <$fd>;\n> +\t\twhile (my $line = <$fd>) {\n> +\t\t\tif (gitweb_check_feature('email_privacy') &&\n> +\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n> +\t\t\t\t$line =~ s/<([^>]+)>/<private>/;\n> +\t\t\t}\n> +\t\t\tprint $line;\n> +\t\t}\n>  \t\tclose $fd\n>  \t\t\tor print \"Reading git-format-patch failed\\n\";\n\nIs that \"patch\" output meant for \"git am\"? Won't this severely break\nthat use-case if so?\n"},{"id":"419827","messageId":"YFahDtIkSmpLD2oZ@camp.crustytoothpaste.net","threadId":"55358","inReplyTo":"8735wpz699.fsf@evledraar.gmail.com","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-03-21T01:27:42Z","receivedAt":"2021-03-21T02:12:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-03-21 at 00:42:58, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:\n> \n> > From: Georgios Kontaxis <geko1702+commits@99rst.org>\n> >\n> > Gitweb extracts content from the Git log and makes it accessible\n> > over HTTP. As a result, e-mail addresses found in commits are\n> > exposed to web crawlers. This may result in unsolicited messages.\n> > This is a feature for redacting e-mail addresses from the generated\n> > HTML content.\n> >\n> > This feature does not prevent someone from downloading the\n> > unredacted commit log and extracting information from it.\n> > It aims to hinder the low-effort bulk collection of e-mail\n> > addresses by web crawlers.\n> \n> So web crawlers that aren't going to obey robots.txt?\n> \n> I'm not opposed to this feature, but a glance at gitweb's documentation\n> seems to show that we don't discuss how to set robots.txt up for it at\n> all.\n> \n> Perhaps having that in the docs or otherwise in the default setup would\n> get us most of the win of this feature?\n\nI'm going to guess that the two features are orthogonal.  robots.txt is\ngreat for communicating to well-meaning actors what you do and don't\nwant crawled.  For example, one might ask a web crawler not to crawl\nindividual commits because that creates excessive load on the server.\n\nThis option is about preventing email harvesting, usually for the\npurposes of sending spam.  Spam is email abuse and all reasonable people\nknow it's unacceptable, so by definition the people doing this are bad\nactors and are not likely to honor the robots.txt.  As someone who runs\nhis own mail server, that is certainly my experience.\n\nSo I am in favor of this feature.  I think it mirrors what many other\ntools do in this space and having it as an option is valuable.\n\n> > diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> > index 7963a79ba98b..10653d8670a8 100644\n> > --- a/Documentation/gitweb.conf.txt\n> > +++ b/Documentation/gitweb.conf.txt\n> > @@ -896,6 +896,18 @@ same as of the snippet above:\n> >  It is an error to specify a ref that does not pass \"git check-ref-format\"\n> >  scrutiny. Duplicated values are filtered.\n> >  \n> > +email_privacy::\n> > +    Redact e-mail addresses from the generated HTML, etc. content.\n> > +    This hides e-mail addresses found in the commit log from web crawlers.\n> > +    Enabled by default.\n> > ++\n> > +It is highly recommended to keep this feature enabled unless web crawlers\n> > +are hindered in some other way. You can disable this feature as shown below:\n> > ++\n> > +---------------------------------------------------------------------------\n> > +$feature{'email_privacy'}{'default'} = [0];\n> > +---------------------------------------------------------------------------\n> \n> I think there's plenty of gitweb users that are going to be relying on\n> the current behavior, so doesn't it make more sense for this to be\n> opt-in rather than opt-out?\n\nI agree this make sense as an opt-in feature.  While many people will\nwant to enable it, users who are performing an upgrade won't necessarily\nwant the behavior to change right away.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"419831","messageId":"pull.910.v2.git.1616297564158.gitgitgadget@gmail.com","threadId":"55358","inReplyTo":"pull.910.git.1616283780358.gitgitgadget@gmail.com","subject":"[PATCH v2] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-21T03:32:43Z","receivedAt":"2021-03-21T03:33:53Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"From: Georgios Kontaxis <geko1702+commits@99rst.org>\n\nGitweb extracts content from the Git log and makes it accessible\nover HTTP. As a result, e-mail addresses found in commits are\nexposed to web crawlers and they may not respect robots.txt.\nThis may result in unsolicited messages.\nThis is a feature for redacting e-mail addresses\nfrom the generated HTML, etc. content.\n\nThis feature does not prevent someone from downloading the\nunredacted commit log, e.g., by cloning the repository, and\nextracting information from it.\nIt aims to hinder the low-effort bulk collection of e-mail\naddresses by web crawlers.\n\nChanges since v1:\n- Turned off the feature by default.\n- Removed duplicate code.\n- Added note about Gitweb consumers receiving redacted logs.\n\nSigned-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n---\n    gitweb: Redacted e-mail addresses feature.\n    \n    Gitweb extracts content from the Git log and makes it accessible over\n    HTTP. As a result, e-mail addresses found in commits are exposed to web\n    crawlers. This may result in unsolicited messages. This is a feature for\n    redacting e-mail addresses from the generated HTML content.\n    \n    This feature does not prevent someone from downloading the unredacted\n    commit log and extracting information from it. It aims to hinder the\n    low-effort bulk collection of e-mail addresses by web crawlers.\n    \n    Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/910\n\nRange-diff vs v1:\n\n 1:  6fe6ebdb8e59 ! 1:  74af11ca8bf2 gitweb: redacted e-mail addresses feature.\n     @@ Commit message\n      \n          Gitweb extracts content from the Git log and makes it accessible\n          over HTTP. As a result, e-mail addresses found in commits are\n     -    exposed to web crawlers. This may result in unsolicited messages.\n     -    This is a feature for redacting e-mail addresses from the generated\n     -    HTML content.\n     +    exposed to web crawlers and they may not respect robots.txt.\n     +    This may result in unsolicited messages.\n     +    This is a feature for redacting e-mail addresses\n     +    from the generated HTML, etc. content.\n      \n          This feature does not prevent someone from downloading the\n     -    unredacted commit log and extracting information from it.\n     +    unredacted commit log, e.g., by cloning the repository, and\n     +    extracting information from it.\n          It aims to hinder the low-effort bulk collection of e-mail\n          addresses by web crawlers.\n      \n     +    Changes since v1:\n     +    - Turned off the feature by default.\n     +    - Removed duplicate code.\n     +    - Added note about Gitweb consumers receiving redacted logs.\n     +\n          Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n      \n       ## Documentation/gitweb.conf.txt ##\n     @@ Documentation/gitweb.conf.txt: same as of the snippet above:\n      +email_privacy::\n      +    Redact e-mail addresses from the generated HTML, etc. content.\n      +    This hides e-mail addresses found in the commit log from web crawlers.\n     -+    Enabled by default.\n     ++    Disabled by default.\n      ++\n     -+It is highly recommended to keep this feature enabled unless web crawlers\n     -+are hindered in some other way. You can disable this feature as shown below:\n     ++It is highly recommended to enable this feature unless web crawlers are\n     ++hindered in some other way. Note that crawlers intent on harvesting e-mail\n     ++addresses may disregard robots.txt. You can enable this feature like so:\n      ++\n      +---------------------------------------------------------------------------\n     -+$feature{'email_privacy'}{'default'} = [0];\n     ++$feature{'email_privacy'}{'default'} = [1];\n      +---------------------------------------------------------------------------\n     +++\n     ++Note that if Gitweb is not the final step in a workflow then subsequent\n     ++steps may misbehave because of the redacted information they receive.\n      +\n       \n       EXAMPLES\n     @@ gitweb/gitweb.perl: sub evaluate_uri {\n       \t\t'override' => 0,\n       \t\t'default' => []},\n      +\n     -+    # Redact e-mail addresses.\n     ++\t# Redact e-mail addresses.\n      +\n     -+    # To disable system wide have in $GITWEB_CONFIG\n     -+    # $feature{'email_privacy'}{'default'} = [0];\n     ++\t# To enable system wide have in $GITWEB_CONFIG\n     ++\t# $feature{'email_privacy'}{'default'} = [1];\n      +\t'email_privacy' => {\n      +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n      +\t\t'override' => 0,\n     -+\t\t'default' => [1]},\n     ++\t\t'default' => [0]},\n       );\n       \n       sub gitweb_get_feature {\n     +@@ gitweb/gitweb.perl: sub parse_date {\n     + \treturn %date;\n     + }\n     + \n     ++sub hide_mailaddr_if_private {\n     ++\tmy $line = shift;\n     ++\treturn $line unless (gitweb_check_feature('email_privacy') &&\n     ++\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n     ++\treturn hide_mailaddr($line)\n     ++}\n     ++\n     ++sub hide_mailaddr {\n     ++\tmy $mailaddr = shift;\n     ++\t$mailaddr =~ s/<([^>]*)>/<private>/;\n     ++\treturn $mailaddr;\n     ++}\n     ++\n     + sub parse_tag {\n     + \tmy $tag_id = shift;\n     + \tmy %tag;\n      @@ gitweb/gitweb.perl: sub parse_tag {\n       \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n       \t\t\t\t$tag{'author_name'}  = $1;\n       \t\t\t\t$tag{'author_email'} = $2;\n      +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n      +\t\t\t\t\t$tag{'author_email'} = \"private\";\n     -+\t\t\t\t\t$tag{'author'} =~ s/<([^>]+)>/<private>/;\n     ++\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n      +\t\t\t\t}\n       \t\t\t} else {\n       \t\t\t\t$tag{'author_name'} = $tag{'author'};\n     @@ gitweb/gitweb.perl: sub parse_commit_text {\n       \t\t\t\t$co{'author_email'} = $2;\n      +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n      +\t\t\t\t\t$co{'author_email'} = \"private\";\n     -+\t\t\t\t\t$co{'author'} =~ s/<([^>]+)>/<private>/;\n     ++\t\t\t\t\t$co{'author'} = hide_mailaddr($co{'author'});\n      +\t\t\t\t}\n       \t\t\t} else {\n       \t\t\t\t$co{'author_name'} = $co{'author'};\n     @@ gitweb/gitweb.perl: sub parse_commit_text {\n       \t\t\t\t$co{'committer_email'} = $2;\n      +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n      +\t\t\t\t\t$co{'committer_email'} = \"private\";\n     -+\t\t\t\t\t$co{'committer'} =~ s/<([^>]+)>/<private>/;\n     ++\t\t\t\t\t$co{'committer'} = hide_mailaddr($co{'committer'});\n      +\t\t\t\t}\n       \t\t\t} else {\n       \t\t\t\t$co{'committer_name'} = $co{'committer'};\n     @@ gitweb/gitweb.perl: sub parse_commit_text {\n      +\t# remove added spaces, redact e-mail addresses if applicable.\n       \tforeach my $line (@commit_lines) {\n       \t\t$line =~ s/^    //;\n     -+\t\tif (gitweb_check_feature('email_privacy') &&\n     -+\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n     -+\t\t\t$line =~ s/<([^>]+)>/<private>/;\n     -+\t\t}\n     ++\t\t$line = hide_mailaddr_if_private($line);\n       \t}\n       \t$co{'comment'} = \\@commit_lines;\n       \n     @@ gitweb/gitweb.perl: sub git_commitdiff {\n      -\t\tlocal $/ = undef;\n      -\t\tprint <$fd>;\n      +\t\twhile (my $line = <$fd>) {\n     -+\t\t\tif (gitweb_check_feature('email_privacy') &&\n     -+\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n     -+\t\t\t\t$line =~ s/<([^>]+)>/<private>/;\n     -+\t\t\t}\n     -+\t\t\tprint $line;\n     ++\t\t\tprint hide_mailaddr_if_private($line);\n      +\t\t}\n       \t\tclose $fd\n       \t\t\tor print \"Reading git-format-patch failed\\n\";\n\n\n Documentation/gitweb.conf.txt | 16 +++++++++++++\n gitweb/gitweb.perl            | 42 ++++++++++++++++++++++++++++++++---\n 2 files changed, 55 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba98b..b7af3240177d 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -896,6 +896,22 @@ same as of the snippet above:\n It is an error to specify a ref that does not pass \"git check-ref-format\"\n scrutiny. Duplicated values are filtered.\n \n+email_privacy::\n+    Redact e-mail addresses from the generated HTML, etc. content.\n+    This hides e-mail addresses found in the commit log from web crawlers.\n+    Disabled by default.\n++\n+It is highly recommended to enable this feature unless web crawlers are\n+hindered in some other way. Note that crawlers intent on harvesting e-mail\n+addresses may disregard robots.txt. You can enable this feature like so:\n++\n+---------------------------------------------------------------------------\n+$feature{'email_privacy'}{'default'} = [1];\n+---------------------------------------------------------------------------\n++\n+Note that if Gitweb is not the final step in a workflow then subsequent\n+steps may misbehave because of the redacted information they receive.\n+\n \n EXAMPLES\n --------\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782eccb..210228f32efd 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -569,6 +569,15 @@ sub evaluate_uri {\n \t\t'sub' => \\&feature_extra_branch_refs,\n \t\t'override' => 0,\n \t\t'default' => []},\n+\n+\t# Redact e-mail addresses.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'email_privacy'}{'default'} = [1];\n+\t'email_privacy' => {\n+\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -3449,6 +3458,19 @@ sub parse_date {\n \treturn %date;\n }\n \n+sub hide_mailaddr_if_private {\n+\tmy $line = shift;\n+\treturn $line unless (gitweb_check_feature('email_privacy') &&\n+\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n+\treturn hide_mailaddr($line)\n+}\n+\n+sub hide_mailaddr {\n+\tmy $mailaddr = shift;\n+\t$mailaddr =~ s/<([^>]*)>/<private>/;\n+\treturn $mailaddr;\n+}\n+\n sub parse_tag {\n \tmy $tag_id = shift;\n \tmy %tag;\n@@ -3471,6 +3493,10 @@ sub parse_tag {\n \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$tag{'author_name'}  = $1;\n \t\t\t\t$tag{'author_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$tag{'author_email'} = \"private\";\n+\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$tag{'author_name'} = $tag{'author'};\n \t\t\t}\n@@ -3519,6 +3545,10 @@ sub parse_commit_text {\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'author_name'}  = $1;\n \t\t\t\t$co{'author_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$co{'author_email'} = \"private\";\n+\t\t\t\t\t$co{'author'} = hide_mailaddr($co{'author'});\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n@@ -3529,6 +3559,10 @@ sub parse_commit_text {\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'committer_name'}  = $1;\n \t\t\t\t$co{'committer_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$co{'committer_email'} = \"private\";\n+\t\t\t\t\t$co{'committer'} = hide_mailaddr($co{'committer'});\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\t}\n@@ -3568,9 +3602,10 @@ sub parse_commit_text {\n \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n \t}\n-\t# remove added spaces\n+\t# remove added spaces, redact e-mail addresses if applicable.\n \tforeach my $line (@commit_lines) {\n \t\t$line =~ s/^    //;\n+\t\t$line = hide_mailaddr_if_private($line);\n \t}\n \t$co{'comment'} = \\@commit_lines;\n \n@@ -8060,8 +8095,9 @@ sub git_commitdiff {\n \t\tclose $fd\n \t\t\tor print \"Reading git-diff-tree failed\\n\";\n \t} elsif ($format eq 'patch') {\n-\t\tlocal $/ = undef;\n-\t\tprint <$fd>;\n+\t\twhile (my $line = <$fd>) {\n+\t\t\tprint hide_mailaddr_if_private($line);\n+\t\t}\n \t\tclose $fd\n \t\t\tor print \"Reading git-format-patch failed\\n\";\n \t}\n\nbase-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n-- \ngitgitgadget\n"},{"id":"419832","messageId":"ebd4f3671db8dd8ce195d3384d44e433.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"8735wpz699.fsf@evledraar.gmail.com","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-21T03:38:19Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":">\n> On Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:\n>\n>> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers. This may result in unsolicited messages.\n>> This is a feature for redacting e-mail addresses from the generated\n>> HTML content.\n>>\n>> This feature does not prevent someone from downloading the\n>> unredacted commit log and extracting information from it.\n>> It aims to hinder the low-effort bulk collection of e-mail\n>> addresses by web crawlers.\n>\n> So web crawlers that aren't going to obey robots.txt?\n>\n> I'm not opposed to this feature, but a glance at gitweb's documentation\n> seems to show that we don't discuss how to set robots.txt up for it at\n> all.\n>\n> Perhaps having that in the docs or otherwise in the default setup would\n> get us most of the win of this feature?\n>\nFile robots.txt is basically asking nicely and we should work on that.\nAt the same time crawlers that look for addresses to send SPAM to\nwill probably ignore it so this change is meant for them.\n\n>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>> ---\n>\n> Odd:\n>\n>>     gitweb: Redacted e-mail addresses feature.\n>>\n>>     Gitweb extracts content from the Git log and makes it accessible\n>> over\n>>     HTTP. As a result, e-mail addresses found in commits are exposed to\n>> web\n>>     crawlers. This may result in unsolicited messages. This is a feature\n>> for\n>>     redacting e-mail addresses from the generated HTML content.\n>>\n>>     This feature does not prevent someone from downloading the\n>> unredacted\n>>     commit log and extracting information from it. It aims to hinder the\n>>     low-effort bulk collection of e-mail addresses by web crawlers.\n>>\n>>     Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n>\n> To have this duplication of the patch here below \"---\", some GGG feature\n> gone awry?\n>\n>> Published-As:\n>> https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n>> pr-910/kontaxis/kontaxis/email_privacy-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/910\n>>\n>>  Documentation/gitweb.conf.txt | 12 ++++++++++++\n>>  gitweb/gitweb.perl            | 36 ++++++++++++++++++++++++++++++++---\n>>  2 files changed, 45 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/Documentation/gitweb.conf.txt\n>> b/Documentation/gitweb.conf.txt\n>> index 7963a79ba98b..10653d8670a8 100644\n>> --- a/Documentation/gitweb.conf.txt\n>> +++ b/Documentation/gitweb.conf.txt\n>> @@ -896,6 +896,18 @@ same as of the snippet above:\n>>  It is an error to specify a ref that does not pass \"git\n>> check-ref-format\"\n>>  scrutiny. Duplicated values are filtered.\n>>\n>> +email_privacy::\n>> +    Redact e-mail addresses from the generated HTML, etc. content.\n>> +    This hides e-mail addresses found in the commit log from web\n>> crawlers.\n>> +    Enabled by default.\n>> ++\n>> +It is highly recommended to keep this feature enabled unless web\n>> crawlers\n>> +are hindered in some other way. You can disable this feature as shown\n>> below:\n>> ++\n>> +---------------------------------------------------------------------------\n>> +$feature{'email_privacy'}{'default'} = [0];\n>> +---------------------------------------------------------------------------\n>\n> I think there's plenty of gitweb users that are going to be relying on\n> the current behavior, so doesn't it make more sense for this to be\n> opt-in rather than opt-out?\n>\nMy concern is that Gitweb operators may not understand the need\nfor this feature or maybe won't be aware the feature exists.\n\nNevertheless, I've changed the feature to be off by default.\nPerhaps we can revisit this decision in the future? :)\n\n>>\n>>  EXAMPLES\n>>  --------\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 0959a782eccb..9d21c2583e18 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -569,6 +569,15 @@ sub evaluate_uri {\n>>  \t\t'sub' => \\&feature_extra_branch_refs,\n>>  \t\t'override' => 0,\n>>  \t\t'default' => []},\n>> +\n>> +    # Redact e-mail addresses.\n>> +\n>> +    # To disable system wide have in $GITWEB_CONFIG\n>> +    # $feature{'email_privacy'}{'default'} = [0];\n>> +\t'email_privacy' => {\n>> +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>> +\t\t'override' => 0,\n>> +\t\t'default' => [1]},\n>>  );\n>> [...]\n>>  sub gitweb_get_feature {\n>> @@ -3471,6 +3480,10 @@ sub parse_tag {\n>>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$tag{'author_name'}  = $1;\n>>  \t\t\t\t$tag{'author_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n>> +\t\t\t\t\t$tag{'author'} =~ s/<([^>]+)>/<private>/;\n>> +\t\t\t\t}\n>>  \t\t\t} else {\n>>  \t\t\t\t$tag{'author_name'} = $tag{'author'};\n>>  \t\t\t}\n>> @@ -3519,6 +3532,10 @@ sub parse_commit_text {\n>>  \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$co{'author_name'}  = $1;\n>>  \t\t\t\t$co{'author_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$co{'author_email'} = \"private\";\n>> +\t\t\t\t\t$co{'author'} =~ s/<([^>]+)>/<private>/;\n>> +\t\t\t\t}\n>>  \t\t\t} else {\n>>  \t\t\t\t$co{'author_name'} = $co{'author'};\n>>  \t\t\t}\n>> @@ -3529,6 +3546,10 @@ sub parse_commit_text {\n>>  \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$co{'committer_name'}  = $1;\n>>  \t\t\t\t$co{'committer_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$co{'committer_email'} = \"private\";\n>> +\t\t\t\t\t$co{'committer'} =~ s/<([^>]+)>/<private>/;\n>> +\t\t\t\t}\n>>  \t\t\t} else {\n>>  \t\t\t\t$co{'committer_name'} = $co{'committer'};\n>>  \t\t\t}\n>> @@ -3568,9 +3589,13 @@ sub parse_commit_text {\n>>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>>  \t}\n>> -\t# remove added spaces\n>> +\t# remove added spaces, redact e-mail addresses if applicable.\n>>  \tforeach my $line (@commit_lines) {\n>>  \t\t$line =~ s/^    //;\n>> +\t\tif (gitweb_check_feature('email_privacy') &&\n>> +\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n>> +\t\t\t$line =~ s/<([^>]+)>/<private>/;\n>> +\t\t}\n>>  \t}\n>>  \t$co{'comment'} = \\@commit_lines;\n>\n> All of these hunks (and the below) should use some new function that\n> does this feature check + sanitizing instead of copy/pasting mostly the\n> same code N times. e.g.:\n>\n>     sub maybe_hide_email {\n>         my $email = shift;\n>         return $email unless gitweb_check_feature('email_privacy');\n>         return hide_email($email);\n>     }\n>\n> then:\n>\n>     $tag{author_email} = maybe_hide_email($2);\n>\n> Also it looks like this isn't a new issue, but does this need to\n> implement its own E-Mail parser? We ship with Mail::Address for\n> git-send-email, can gitweb (and the elided hide_email() function above)\n> use that too?\n>\nThanks for the suggestion.\nI've removed the duplicate code.\n\nThe places where there are e-mail addresses today are pretty specific.\n(Pretty much the author, committer fields and the Signed-off-by lines)\n\nIn theory someone could write a comment with a bunch of addresses in it\nbut that would be unstructured text and I think Mail::Address is not good\nwith that.\n\nI can definitely keep working on this topic but maybe in subsequent PRs?\nAssuming we're not exposing any addresses right now or redacting things\nwe shouldn't.\n\n>\n>> @@ -8060,8 +8085,13 @@ sub git_commitdiff {\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>>  \t} elsif ($format eq 'patch') {\n>> -\t\tlocal $/ = undef;\n>> -\t\tprint <$fd>;\n>> +\t\twhile (my $line = <$fd>) {\n>> +\t\t\tif (gitweb_check_feature('email_privacy') &&\n>> +\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/) {\n>> +\t\t\t\t$line =~ s/<([^>]+)>/<private>/;\n>> +\t\t\t}\n>> +\t\t\tprint $line;\n>> +\t\t}\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-format-patch failed\\n\";\n>\n> Is that \"patch\" output meant for \"git am\"? Won't this severely break\n> that use-case if so?\n>\nNot sure who may be consuming that output.\nI've added a note in the documentation for gitweb.conf.\n\nIf a web crawler can get to the information by following URLs\nthen I think we should redact it.\n\nHopefully by documenting this as a potential issue Gitweb operators\ncan create a workaround specific to their use case.\nPossibly implementing access control and leaving this feature off.\n\n"},{"id":"419836","messageId":"xmqqft0pcagu.fsf@gitster.g","threadId":"55358","inReplyTo":"pull.910.git.1616283780358.gitgitgadget@gmail.com","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T06:00:33Z","receivedAt":"2021-03-21T06:01:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> +    # To disable system wide have in $GITWEB_CONFIG\n> +    # $feature{'email_privacy'}{'default'} = [0];\n> +\t'email_privacy' => {\n> +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n> +\t\t'override' => 0,\n> +\t\t'default' => [1]},\n>  );\n\nI do not see why this should default to true.\n\nIt would break existing installations, who have been perfectly happy\nwith the convenience of supplying a ready access to potential new\ncontributors who/which addresses to contact plausible mentors in the\nprojects they are interested in.\n\nAnd more importantly, I do not see why it should be made impossible\nto override per repository/project in a multi-tenant installation.\nSome projects may be more \"privacy\" sensitive than others.  Those\nwho want to use tighter setting should be able to enable it even\nwhen the side-wide default is set to false, no?\n\nThanks.\n"},{"id":"419837","messageId":"xmqqblbdc9n6.fsf@gitster.g","threadId":"55358","inReplyTo":"xmqqft0pcagu.fsf@gitster.g","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T06:18:21Z","receivedAt":"2021-03-21T06:19:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> And more importantly, I do not see why it should be made impossible\n> to override per repository/project in a multi-tenant installation.\n> Some projects may be more \"privacy\" sensitive than others.  Those\n> who want to use tighter setting should be able to enable it even\n> when the side-wide default is set to false, no?\n\nTo answer an inevitable and natural follow-up question preemptively,\nthe primary reason why we have the override => 0 mechanism is so\nthat site administrators can disable certain expensive features\n(like blame, snapshot, etc.) no matter what each project hosted by\nthem wish.\n\nAnd hiding contributor identity would not be a choice that is based\non how expensive the feature is to run.\n"},{"id":"419838","messageId":"20b1b63fe22560205d7e7c50b9f826e5.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"xmqqft0pcagu.fsf@gitster.g","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-21T07:06:55Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> \"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n>> +    # To disable system wide have in $GITWEB_CONFIG\n>> +    # $feature{'email_privacy'}{'default'} = [0];\n>> +\t'email_privacy' => {\n>> +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>> +\t\t'override' => 0,\n>> +\t\t'default' => [1]},\n>>  );\n>\n> I do not see why this should default to true.\n>\nI've changed the default to \"false\". V2 should reflect the change.\n\n> It would break existing installations, who have been perfectly happy\n> with the convenience of supplying a ready access to potential new\n> contributors who/which addresses to contact plausible mentors in the\n> projects they are interested in.\n>\n> And more importantly, I do not see why it should be made impossible\n> to override per repository/project in a multi-tenant installation.\n> Some projects may be more \"privacy\" sensitive than others.  Those\n> who want to use tighter setting should be able to enable it even\n> when the side-wide default is set to false, no?\n>\n> Thanks.\n>\nI was actually thinking about the other way around;\npreventing projects from disabling this feature.\n\nSounds like the \"override\" flag is for other types\nof use cases though. I'll change it to \"true\".\n\nThanks for the feedback.\n\n"},{"id":"419862","messageId":"xmqqsg4obg63.fsf@gitster.g","threadId":"55358","inReplyTo":"20b1b63fe22560205d7e7c50b9f826e5.squirrel@mail.kodaksys.org","subject":"Re: [PATCH] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T16:55:00Z","receivedAt":"2021-03-21T16:55:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n\n>> And more importantly, I do not see why it should be made impossible\n>> to override per repository/project in a multi-tenant installation.\n>> Some projects may be more \"privacy\" sensitive than others.  Those\n>> who want to use tighter setting should be able to enable it even\n>> when the side-wide default is set to false, no?\n>>\n>> Thanks.\n>>\n> I was actually thinking about the other way around;\n> preventing projects from disabling this feature.\n\nYes, it cuts both ways, and override should be allowed for most\ncases (unless absolutely necessary for healthy operation of the\nsystem) for a simple reason that anybody who sets site-wide default\nis not in a better position than those who set per-repository or\nper-project setting to judge what is good for them.\n\nThanks.\n"},{"id":"419875","messageId":"pull.910.v3.git.1616347731514.gitgitgadget@gmail.com","threadId":"55358","inReplyTo":"pull.910.v2.git.1616297564158.gitgitgadget@gmail.com","subject":"[PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-21T17:28:50Z","receivedAt":"2021-03-21T17:29:27Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"From: Georgios Kontaxis <geko1702+commits@99rst.org>\n\nGitweb extracts content from the Git log and makes it accessible\nover HTTP. As a result, e-mail addresses found in commits are\nexposed to web crawlers and they may not respect robots.txt.\nThis may result in unsolicited messages.\nThis is a feature for redacting e-mail addresses\nfrom the generated HTML, etc. content.\n\nThis feature does not prevent someone from downloading the\nunredacted commit log, e.g., by cloning the repository, and\nextracting information from it.\nIt aims to hinder the low-effort bulk collection of e-mail\naddresses by web crawlers.\n\nChanges since v1:\n- Turned off the feature by default.\n- Removed duplicate code.\n- Added note about Gitweb consumers receiving redacted logs.\n\nChanges since v2:\n- The feature can be set on a per-project basis. ('override' => 1)\n\nSigned-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n---\n    gitweb: Redacted e-mail addresses feature.\n    \n    Gitweb extracts content from the Git log and makes it accessible over\n    HTTP. As a result, e-mail addresses found in commits are exposed to web\n    crawlers. This may result in unsolicited messages. This is a feature for\n    redacting e-mail addresses from the generated HTML content.\n    \n    This feature does not prevent someone from downloading the unredacted\n    commit log and extracting information from it. It aims to hinder the\n    low-effort bulk collection of e-mail addresses by web crawlers.\n    \n    Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/910\n\nRange-diff vs v2:\n\n 1:  74af11ca8bf2 ! 1:  930cdefe7ee0 gitweb: redacted e-mail addresses feature.\n     @@ Commit message\n          - Removed duplicate code.\n          - Added note about Gitweb consumers receiving redacted logs.\n      \n     +    Changes since v2:\n     +    - The feature can be set on a per-project basis. ('override' => 1)\n     +\n          Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n      \n       ## Documentation/gitweb.conf.txt ##\n     @@ gitweb/gitweb.perl: sub evaluate_uri {\n      +\t# $feature{'email_privacy'}{'default'} = [1];\n      +\t'email_privacy' => {\n      +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n     -+\t\t'override' => 0,\n     ++\t\t'override' => 1,\n      +\t\t'default' => [0]},\n       );\n       \n\n\n Documentation/gitweb.conf.txt | 16 +++++++++++++\n gitweb/gitweb.perl            | 42 ++++++++++++++++++++++++++++++++---\n 2 files changed, 55 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba98b..b7af3240177d 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -896,6 +896,22 @@ same as of the snippet above:\n It is an error to specify a ref that does not pass \"git check-ref-format\"\n scrutiny. Duplicated values are filtered.\n \n+email_privacy::\n+    Redact e-mail addresses from the generated HTML, etc. content.\n+    This hides e-mail addresses found in the commit log from web crawlers.\n+    Disabled by default.\n++\n+It is highly recommended to enable this feature unless web crawlers are\n+hindered in some other way. Note that crawlers intent on harvesting e-mail\n+addresses may disregard robots.txt. You can enable this feature like so:\n++\n+---------------------------------------------------------------------------\n+$feature{'email_privacy'}{'default'} = [1];\n+---------------------------------------------------------------------------\n++\n+Note that if Gitweb is not the final step in a workflow then subsequent\n+steps may misbehave because of the redacted information they receive.\n+\n \n EXAMPLES\n --------\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782eccb..174cc566d97d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -569,6 +569,15 @@ sub evaluate_uri {\n \t\t'sub' => \\&feature_extra_branch_refs,\n \t\t'override' => 0,\n \t\t'default' => []},\n+\n+\t# Redact e-mail addresses.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'email_privacy'}{'default'} = [1];\n+\t'email_privacy' => {\n+\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n+\t\t'override' => 1,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -3449,6 +3458,19 @@ sub parse_date {\n \treturn %date;\n }\n \n+sub hide_mailaddr_if_private {\n+\tmy $line = shift;\n+\treturn $line unless (gitweb_check_feature('email_privacy') &&\n+\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n+\treturn hide_mailaddr($line)\n+}\n+\n+sub hide_mailaddr {\n+\tmy $mailaddr = shift;\n+\t$mailaddr =~ s/<([^>]*)>/<private>/;\n+\treturn $mailaddr;\n+}\n+\n sub parse_tag {\n \tmy $tag_id = shift;\n \tmy %tag;\n@@ -3471,6 +3493,10 @@ sub parse_tag {\n \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$tag{'author_name'}  = $1;\n \t\t\t\t$tag{'author_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$tag{'author_email'} = \"private\";\n+\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$tag{'author_name'} = $tag{'author'};\n \t\t\t}\n@@ -3519,6 +3545,10 @@ sub parse_commit_text {\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'author_name'}  = $1;\n \t\t\t\t$co{'author_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$co{'author_email'} = \"private\";\n+\t\t\t\t\t$co{'author'} = hide_mailaddr($co{'author'});\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n@@ -3529,6 +3559,10 @@ sub parse_commit_text {\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'committer_name'}  = $1;\n \t\t\t\t$co{'committer_email'} = $2;\n+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n+\t\t\t\t\t$co{'committer_email'} = \"private\";\n+\t\t\t\t\t$co{'committer'} = hide_mailaddr($co{'committer'});\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\t}\n@@ -3568,9 +3602,10 @@ sub parse_commit_text {\n \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n \t}\n-\t# remove added spaces\n+\t# remove added spaces, redact e-mail addresses if applicable.\n \tforeach my $line (@commit_lines) {\n \t\t$line =~ s/^    //;\n+\t\t$line = hide_mailaddr_if_private($line);\n \t}\n \t$co{'comment'} = \\@commit_lines;\n \n@@ -8060,8 +8095,9 @@ sub git_commitdiff {\n \t\tclose $fd\n \t\t\tor print \"Reading git-diff-tree failed\\n\";\n \t} elsif ($format eq 'patch') {\n-\t\tlocal $/ = undef;\n-\t\tprint <$fd>;\n+\t\twhile (my $line = <$fd>) {\n+\t\t\tprint hide_mailaddr_if_private($line);\n+\t\t}\n \t\tclose $fd\n \t\t\tor print \"Reading git-format-patch failed\\n\";\n \t}\n\nbase-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n-- \ngitgitgadget\n"},{"id":"419878","messageId":"87r1k8qs73.fsf@evledraar.gmail.com","threadId":"55358","inReplyTo":"pull.910.v3.git.1616347731514.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-21T18:26:08Z","receivedAt":"2021-03-21T18:32:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:\n\n> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>\n> Gitweb extracts content from the Git log and makes it accessible\n> over HTTP. As a result, e-mail addresses found in commits are\n> exposed to web crawlers and they may not respect robots.txt.\n> This may result in unsolicited messages.\n> This is a feature for redacting e-mail addresses\n> from the generated HTML, etc. content.\n>\n> This feature does not prevent someone from downloading the\n> unredacted commit log, e.g., by cloning the repository, and\n> extracting information from it.\n> It aims to hinder the low-effort bulk collection of e-mail\n> addresses by web crawlers.\n>\n> Changes since v1:\n> - Turned off the feature by default.\n> - Removed duplicate code.\n> - Added note about Gitweb consumers receiving redacted logs.\n>\n> Changes since v2:\n> - The feature can be set on a per-project basis. ('override' => 1)\n>\n> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n> ---\n>     gitweb: Redacted e-mail addresses feature.\n>     \n>     Gitweb extracts content from the Git log and makes it accessible over\n>     HTTP. As a result, e-mail addresses found in commits are exposed to web\n>     crawlers. This may result in unsolicited messages. This is a feature for\n>     redacting e-mail addresses from the generated HTML content.\n>     \n>     This feature does not prevent someone from downloading the unredacted\n>     commit log and extracting information from it. It aims to hinder the\n>     low-effort bulk collection of e-mail addresses by web crawlers.\n>     \n>     Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/910\n>\n> Range-diff vs v2:\n>\n>  1:  74af11ca8bf2 ! 1:  930cdefe7ee0 gitweb: redacted e-mail addresses feature.\n>      @@ Commit message\n>           - Removed duplicate code.\n>           - Added note about Gitweb consumers receiving redacted logs.\n>       \n>      +    Changes since v2:\n>      +    - The feature can be set on a per-project basis. ('override' => 1)\n>      +\n>           Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>       \n>        ## Documentation/gitweb.conf.txt ##\n>      @@ gitweb/gitweb.perl: sub evaluate_uri {\n>       +\t# $feature{'email_privacy'}{'default'} = [1];\n>       +\t'email_privacy' => {\n>       +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>      -+\t\t'override' => 0,\n>      ++\t\t'override' => 1,\n>       +\t\t'default' => [0]},\n>        );\n>        \n>\n>\n>  Documentation/gitweb.conf.txt | 16 +++++++++++++\n>  gitweb/gitweb.perl            | 42 ++++++++++++++++++++++++++++++++---\n>  2 files changed, 55 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index 7963a79ba98b..b7af3240177d 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -896,6 +896,22 @@ same as of the snippet above:\n>  It is an error to specify a ref that does not pass \"git check-ref-format\"\n>  scrutiny. Duplicated values are filtered.\n>  \n> +email_privacy::\n> +    Redact e-mail addresses from the generated HTML, etc. content.\n> +    This hides e-mail addresses found in the commit log from web crawlers.\n> +    Disabled by default.\n> ++\n> +It is highly recommended to enable this feature unless web crawlers are\n> +hindered in some other way. Note that crawlers intent on harvesting e-mail\n> +addresses may disregard robots.txt. You can enable this feature like so:\n> ++\n> +---------------------------------------------------------------------------\n> +$feature{'email_privacy'}{'default'} = [1];\n> +---------------------------------------------------------------------------\n> ++\n> +Note that if Gitweb is not the final step in a workflow then subsequent\n> +steps may misbehave because of the redacted information they receive.\n> +\n>  \n>  EXAMPLES\n>  --------\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0959a782eccb..174cc566d97d 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -569,6 +569,15 @@ sub evaluate_uri {\n>  \t\t'sub' => \\&feature_extra_branch_refs,\n>  \t\t'override' => 0,\n>  \t\t'default' => []},\n> +\n> +\t# Redact e-mail addresses.\n> +\n> +\t# To enable system wide have in $GITWEB_CONFIG\n> +\t# $feature{'email_privacy'}{'default'} = [1];\n> +\t'email_privacy' => {\n> +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n> +\t\t'override' => 1,\n> +\t\t'default' => [0]},\n>  );\n>  \n>  sub gitweb_get_feature {\n> @@ -3449,6 +3458,19 @@ sub parse_date {\n>  \treturn %date;\n>  }\n>  \n> [snip]\n\nSo in the v1 feedback I suggested:\n\nBEGIN QUOTE\n    sub maybe_hide_email {\n        my $email = shift;\n        return $email unless gitweb_check_feature('email_privacy');\n        return hide_email($email);\n    }\n\nthen:\n\n    $tag{author_email} = maybe_hide_email($2);\nEND QUOTE\n\nBut:\n\n>  sub parse_tag {\n>  \tmy $tag_id = shift;\n>  \tmy %tag;\n> @@ -3471,6 +3493,10 @@ sub parse_tag {\n>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>  \t\t\t\t$tag{'author_name'}  = $1;\n>  \t\t\t\t$tag{'author_email'} = $2;\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n> +\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n> +\t\t\t\t}\n\nThis code seems quite awkward, we've already done the regex match, but\nthis code:\n\n> [snip]\n> +sub hide_mailaddr_if_private {\n> +\tmy $line = shift;\n> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n> +\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n> +\treturn hide_mailaddr($line)\n> +}\n> +\n> +sub hide_mailaddr {\n> +\tmy $mailaddr = shift;\n> +\t$mailaddr =~ s/<([^>]*)>/<private>/;\n> +\treturn $mailaddr;\n> +}\n\nIs going to do it again incrementally, and then just act on a\nsearch-replacement if we've got the feature enabled.\n\nIt seems much simpler to just turn your:\n\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n> +\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n> +\t\t\t\t}\n\nInto:\n\n    $tag{'author'} = maybe_hide_mailaddr($tag{author}, \\$tag{author_email});\n\nUsing:\n\n    sub maybe_hide_email {\n        my ($email, $ref) = shift;\n        return $email unless gitweb_check_feature('email_privacy');\n        $$ref = \"private\" if $ref;\n        return hide_email($email);\n    }\n\nWhich also works for the case where you don't have a \"private\" hash key\nto assign to. But maybe it overcomplicates things...\n\n>  \t\t\t} else {\n>  \t\t\t\t$tag{'author_name'} = $tag{'author'};\n>  \t\t\t}\n> @@ -3519,6 +3545,10 @@ sub parse_commit_text {\n>  \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>  \t\t\t\t$co{'author_name'}  = $1;\n>  \t\t\t\t$co{'author_email'} = $2;\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$co{'author_email'} = \"private\";\n> +\t\t\t\t\t$co{'author'} = hide_mailaddr($co{'author'});\n> +\t\t\t\t}\n>  \t\t\t} else {\n>  \t\t\t\t$co{'author_name'} = $co{'author'};\n>  \t\t\t}\n> @@ -3529,6 +3559,10 @@ sub parse_commit_text {\n>  \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n>  \t\t\t\t$co{'committer_name'}  = $1;\n>  \t\t\t\t$co{'committer_email'} = $2;\n> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n> +\t\t\t\t\t$co{'committer_email'} = \"private\";\n> +\t\t\t\t\t$co{'committer'} = hide_mailaddr($co{'committer'});\n> +\t\t\t\t}\n> [...]\n>  \t\t\t} else {\n>  \t\t\t\t$co{'committer_name'} = $co{'committer'};\n>  \t\t\t}\n> @@ -3568,9 +3602,10 @@ sub parse_commit_text {\n>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>  \t}\n> -\t# remove added spaces\n> +\t# remove added spaces, redact e-mail addresses if applicable.\n>  \tforeach my $line (@commit_lines) {\n>  \t\t$line =~ s/^    //;\n> +\t\t$line = hide_mailaddr_if_private($line);\n>  \t}\n>  \t$co{'comment'} = \\@commit_lines;\n>  \n> @@ -8060,8 +8095,9 @@ sub git_commitdiff {\n>  \t\tclose $fd\n>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>  \t} elsif ($format eq 'patch') {\n> -\t\tlocal $/ = undef;\n> -\t\tprint <$fd>;\n> +\t\twhile (my $line = <$fd>) {\n> +\t\t\tprint hide_mailaddr_if_private($line);\n> +\t\t}\n\nUrm, have you tested this? How does a while loop over a <$fd> make sense\nwhen $/ is undef, the readline() operator will always return just one\nrecord, so having a while loop doesn't make sense.\n\nI'm not sure of the input here, but given that if you're expecting to\nreplace all e-mail addresses on all lines with this function that's not\nhow it'll work, the s/// doesn't have a /g, so it'll stop at the first\nreplacement.\n\n>  \t\tclose $fd\n>  \t\t\tor print \"Reading git-format-patch failed\\n\";\n>  \t}\n>\n> base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n\n"},{"id":"419881","messageId":"xmqqzgyw9wn3.fsf@gitster.g","threadId":"55358","inReplyTo":"pull.910.v3.git.1616347731514.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T18:42:08Z","receivedAt":"2021-03-21T18:43:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>\n> Gitweb extracts content from the Git log and makes it accessible\n> over HTTP. As a result, e-mail addresses found in commits are\n> exposed to web crawlers and they may not respect robots.txt.\n> This may result in unsolicited messages.\n> This is a feature for redacting e-mail addresses\n> from the generated HTML, etc. content.\n>\n> This feature does not prevent someone from downloading the\n> unredacted commit log, e.g., by cloning the repository, and\n> extracting information from it.\n> It aims to hinder the low-effort bulk collection of e-mail\n> addresses by web crawlers.\n\nLines from here ...\n\n>\n> Changes since v1:\n> - Turned off the feature by default.\n> - Removed duplicate code.\n> - Added note about Gitweb consumers receiving redacted logs.\n>\n> Changes since v2:\n> - The feature can be set on a per-project basis. ('override' => 1)\n\n... to here are to help reviewers on the mailing list while\niterations of the same change is being reviewed and polished.\n\nBut it is useless noise for those who only read \"git log\".  They\nsimply do not have access to earlier iterations.\n\nSuch \"changes between iterations\" comments needs to be written after\nthe three-dash lines.\n\n> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n> ---\n>     gitweb: Redacted e-mail addresses feature.\n>     \n>     Gitweb extracts content from the Git log and makes it accessible over\n>     HTTP. As a result, e-mail addresses found in commits are exposed to web\n>     crawlers. This may result in unsolicited messages. This is a feature for\n>     redacting e-mail addresses from the generated HTML content.\n>     \n>     This feature does not prevent someone from downloading the unredacted\n>     commit log and extracting information from it. It aims to hinder the\n>     low-effort bulk collection of e-mail addresses by web crawlers.\n>     \n>     Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/910\n>\n> Range-diff vs v2:\n>\n>  1:  74af11ca8bf2 ! 1:  930cdefe7ee0 gitweb: redacted e-mail addresses feature.\n>      @@ Commit message\n>           - Removed duplicate code.\n>           - Added note about Gitweb consumers receiving redacted logs.\n>       \n>      +    Changes since v2:\n>      +    - The feature can be set on a per-project basis. ('override' => 1)\n>      +\n>           Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>       \n>        ## Documentation/gitweb.conf.txt ##\n>      @@ gitweb/gitweb.perl: sub evaluate_uri {\n>       +\t# $feature{'email_privacy'}{'default'} = [1];\n>       +\t'email_privacy' => {\n>       +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>      -+\t\t'override' => 0,\n>      ++\t\t'override' => 1,\n>       +\t\t'default' => [0]},\n>        );\n>        \n>\n>\n>  Documentation/gitweb.conf.txt | 16 +++++++++++++\n>  gitweb/gitweb.perl            | 42 ++++++++++++++++++++++++++++++++---\n>  2 files changed, 55 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index 7963a79ba98b..b7af3240177d 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -896,6 +896,22 @@ same as of the snippet above:\n>  It is an error to specify a ref that does not pass \"git check-ref-format\"\n>  scrutiny. Duplicated values are filtered.\n>  \n> +email_privacy::\n> +    Redact e-mail addresses from the generated HTML, etc. content.\n> +    This hides e-mail addresses found in the commit log from web crawlers.\n> +    Disabled by default.\n> ++\n> +It is highly recommended to enable this feature unless web crawlers are\n> +hindered in some other way. Note that crawlers intent on harvesting e-mail\n> +addresses may disregard robots.txt.\n\nUp to this line is more-or-less OK, but with a few comments:\n\n - \"This hides ... from web crawlers\"?  Doesn't this hide it\n   indiscriminately, whether the requester is an interactive\n   end-user or a crawler?\n\n - The name of the configured feature, 'email_privacy'.  There is\n   another existing feature with underscore in its name\n   (remote_heads), but I think it is an odd-ball mistake we do not\n   want to imitate.  Instead, I think the \"extra branch refs\" may be\n   a good example to follow.  The feature name (written in Perl\n   code) is \"extra-branch-refs\" (downcased words with inter-word\n   dashes) and the corresponding configuration (I am not saying we\n   should add one to support this feature, if we do not have one\n   already) is \"gitweb.extraBranchRefs\" (camelCased words).\n\n - Do not exaggerate with words like \"highly\", but trust the\n   intelligence of your readers to make the right decision when they\n   understand the reason why this feature exists in the first place.\n\n - I do not think this entry should be added to the end of feature\n   list.  How about listing it just after 'avatar', which is also\n   about how author/committer identity is shown?\n\n> +---------------------------------------------------------------------------\n> +$feature{'email_privacy'}{'default'} = [1];\n> +---------------------------------------------------------------------------\n\nI do not think this should be here.  None of the boolean features\nlisted early in \"Features in `%feature`\" section like blame,\nsnapshot, grep, pickaxe, ... features tell readers how to enable a\nspecific feature like that.\n\nUnless the syntax to configure a feature is one-off oddball that is\ndifferent from all other features, we shouldn't clutter the\ndescription with an example like this.\n\nAt the beginning of the section \"Configuring Gitweb Features\" there\nis a general description and that should be sufficient (and if it is\nnot sufficient, it is OK to add an example to that section).\n\n> +Note that if Gitweb is not the final step in a workflow then subsequent\n> +steps may misbehave because of the redacted information they receive.\n\nIn other words, this will break crawlers that expect email addresses\nare there and want to use it for some good purpose.  But that is an\nnatural consequence of the \"feature\", and should be described as\nsuch when we said \"Redact e-mail addresses\", not as a \"by the way\"\nmention in a footnote.\n\n\t... after reading more, readers realize that the damage is\n\tfar worse in the current incarnation of the patch, but let's\n\tread on ...\n\n> @@ -3449,6 +3458,19 @@ sub parse_date {\n>  \treturn %date;\n>  }\n>  \n> +sub hide_mailaddr_if_private {\n> +\tmy $line = shift;\n> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n> +\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n\nI find that the second line is way too deeply indented.  Wouldn't\n\n> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n> +\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n\nbe enough?\n\n> +\treturn hide_mailaddr($line)\n> +}\n> +\n> +sub hide_mailaddr {\n> +\tmy $mailaddr = shift;\n> +\t$mailaddr =~ s/<([^>]*)>/<private>/;\n\ns/private/redacted/ perhaps?\n\n> @@ -3568,9 +3602,10 @@ sub parse_commit_text {\n>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>  \t}\n> -\t# remove added spaces\n> +\t# remove added spaces, redact e-mail addresses if applicable.\n>  \tforeach my $line (@commit_lines) {\n>  \t\t$line =~ s/^    //;\n> +\t\t$line = hide_mailaddr_if_private($line);\n\nA commit log that ends with\n\n\tThank you, A <a@example.com> and B <b@example.com>, for discovering\n\tthis bug and quickly proposing a solution.\n\nwould recact only the first one but not the other, I suspect.\n\nMuch more problematic is that I see too many hits from:\n\n    $ git log v2.30.0..v2.31.0 | grep ' <[^>@]*>'\n\nThat is, \"find a line in the commit log message that has a SP, open <bra,\nrun of characters that are not ket> or at@sign, closed with ket>\".  These\nare clearly not e-mail addresses.\n\nThis \"feature\" butchers a commit title:\n\n    mktag doc: say <hash> not <sha1>\n\nto\n\n    mktag doc: say <private> not <sha1>\n\ndoesn't it?\n\n> @@ -8060,8 +8095,9 @@ sub git_commitdiff {\n>  \t\tclose $fd\n>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>  \t} elsif ($format eq 'patch') {\n> -\t\tlocal $/ = undef;\n> -\t\tprint <$fd>;\n> +\t\twhile (my $line = <$fd>) {\n> +\t\t\tprint hide_mailaddr_if_private($line);\n> +\t\t}\n\nAnd this is even worse.\n\n $ git log -p --no-merges v2.30.0..v2.31.0 | grep ' <[^>@]*>' | wc -l\n\ngives me ~4700 hits.  Because the patch text as well as the log\nmessage is munged, the \"feature\" makes the output utterly unreliable\nas an input to \"git am\" by munging too much.  Interesting examples\nare like these:\n\n        -\tfor (i = 0; i < pairs->nr; ++i) {\n                'git <command> [<revision>...] -- [<file>...]'\n        +\tfor (i = split; i < geometry->pack_nr; i++) {\n\nNow, I am *NOT* saying that you should tighten the e-mail address\nsyntax detection and keep munging the patch text.  The lines that\nbegin with minus and SP in a patch must match the preimage text,\nso munging out existing e-mail addresses from them will make the\npatch fail to find the part that needs to be modified.  And\nreplacing an e-mail address on a line that begins with plus would\nredact it from the source---if they wrote an address, they must have\nmeant it to be available to those who consume the source, so I doubt\nthe wisdom of munging the patch part at all.\n\nI may be sympathetic to the cause of the patch, but, I do not agree\nwith its execution in this iteration of the patch.\n\nThanks.\n\n"},{"id":"419882","messageId":"xmqqv99k9wc9.fsf@gitster.g","threadId":"55358","inReplyTo":"87r1k8qs73.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T18:48:38Z","receivedAt":"2021-03-21T18:49:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> But:\n>\n>>  sub parse_tag {\n>>  \tmy $tag_id = shift;\n>>  \tmy %tag;\n>> @@ -3471,6 +3493,10 @@ sub parse_tag {\n>>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$tag{'author_name'}  = $1;\n>>  \t\t\t\t$tag{'author_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n>> +\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n>> +\t\t\t\t}\n>\n> This code seems quite awkward, we've already done the regex match, but\n> this code:\n>\n>> [snip]\n>> +sub hide_mailaddr_if_private {\n>> +\tmy $line = shift;\n>> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n>> +\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n>> +\treturn hide_mailaddr($line)\n>> +}\n>> +\n>> +sub hide_mailaddr {\n>> +\tmy $mailaddr = shift;\n>> +\t$mailaddr =~ s/<([^>]*)>/<private>/;\n>> +\treturn $mailaddr;\n>> +}\n>\n> Is going to do it again incrementally, and then just act on a\n> search-replacement if we've got the feature enabled.\n\nI think you misread the patch the same way as I did initially.  What\nis called in parse_tag is not the \"if-private\" version---the caller\nknows that $tag{'author'} MUST BE an address so unconditionally redact\nwhen the feature is set by bypassing the _if_private variant.\n\nHaving said that, I do think\n\n>     sub maybe_hide_email {\n>         my ($email, $ref) = shift;\n>         return $email unless gitweb_check_feature('email_privacy');\n>         $$ref = \"private\" if $ref;\n>         return hide_email($email);\n>     }\n\nthis helper is how you would simplify these numerous callers.\n\n>>  \t} elsif ($format eq 'patch') {\n>> -\t\tlocal $/ = undef;\n>> -\t\tprint <$fd>;\n>> +\t\twhile (my $line = <$fd>) {\n>> +\t\t\tprint hide_mailaddr_if_private($line);\n>> +\t\t}\n>\n> Urm, have you tested this? How does a while loop over a <$fd> make sense\n> when $/ is undef, the readline() operator will always return just one\n> record, so having a while loop doesn't make sense.\n>\n> I'm not sure of the input here, but given that if you're expecting to\n> replace all e-mail addresses on all lines with this function that's not\n> how it'll work, the s/// doesn't have a /g, so it'll stop at the first\n> replacement.\n>\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-format-patch failed\\n\";\n>>  \t}\n\nAll true.  This one is reading from \"format-patch --stdout\" output,\nbut for the reasons I mentioned in my review, I do not think it\nshould touch the 'patch' output at all.\n\n"},{"id":"419883","messageId":"xmqqo8fc9vwp.fsf@gitster.g","threadId":"55358","inReplyTo":"xmqqzgyw9wn3.fsf@gitster.g","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T18:57:58Z","receivedAt":"2021-03-21T18:58:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> @@ -8060,8 +8095,9 @@ sub git_commitdiff {\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>>  \t} elsif ($format eq 'patch') {\n>> -\t\tlocal $/ = undef;\n>> -\t\tprint <$fd>;\n>> +\t\twhile (my $line = <$fd>) {\n>> +\t\t\tprint hide_mailaddr_if_private($line);\n>> +\t\t}\n>\n> And this is even worse.\n\nAnd also Ævar is right.  The original just has a format-patch output\nin $_ and prints it to the output stream in one go.  It won't make\nany sense to attempt reading from that stream, which the original\ncode used to write to, at all.\n\nIn any case, I've already said that I do not think it is a good idea\nto touch the 'patch' codepath at all, so this code won't do any\ndamage in the end by reading from the output pipe connected to\nend-user's browser ;-)\n\nThanks.\n"},{"id":"419884","messageId":"xmqqeeg89vjg.fsf@gitster.g","threadId":"55358","inReplyTo":"xmqqo8fc9vwp.fsf@gitster.g","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T19:05:55Z","receivedAt":"2021-03-21T19:07:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"> And also Ævar is right.  The original just has a format-patch output\n> in $_ and prints it to the output stream in one go.\n\nSorry but I misread the patch.  By dropping the $/ = undef, it wants\nto read from $fd (which is \"format-patch --stdout\" output) one line\nat a time.  So unless a line has two addresses, this would \"work\" as\nintended.\n\nAgain, as I said, I do not agree with the definition of \"work\" here,\nthough ;-).\n"},{"id":"419885","messageId":"35ffea669f679fe3605f3f14fc50c945.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"87r1k8qs73.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-21T19:56:09Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":">\n> On Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:\n>\n>> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers and they may not respect robots.txt.\n>> This may result in unsolicited messages.\n>> This is a feature for redacting e-mail addresses\n>> from the generated HTML, etc. content.\n>>\n>> This feature does not prevent someone from downloading the\n>> unredacted commit log, e.g., by cloning the repository, and\n>> extracting information from it.\n>> It aims to hinder the low-effort bulk collection of e-mail\n>> addresses by web crawlers.\n>>\n>> Changes since v1:\n>> - Turned off the feature by default.\n>> - Removed duplicate code.\n>> - Added note about Gitweb consumers receiving redacted logs.\n>>\n>> Changes since v2:\n>> - The feature can be set on a per-project basis. ('override' => 1)\n>>\n>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>> ---\n>>     gitweb: Redacted e-mail addresses feature.\n>>\n>>     Gitweb extracts content from the Git log and makes it accessible\n>> over\n>>     HTTP. As a result, e-mail addresses found in commits are exposed to\n>> web\n>>     crawlers. This may result in unsolicited messages. This is a feature\n>> for\n>>     redacting e-mail addresses from the generated HTML content.\n>>\n>>     This feature does not prevent someone from downloading the\n>> unredacted\n>>     commit log and extracting information from it. It aims to hinder the\n>>     low-effort bulk collection of e-mail addresses by web crawlers.\n>>\n>>     Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n>>\n>> Published-As:\n>> https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n>> pr-910/kontaxis/kontaxis/email_privacy-v3\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/910\n>>\n>> Range-diff vs v2:\n>>\n>>  1:  74af11ca8bf2 ! 1:  930cdefe7ee0 gitweb: redacted e-mail addresses\n>> feature.\n>>      @@ Commit message\n>>           - Removed duplicate code.\n>>           - Added note about Gitweb consumers receiving redacted logs.\n>>\n>>      +    Changes since v2:\n>>      +    - The feature can be set on a per-project basis. ('override'\n>> => 1)\n>>      +\n>>           Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>>        ## Documentation/gitweb.conf.txt ##\n>>      @@ gitweb/gitweb.perl: sub evaluate_uri {\n>>       +\t# $feature{'email_privacy'}{'default'} = [1];\n>>       +\t'email_privacy' => {\n>>       +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>>      -+\t\t'override' => 0,\n>>      ++\t\t'override' => 1,\n>>       +\t\t'default' => [0]},\n>>        );\n>>\n>>\n>>\n>>  Documentation/gitweb.conf.txt | 16 +++++++++++++\n>>  gitweb/gitweb.perl            | 42 ++++++++++++++++++++++++++++++++---\n>>  2 files changed, 55 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/Documentation/gitweb.conf.txt\n>> b/Documentation/gitweb.conf.txt\n>> index 7963a79ba98b..b7af3240177d 100644\n>> --- a/Documentation/gitweb.conf.txt\n>> +++ b/Documentation/gitweb.conf.txt\n>> @@ -896,6 +896,22 @@ same as of the snippet above:\n>>  It is an error to specify a ref that does not pass \"git\n>> check-ref-format\"\n>>  scrutiny. Duplicated values are filtered.\n>>\n>> +email_privacy::\n>> +    Redact e-mail addresses from the generated HTML, etc. content.\n>> +    This hides e-mail addresses found in the commit log from web\n>> crawlers.\n>> +    Disabled by default.\n>> ++\n>> +It is highly recommended to enable this feature unless web crawlers are\n>> +hindered in some other way. Note that crawlers intent on harvesting\n>> e-mail\n>> +addresses may disregard robots.txt. You can enable this feature like\n>> so:\n>> ++\n>> +---------------------------------------------------------------------------\n>> +$feature{'email_privacy'}{'default'} = [1];\n>> +---------------------------------------------------------------------------\n>> ++\n>> +Note that if Gitweb is not the final step in a workflow then subsequent\n>> +steps may misbehave because of the redacted information they receive.\n>> +\n>>\n>>  EXAMPLES\n>>  --------\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 0959a782eccb..174cc566d97d 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -569,6 +569,15 @@ sub evaluate_uri {\n>>  \t\t'sub' => \\&feature_extra_branch_refs,\n>>  \t\t'override' => 0,\n>>  \t\t'default' => []},\n>> +\n>> +\t# Redact e-mail addresses.\n>> +\n>> +\t# To enable system wide have in $GITWEB_CONFIG\n>> +\t# $feature{'email_privacy'}{'default'} = [1];\n>> +\t'email_privacy' => {\n>> +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>> +\t\t'override' => 1,\n>> +\t\t'default' => [0]},\n>>  );\n>>\n>>  sub gitweb_get_feature {\n>> @@ -3449,6 +3458,19 @@ sub parse_date {\n>>  \treturn %date;\n>>  }\n>>\n>> [snip]\n>\n> So in the v1 feedback I suggested:\n>\n> BEGIN QUOTE\n>     sub maybe_hide_email {\n>         my $email = shift;\n>         return $email unless gitweb_check_feature('email_privacy');\n>         return hide_email($email);\n>     }\n>\n> then:\n>\n>     $tag{author_email} = maybe_hide_email($2);\n> END QUOTE\n>\n> But:\n>\n>>  sub parse_tag {\n>>  \tmy $tag_id = shift;\n>>  \tmy %tag;\n>> @@ -3471,6 +3493,10 @@ sub parse_tag {\n>>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$tag{'author_name'}  = $1;\n>>  \t\t\t\t$tag{'author_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n>> +\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n>> +\t\t\t\t}\n>\n> This code seems quite awkward, we've already done the regex match, but\n> this code:\n>\n>> [snip]\n>> +sub hide_mailaddr_if_private {\n>> +\tmy $line = shift;\n>> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n>> +\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n>> +\treturn hide_mailaddr($line)\n>> +}\n>> +\n>> +sub hide_mailaddr {\n>> +\tmy $mailaddr = shift;\n>> +\t$mailaddr =~ s/<([^>]*)>/<private>/;\n>> +\treturn $mailaddr;\n>> +}\n>\n> Is going to do it again incrementally, and then just act on a\n> search-replacement if we've got the feature enabled.\n>\nI think hide_mailaddr_if_private (which checks the feature flag\nand looks for the pattern) is only called when we haven't done so\nalready. In every other case we call hide_mailaddr directly.\nSo there's no duplication of checks.\n\n> It seems much simpler to just turn your:\n>\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$tag{'author_email'} = \"private\";\n>> +\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n>> +\t\t\t\t}\n>\n> Into:\n>\n>     $tag{'author'} = maybe_hide_mailaddr($tag{author},\n> \\$tag{author_email});\n>\n> Using:\n>\n>     sub maybe_hide_email {\n>         my ($email, $ref) = shift;\n>         return $email unless gitweb_check_feature('email_privacy');\n>         $$ref = \"private\" if $ref;\n>         return hide_email($email);\n>     }\n>\n> Which also works for the case where you don't have a \"private\" hash key\n> to assign to. But maybe it overcomplicates things...\n>\n>>  \t\t\t} else {\n>>  \t\t\t\t$tag{'author_name'} = $tag{'author'};\n>>  \t\t\t}\n>> @@ -3519,6 +3545,10 @@ sub parse_commit_text {\n>>  \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$co{'author_name'}  = $1;\n>>  \t\t\t\t$co{'author_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$co{'author_email'} = \"private\";\n>> +\t\t\t\t\t$co{'author'} = hide_mailaddr($co{'author'});\n>> +\t\t\t\t}\n>>  \t\t\t} else {\n>>  \t\t\t\t$co{'author_name'} = $co{'author'};\n>>  \t\t\t}\n>> @@ -3529,6 +3559,10 @@ sub parse_commit_text {\n>>  \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n>>  \t\t\t\t$co{'committer_name'}  = $1;\n>>  \t\t\t\t$co{'committer_email'} = $2;\n>> +\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n>> +\t\t\t\t\t$co{'committer_email'} = \"private\";\n>> +\t\t\t\t\t$co{'committer'} = hide_mailaddr($co{'committer'});\n>> +\t\t\t\t}\n>> [...]\n>>  \t\t\t} else {\n>>  \t\t\t\t$co{'committer_name'} = $co{'committer'};\n>>  \t\t\t}\n>> @@ -3568,9 +3602,10 @@ sub parse_commit_text {\n>>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>>  \t}\n>> -\t# remove added spaces\n>> +\t# remove added spaces, redact e-mail addresses if applicable.\n>>  \tforeach my $line (@commit_lines) {\n>>  \t\t$line =~ s/^    //;\n>> +\t\t$line = hide_mailaddr_if_private($line);\n>>  \t}\n>>  \t$co{'comment'} = \\@commit_lines;\n>>\n>> @@ -8060,8 +8095,9 @@ sub git_commitdiff {\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>>  \t} elsif ($format eq 'patch') {\n>> -\t\tlocal $/ = undef;\n>> -\t\tprint <$fd>;\n>> +\t\twhile (my $line = <$fd>) {\n>> +\t\t\tprint hide_mailaddr_if_private($line);\n>> +\t\t}\n>\n> Urm, have you tested this? How does a while loop over a <$fd> make sense\n> when $/ is undef, the readline() operator will always return just one\n> record, so having a while loop doesn't make sense.\n>\n$/ is not undef, the opposite, it's the default value which results\nin reading from <$fd> one line at a time. (where line is a sequence\nterminated by CRLF or LF)\n\n> I'm not sure of the input here, but given that if you're expecting to\n> replace all e-mail addresses on all lines with this function that's not\n> how it'll work, the s/// doesn't have a /g, so it'll stop at the first\n> replacement.\n>\nSo we are reading from <$fd> one line at a time and looking for the\nfirst \"expr1 <expr2>\" occurrence in each line. This successfully redacts\nthe author, committer, signed-off-by lines.\n\nNot doing a global replacement per line is intentional so as to avoid\nfalse positives.\nSo far I haven't noticed missing any addresses that should have been\nredacted.\n\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-format-patch failed\\n\";\n>>  \t}\n>>\n>> base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n>\n>\n\n\n"},{"id":"419886","messageId":"dfb1fe6cdf78788250009abe8e1489c8.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"xmqqzgyw9wn3.fsf@gitster.g","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-21T20:08:34Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> \"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n>> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers and they may not respect robots.txt.\n>> This may result in unsolicited messages.\n>> This is a feature for redacting e-mail addresses\n>> from the generated HTML, etc. content.\n>>\n>> This feature does not prevent someone from downloading the\n>> unredacted commit log, e.g., by cloning the repository, and\n>> extracting information from it.\n>> It aims to hinder the low-effort bulk collection of e-mail\n>> addresses by web crawlers.\n>\n> Lines from here ...\n>\n>>\n>> Changes since v1:\n>> - Turned off the feature by default.\n>> - Removed duplicate code.\n>> - Added note about Gitweb consumers receiving redacted logs.\n>>\n>> Changes since v2:\n>> - The feature can be set on a per-project basis. ('override' => 1)\n>\n> ... to here are to help reviewers on the mailing list while\n> iterations of the same change is being reviewed and polished.\n>\n> But it is useless noise for those who only read \"git log\".  They\n> simply do not have access to earlier iterations.\n>\n> Such \"changes between iterations\" comments needs to be written after\n> the three-dash lines.\n>\n>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>> ---\n>>     gitweb: Redacted e-mail addresses feature.\n>>\n>>     Gitweb extracts content from the Git log and makes it accessible\n>> over\n>>     HTTP. As a result, e-mail addresses found in commits are exposed to\n>> web\n>>     crawlers. This may result in unsolicited messages. This is a feature\n>> for\n>>     redacting e-mail addresses from the generated HTML content.\n>>\n>>     This feature does not prevent someone from downloading the\n>> unredacted\n>>     commit log and extracting information from it. It aims to hinder the\n>>     low-effort bulk collection of e-mail addresses by web crawlers.\n>>\n>>     Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n>>\n>> Published-As:\n>> https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n>> pr-910/kontaxis/kontaxis/email_privacy-v3\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/910\n>>\n>> Range-diff vs v2:\n>>\n>>  1:  74af11ca8bf2 ! 1:  930cdefe7ee0 gitweb: redacted e-mail addresses\n>> feature.\n>>      @@ Commit message\n>>           - Removed duplicate code.\n>>           - Added note about Gitweb consumers receiving redacted logs.\n>>\n>>      +    Changes since v2:\n>>      +    - The feature can be set on a per-project basis. ('override'\n>> => 1)\n>>      +\n>>           Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>>        ## Documentation/gitweb.conf.txt ##\n>>      @@ gitweb/gitweb.perl: sub evaluate_uri {\n>>       +\t# $feature{'email_privacy'}{'default'} = [1];\n>>       +\t'email_privacy' => {\n>>       +\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n>>      -+\t\t'override' => 0,\n>>      ++\t\t'override' => 1,\n>>       +\t\t'default' => [0]},\n>>        );\n>>\n>>\n>>\n>>  Documentation/gitweb.conf.txt | 16 +++++++++++++\n>>  gitweb/gitweb.perl            | 42 ++++++++++++++++++++++++++++++++---\n>>  2 files changed, 55 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/Documentation/gitweb.conf.txt\n>> b/Documentation/gitweb.conf.txt\n>> index 7963a79ba98b..b7af3240177d 100644\n>> --- a/Documentation/gitweb.conf.txt\n>> +++ b/Documentation/gitweb.conf.txt\n>> @@ -896,6 +896,22 @@ same as of the snippet above:\n>>  It is an error to specify a ref that does not pass \"git\n>> check-ref-format\"\n>>  scrutiny. Duplicated values are filtered.\n>>\n>> +email_privacy::\n>> +    Redact e-mail addresses from the generated HTML, etc. content.\n>> +    This hides e-mail addresses found in the commit log from web\n>> crawlers.\n>> +    Disabled by default.\n>> ++\n>> +It is highly recommended to enable this feature unless web crawlers are\n>> +hindered in some other way. Note that crawlers intent on harvesting\n>> e-mail\n>> +addresses may disregard robots.txt.\n>\n> Up to this line is more-or-less OK, but with a few comments:\n>\n>  - \"This hides ... from web crawlers\"?  Doesn't this hide it\n>    indiscriminately, whether the requester is an interactive\n>    end-user or a crawler?\n>\nCorrect. It doesn't say \"it only hides from\".\nI can see how it may be confusing. I'll rephrase.\n\n>  - The name of the configured feature, 'email_privacy'.  There is\n>    another existing feature with underscore in its name\n>    (remote_heads), but I think it is an odd-ball mistake we do not\n>    want to imitate.  Instead, I think the \"extra branch refs\" may be\n>    a good example to follow.  The feature name (written in Perl\n>    code) is \"extra-branch-refs\" (downcased words with inter-word\n>    dashes) and the corresponding configuration (I am not saying we\n>    should add one to support this feature, if we do not have one\n>    already) is \"gitweb.extraBranchRefs\" (camelCased words).\n>\nI'll rename the feature.\n\n>  - Do not exaggerate with words like \"highly\", but trust the\n>    intelligence of your readers to make the right decision when they\n>    understand the reason why this feature exists in the first place.\n>\nWill remove.\n\n>  - I do not think this entry should be added to the end of feature\n>    list.  How about listing it just after 'avatar', which is also\n>    about how author/committer identity is shown?\n>\nWill do.\n\n>> +---------------------------------------------------------------------------\n>> +$feature{'email_privacy'}{'default'} = [1];\n>> +---------------------------------------------------------------------------\n>\n> I do not think this should be here.  None of the boolean features\n> listed early in \"Features in `%feature`\" section like blame,\n> snapshot, grep, pickaxe, ... features tell readers how to enable a\n> specific feature like that.\n>\n> Unless the syntax to configure a feature is one-off oddball that is\n> different from all other features, we shouldn't clutter the\n> description with an example like this.\n>\n> At the beginning of the section \"Configuring Gitweb Features\" there\n> is a general description and that should be sufficient (and if it is\n> not sufficient, it is OK to add an example to that section).\n>\nWill remove.\n\n>> +Note that if Gitweb is not the final step in a workflow then subsequent\n>> +steps may misbehave because of the redacted information they receive.\n>\n> In other words, this will break crawlers that expect email addresses\n> are there and want to use it for some good purpose.  But that is an\n> natural consequence of the \"feature\", and should be described as\n> such when we said \"Redact e-mail addresses\", not as a \"by the way\"\n> mention in a footnote.\n>\n> \t... after reading more, readers realize that the damage is\n> \tfar worse in the current incarnation of the patch, but let's\n> \tread on ...\n>\nWill move this out of the footnote.\n\n>> @@ -3449,6 +3458,19 @@ sub parse_date {\n>>  \treturn %date;\n>>  }\n>>\n>> +sub hide_mailaddr_if_private {\n>> +\tmy $line = shift;\n>> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n>> +\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n>\n> I find that the second line is way too deeply indented.  Wouldn't\n>\n>> +\treturn $line unless (gitweb_check_feature('email_privacy') &&\n>> +\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n>\n> be enough?\n>\nWill adjust.\n\n>> +\treturn hide_mailaddr($line)\n>> +}\n>> +\n>> +sub hide_mailaddr {\n>> +\tmy $mailaddr = shift;\n>> +\t$mailaddr =~ s/<([^>]*)>/<private>/;\n>\n> s/private/redacted/ perhaps?\n>\nWill adjust.\n\n>> @@ -3568,9 +3602,10 @@ sub parse_commit_text {\n>>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>>  \t}\n>> -\t# remove added spaces\n>> +\t# remove added spaces, redact e-mail addresses if applicable.\n>>  \tforeach my $line (@commit_lines) {\n>>  \t\t$line =~ s/^    //;\n>> +\t\t$line = hide_mailaddr_if_private($line);\n>\n> A commit log that ends with\n>\n> \tThank you, A <a@example.com> and B <b@example.com>, for discovering\n> \tthis bug and quickly proposing a solution.\n>\n> would recact only the first one but not the other, I suspect.\n>\n> Much more problematic is that I see too many hits from:\n>\n>     $ git log v2.30.0..v2.31.0 | grep ' <[^>@]*>'\n>\n> That is, \"find a line in the commit log message that has a SP, open <bra,\n> run of characters that are not ket> or at@sign, closed with ket>\".  These\n> are clearly not e-mail addresses.\n>\n> This \"feature\" butchers a commit title:\n>\n>     mktag doc: say <hash> not <sha1>\n>\n> to\n>\n>     mktag doc: say <private> not <sha1>\n>\n> doesn't it?\n>\nTrue. Thanks for pointing these out.\nSounds like the pattern we're looking for needs to be more specific.\n\n>> @@ -8060,8 +8095,9 @@ sub git_commitdiff {\n>>  \t\tclose $fd\n>>  \t\t\tor print \"Reading git-diff-tree failed\\n\";\n>>  \t} elsif ($format eq 'patch') {\n>> -\t\tlocal $/ = undef;\n>> -\t\tprint <$fd>;\n>> +\t\twhile (my $line = <$fd>) {\n>> +\t\t\tprint hide_mailaddr_if_private($line);\n>> +\t\t}\n>\n> And this is even worse.\n>\n>  $ git log -p --no-merges v2.30.0..v2.31.0 | grep ' <[^>@]*>' | wc -l\n>\n> gives me ~4700 hits.  Because the patch text as well as the log\n> message is munged, the \"feature\" makes the output utterly unreliable\n> as an input to \"git am\" by munging too much.  Interesting examples\n> are like these:\n>\n>         -\tfor (i = 0; i < pairs->nr; ++i) {\n>                 'git <command> [<revision>...] -- [<file>...]'\n>         +\tfor (i = split; i < geometry->pack_nr; i++) {\n>\n> Now, I am *NOT* saying that you should tighten the e-mail address\n> syntax detection and keep munging the patch text.  The lines that\n> begin with minus and SP in a patch must match the preimage text,\n> so munging out existing e-mail addresses from them will make the\n> patch fail to find the part that needs to be modified.  And\n> replacing an e-mail address on a line that begins with plus would\n> redact it from the source---if they wrote an address, they must have\n> meant it to be available to those who consume the source, so I doubt\n> the wisdom of munging the patch part at all.\n>\n> I may be sympathetic to the cause of the patch, but, I do not agree\n> with its execution in this iteration of the patch.\n>\nI see your point.\n\nIt seems hiding e-mail addresses should be limited to the commit message,\ni.e., stop at the \"---\" line.\n\n> Thanks.\n>\n>\n\n\n"},{"id":"419891","messageId":"xmqqmtuw884f.fsf@gitster.g","threadId":"55358","inReplyTo":"dfb1fe6cdf78788250009abe8e1489c8.squirrel@mail.kodaksys.org","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-21T22:17:04Z","receivedAt":"2021-03-21T22:18:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n\n>> ... so I doubt\n>> the wisdom of munging the patch part at all.\n>>\n>> I may be sympathetic to the cause of the patch, but, I do not agree\n>> with its execution in this iteration of the patch.\n>>\n> I see your point.\n>\n> It seems hiding e-mail addresses should be limited to the commit message,\n> i.e., stop at the \"---\" line.\n\nI doubt it makes sense to redact anything in the 'patch' view at\nall, actually.  What kind of URL does the crawler need to formulate\nand what pieces of information (like commit object names or branch\nnames) does it need to fill in the URL to get a series of patches\nout of gitweb?  As long as it takes more effort than running \"git\nclone\" against the repository, the crawler would not have much\nincentive to crawl and harvest addresses from the 'patch' pages, and\neven in the log message part, the downsides of butchering the\npayload would outweigh the \"privacy benefit\", I would have to say.\n\nQuite honestly, if a site claims to offer a 'patch' download UI but\nreturns corrupt data back, I would say it is much worse than not\noffering the service at all.  Perhaps disabling the 'patch' feature\nin repositories that enable 'privacy' feature may be a much better\napproach.\n"},{"id":"419897","messageId":"14d3cda766b0212ca9d8204eba35aa4d.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"xmqqmtuw884f.fsf@gitster.g","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-21T23:24:33Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> \"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n>\n>>> ... so I doubt\n>>> the wisdom of munging the patch part at all.\n>>>\n>>> I may be sympathetic to the cause of the patch, but, I do not agree\n>>> with its execution in this iteration of the patch.\n>>>\n>> I see your point.\n>>\n>> It seems hiding e-mail addresses should be limited to the commit\n>> message,\n>> i.e., stop at the \"---\" line.\n>\n> I doubt it makes sense to redact anything in the 'patch' view at\n> all, actually.  What kind of URL does the crawler need to formulate\n> and what pieces of information (like commit object names or branch\n> names) does it need to fill in the URL to get a series of patches\n> out of gitweb?  As long as it takes more effort than running \"git\n> clone\" against the repository, the crawler would not have much\n> incentive to crawl and harvest addresses from the 'patch' pages, and\n> even in the log message part, the downsides of butchering the\n> payload would outweigh the \"privacy benefit\", I would have to say.\n>\nNo effort at all I would say.\nE..g, somehow the web crawler gets to git.kernel.org.\nIt then follows every link, eventually arriving at a commitdiff page.\nIt then follows every link, which includes the URL for the patch output.\nSee how \"wget --mirror\" behaves for instance.\n\nJust to clarify, my goal is not to stop someone who wants to extract\ne-mail address from git.kernel.org specifically.\nThey can just \"git clone\" the repositories and grep through the logs.\nMy goal is to stop generic crawlers (pretty much \"wget --mirror | grep\"\nscripts) from making their way to the logs.\n\n> Quite honestly, if a site claims to offer a 'patch' download UI but\n> returns corrupt data back, I would say it is much worse than not\n> offering the service at all.  Perhaps disabling the 'patch' feature\n> in repositories that enable 'privacy' feature may be a much better\n> approach.\n>\nGood point. I think I'll try that.\n\n\n"},{"id":"419903","messageId":"xmqqft0n95m9.fsf@gitster.g","threadId":"55358","inReplyTo":"14d3cda766b0212ca9d8204eba35aa4d.squirrel@mail.kodaksys.org","subject":"Re: [PATCH v3] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-22T04:25:50Z","receivedAt":"2021-03-22T04:26:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n\n>> Quite honestly, if a site claims to offer a 'patch' download UI but\n>> returns corrupt data back, I would say it is much worse than not\n>> offering the service at all.  Perhaps disabling the 'patch' feature\n>> in repositories that enable 'privacy' feature may be a much better\n>> approach.\n>>\n> Good point. I think I'll try that.\n\nAlternatively, we could add 'captcha' to stop crawlers, perhaps?\n"},{"id":"419909","messageId":"pull.910.v4.git.1616396267010.gitgitgadget@gmail.com","threadId":"55358","inReplyTo":"pull.910.v3.git.1616347731514.gitgitgadget@gmail.com","subject":"[PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-22T06:57:46Z","receivedAt":"2021-03-22T06:58:45Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"From: Georgios Kontaxis <geko1702+commits@99rst.org>\n\nGitweb extracts content from the Git log and makes it accessible\nover HTTP. As a result, e-mail addresses found in commits are\nexposed to web crawlers and they may not respect robots.txt.\nThis may result in unsolicited messages.\nThis is a feature for redacting e-mail addresses\nfrom the generated HTML, etc. content.\n\nThis feature does not prevent someone from downloading the\nunredacted commit log, e.g., by cloning the repository, and\nextracting information from it.\nIt aims to hinder the low-effort bulk collection of e-mail\naddresses by web crawlers.\n\nSigned-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n---\n    gitweb: redacted e-mail addresses feature.\n    \n    Gitweb extracts content from the Git log and makes it accessible over\n    HTTP. As a result, e-mail addresses found in commits are exposed to web\n    crawlers and they may not respect robots.txt. This may result in\n    unsolicited messages. This is a feature for redacting e-mail addresses\n    from the generated HTML, etc. content.\n    \n    This feature does not prevent someone from downloading the unredacted\n    commit log, e.g., by cloning the repository, and extracting information\n    from it. It aims to hinder the low-effort bulk collection of e-mail\n    addresses by web crawlers.\n    \n    Changes since v1:\n    \n     * Turned off the feature by default.\n     * Removed duplicate code.\n     * Added note about Gitweb consumers receiving redacted logs.\n    \n    Changes since v2:\n    \n     * The feature can be set on a per-project basis. ('override' => 1)\n    \n    Changes since v3:\n    \n     * Renamed feature to \"email-privacy\" and improved documentation.\n     * Removed UI elements for git-format-patch since it won't be redacted.\n     * Simplified calls to the address redaction logic.\n     * Mail::Address is now used to reduce false-positive redactions.\n    \n    Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/910\n\nRange-diff vs v3:\n\n 1:  930cdefe7ee0 ! 1:  03a3f41c37ef gitweb: redacted e-mail addresses feature.\n     @@ Commit message\n          It aims to hinder the low-effort bulk collection of e-mail\n          addresses by web crawlers.\n      \n     -    Changes since v1:\n     -    - Turned off the feature by default.\n     -    - Removed duplicate code.\n     -    - Added note about Gitweb consumers receiving redacted logs.\n     -\n     -    Changes since v2:\n     -    - The feature can be set on a per-project basis. ('override' => 1)\n     -\n          Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n      \n       ## Documentation/gitweb.conf.txt ##\n     -@@ Documentation/gitweb.conf.txt: same as of the snippet above:\n     - It is an error to specify a ref that does not pass \"git check-ref-format\"\n     - scrutiny. Duplicated values are filtered.\n     +@@ Documentation/gitweb.conf.txt: default font sizes or lineheights are changed (e.g. via adding extra\n     + CSS stylesheet in `@stylesheets`), it may be appropriate to change\n     + these values.\n       \n     -+email_privacy::\n     -+    Redact e-mail addresses from the generated HTML, etc. content.\n     -+    This hides e-mail addresses found in the commit log from web crawlers.\n     -+    Disabled by default.\n     -++\n     -+It is highly recommended to enable this feature unless web crawlers are\n     -+hindered in some other way. Note that crawlers intent on harvesting e-mail\n     -+addresses may disregard robots.txt. You can enable this feature like so:\n     -++\n     -+---------------------------------------------------------------------------\n     -+$feature{'email_privacy'}{'default'} = [1];\n     -+---------------------------------------------------------------------------\n     -++\n     -+Note that if Gitweb is not the final step in a workflow then subsequent\n     -+steps may misbehave because of the redacted information they receive.\n     ++email-privacy::\n     ++\tRedact e-mail addresses from the generated HTML, etc. content.\n     ++\tThis hides e-mail addresses found in the commit log from HTTP clients.\n     ++\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n     ++\tSuch crawlers may not respect robots.txt.\n     ++\tNote that users and user tools also see the addresses redacted.\n     ++\tIf Gitweb is not the final step in a workflow then subsequent steps\n     ++\tmay misbehave because of the redacted information they receive.\n     ++\tDisabled by default.\n      +\n     - \n     - EXAMPLES\n     - --------\n     + highlight::\n     + \tServer-side syntax highlight support in \"blob\" view.  It requires\n     + \t`$highlight_bin` program to be available (see the description of\n      \n       ## gitweb/gitweb.perl ##\n     +@@\n     + use File::Basename qw(basename);\n     + use Time::HiRes qw(gettimeofday tv_interval);\n     + use Digest::MD5 qw(md5_hex);\n     ++use Git::LoadCPAN::Mail::Address;\n     + \n     + binmode STDOUT, ':utf8';\n     + \n      @@ gitweb/gitweb.perl: sub evaluate_uri {\n       \t\t'sub' => \\&feature_extra_branch_refs,\n       \t\t'override' => 0,\n     @@ gitweb/gitweb.perl: sub evaluate_uri {\n      +\t# Redact e-mail addresses.\n      +\n      +\t# To enable system wide have in $GITWEB_CONFIG\n     -+\t# $feature{'email_privacy'}{'default'} = [1];\n     -+\t'email_privacy' => {\n     -+\t\t'sub' => sub { feature_bool('email_privacy', @_) },\n     ++\t# $feature{'email-privacy'}{'default'} = [1];\n     ++\t'email-privacy' => {\n     ++\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n      +\t\t'override' => 1,\n      +\t\t'default' => [0]},\n       );\n     @@ gitweb/gitweb.perl: sub parse_date {\n       \treturn %date;\n       }\n       \n     -+sub hide_mailaddr_if_private {\n     -+\tmy $line = shift;\n     -+\treturn $line unless (gitweb_check_feature('email_privacy') &&\n     -+\t\t\t\t\t\t$line =~ m/^([^<]+) <([^>]*)>/);\n     -+\treturn hide_mailaddr($line)\n     ++sub is_mailaddr {\n     ++\tmy @addrs = Mail::Address->parse(shift);\n     ++\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n     ++\t\treturn 0;\n     ++\t}\n     ++\treturn 1;\n      +}\n      +\n     -+sub hide_mailaddr {\n     -+\tmy $mailaddr = shift;\n     -+\t$mailaddr =~ s/<([^>]*)>/<private>/;\n     -+\treturn $mailaddr;\n     ++sub hide_mailaddrs_if_private {\n     ++\tmy $line = shift;\n     ++\treturn $line unless gitweb_check_feature('email-privacy');\n     ++\twhile ($line =~ m/(<[^>]+>)/g) {\n     ++\t\tmy $match = $1;\n     ++\t\tif (!is_mailaddr($match)) {\n     ++\t\t\tnext;\n     ++\t\t}\n     ++\t\tmy $offset = pos $line;\n     ++\t\tmy $head = substr $line, 0, $offset - length($match);\n     ++\t\tmy $redaction = \"<redacted>\";\n     ++\t\tmy $tail = substr $line, $offset;\n     ++\t\t$line = $head . $redaction . $tail;\n     ++\t\tpos $line = length($head) + length($redaction);\n     ++\t}\n     ++\treturn $line;\n      +}\n      +\n       sub parse_tag {\n       \tmy $tag_id = shift;\n       \tmy %tag;\n      @@ gitweb/gitweb.perl: sub parse_tag {\n     + \t\t} elsif ($line =~ m/^tag (.+)$/) {\n     + \t\t\t$tag{'name'} = $1;\n     + \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n     +-\t\t\t$tag{'author'} = $1;\n     ++\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n     + \t\t\t$tag{'author_epoch'} = $2;\n     + \t\t\t$tag{'author_tz'} = $3;\n       \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n     - \t\t\t\t$tag{'author_name'}  = $1;\n     - \t\t\t\t$tag{'author_email'} = $2;\n     -+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n     -+\t\t\t\t\t$tag{'author_email'} = \"private\";\n     -+\t\t\t\t\t$tag{'author'} = hide_mailaddr($tag{'author'});\n     -+\t\t\t\t}\n     - \t\t\t} else {\n     - \t\t\t\t$tag{'author_name'} = $tag{'author'};\n     - \t\t\t}\n      @@ gitweb/gitweb.perl: sub parse_commit_text {\n     + \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {\n     + \t\t\tpush @parents, $1;\n     + \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n     +-\t\t\t$co{'author'} = to_utf8($1);\n     ++\t\t\t$co{'author'} = hide_mailaddrs_if_private(to_utf8($1));\n     + \t\t\t$co{'author_epoch'} = $2;\n     + \t\t\t$co{'author_tz'} = $3;\n       \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n     - \t\t\t\t$co{'author_name'}  = $1;\n     - \t\t\t\t$co{'author_email'} = $2;\n     -+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n     -+\t\t\t\t\t$co{'author_email'} = \"private\";\n     -+\t\t\t\t\t$co{'author'} = hide_mailaddr($co{'author'});\n     -+\t\t\t\t}\n     - \t\t\t} else {\n     +@@ gitweb/gitweb.perl: sub parse_commit_text {\n       \t\t\t\t$co{'author_name'} = $co{'author'};\n       \t\t\t}\n     -@@ gitweb/gitweb.perl: sub parse_commit_text {\n     + \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n     +-\t\t\t$co{'committer'} = to_utf8($1);\n     ++\t\t\t$co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));\n     + \t\t\t$co{'committer_epoch'} = $2;\n     + \t\t\t$co{'committer_tz'} = $3;\n       \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n     - \t\t\t\t$co{'committer_name'}  = $1;\n     - \t\t\t\t$co{'committer_email'} = $2;\n     -+\t\t\t\tif (gitweb_check_feature('email_privacy')) {\n     -+\t\t\t\t\t$co{'committer_email'} = \"private\";\n     -+\t\t\t\t\t$co{'committer'} = hide_mailaddr($co{'committer'});\n     -+\t\t\t\t}\n     - \t\t\t} else {\n     - \t\t\t\t$co{'committer_name'} = $co{'committer'};\n     - \t\t\t}\n      @@ gitweb/gitweb.perl: sub parse_commit_text {\n       \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n       \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n     @@ gitweb/gitweb.perl: sub parse_commit_text {\n      +\t# remove added spaces, redact e-mail addresses if applicable.\n       \tforeach my $line (@commit_lines) {\n       \t\t$line =~ s/^    //;\n     -+\t\t$line = hide_mailaddr_if_private($line);\n     ++\t\t$line = hide_mailaddrs_if_private($line);\n       \t}\n       \t$co{'comment'} = \\@commit_lines;\n       \n     -@@ gitweb/gitweb.perl: sub git_commitdiff {\n     - \t\tclose $fd\n     - \t\t\tor print \"Reading git-diff-tree failed\\n\";\n     - \t} elsif ($format eq 'patch') {\n     --\t\tlocal $/ = undef;\n     --\t\tprint <$fd>;\n     -+\t\twhile (my $line = <$fd>) {\n     -+\t\t\tprint hide_mailaddr_if_private($line);\n     -+\t\t}\n     - \t\tclose $fd\n     - \t\t\tor print \"Reading git-format-patch failed\\n\";\n     +@@ gitweb/gitweb.perl: sub git_log_generic {\n     + \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n     + \t}\n     + \tmy $patch_max = gitweb_get_feature('patches');\n     +-\tif ($patch_max && !defined $file_name) {\n     ++\tif ($patch_max && !defined $file_name &&\n     ++\t\t!gitweb_check_feature('email-privacy')) {\n     + \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n     + \t\t\t$paging_nav .= \" &sdot; \" .\n     + \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n     +@@ gitweb/gitweb.perl: sub git_commit {\n     + \t\t\t} @$parents ) .\n     + \t\t\t')';\n       \t}\n     +-\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n     ++\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n     ++\t\t!gitweb_check_feature('email-privacy')) {\n     + \t\t$formats_nav .= \" | \" .\n     + \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n     + \t\t\t\t\"patch\");\n     +@@ gitweb/gitweb.perl: sub git_commitdiff {\n     + \t\t$formats_nav =\n     + \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n     + \t\t\t        \"raw\");\n     +-\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n     ++\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n     ++\t\t\t!gitweb_check_feature('email-privacy')) {\n     + \t\t\t$formats_nav .= \" | \" .\n     + \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n     + \t\t\t\t\t\"patch\");\n     +\n     + ## t/lib-gitweb.sh ##\n     +@@ t/lib-gitweb.sh: gitweb_run () {\n     + \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n     + \texport GITWEB_CONFIG\n     + \n     ++\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n     ++\texport PERL5LIB\n     ++\n     + \t# some of git commands write to STDERR on error, but this is not\n     + \t# written to web server logs, so we are not interested in that:\n     + \t# we are interested only in properly formatted errors/warnings\n\n\n Documentation/gitweb.conf.txt | 10 +++++++\n gitweb/gitweb.perl            | 54 ++++++++++++++++++++++++++++++-----\n t/lib-gitweb.sh               |  3 ++\n 3 files changed, 60 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba98b..47d85717ccaa 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -751,6 +751,16 @@ default font sizes or lineheights are changed (e.g. via adding extra\n CSS stylesheet in `@stylesheets`), it may be appropriate to change\n these values.\n \n+email-privacy::\n+\tRedact e-mail addresses from the generated HTML, etc. content.\n+\tThis hides e-mail addresses found in the commit log from HTTP clients.\n+\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n+\tSuch crawlers may not respect robots.txt.\n+\tNote that users and user tools also see the addresses redacted.\n+\tIf Gitweb is not the final step in a workflow then subsequent steps\n+\tmay misbehave because of the redacted information they receive.\n+\tDisabled by default.\n+\n highlight::\n \tServer-side syntax highlight support in \"blob\" view.  It requires\n \t`$highlight_bin` program to be available (see the description of\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782eccb..6630c76d92fd 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -21,6 +21,7 @@\n use File::Basename qw(basename);\n use Time::HiRes qw(gettimeofday tv_interval);\n use Digest::MD5 qw(md5_hex);\n+use Git::LoadCPAN::Mail::Address;\n \n binmode STDOUT, ':utf8';\n \n@@ -569,6 +570,15 @@ sub evaluate_uri {\n \t\t'sub' => \\&feature_extra_branch_refs,\n \t\t'override' => 0,\n \t\t'default' => []},\n+\n+\t# Redact e-mail addresses.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'email-privacy'}{'default'} = [1];\n+\t'email-privacy' => {\n+\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n+\t\t'override' => 1,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -3449,6 +3459,32 @@ sub parse_date {\n \treturn %date;\n }\n \n+sub is_mailaddr {\n+\tmy @addrs = Mail::Address->parse(shift);\n+\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub hide_mailaddrs_if_private {\n+\tmy $line = shift;\n+\treturn $line unless gitweb_check_feature('email-privacy');\n+\twhile ($line =~ m/(<[^>]+>)/g) {\n+\t\tmy $match = $1;\n+\t\tif (!is_mailaddr($match)) {\n+\t\t\tnext;\n+\t\t}\n+\t\tmy $offset = pos $line;\n+\t\tmy $head = substr $line, 0, $offset - length($match);\n+\t\tmy $redaction = \"<redacted>\";\n+\t\tmy $tail = substr $line, $offset;\n+\t\t$line = $head . $redaction . $tail;\n+\t\tpos $line = length($head) + length($redaction);\n+\t}\n+\treturn $line;\n+}\n+\n sub parse_tag {\n \tmy $tag_id = shift;\n \tmy %tag;\n@@ -3465,7 +3501,7 @@ sub parse_tag {\n \t\t} elsif ($line =~ m/^tag (.+)$/) {\n \t\t\t$tag{'name'} = $1;\n \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$tag{'author'} = $1;\n+\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n \t\t\t$tag{'author_epoch'} = $2;\n \t\t\t$tag{'author_tz'} = $3;\n \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3513,7 +3549,7 @@ sub parse_commit_text {\n \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {\n \t\t\tpush @parents, $1;\n \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'author'} = to_utf8($1);\n+\t\t\t$co{'author'} = hide_mailaddrs_if_private(to_utf8($1));\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3523,7 +3559,7 @@ sub parse_commit_text {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'committer'} = to_utf8($1);\n+\t\t\t$co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3568,9 +3604,10 @@ sub parse_commit_text {\n \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n \t}\n-\t# remove added spaces\n+\t# remove added spaces, redact e-mail addresses if applicable.\n \tforeach my $line (@commit_lines) {\n \t\t$line =~ s/^    //;\n+\t\t$line = hide_mailaddrs_if_private($line);\n \t}\n \t$co{'comment'} = \\@commit_lines;\n \n@@ -7489,7 +7526,8 @@ sub git_log_generic {\n \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n \t}\n \tmy $patch_max = gitweb_get_feature('patches');\n-\tif ($patch_max && !defined $file_name) {\n+\tif ($patch_max && !defined $file_name &&\n+\t\t!gitweb_check_feature('email-privacy')) {\n \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n \t\t\t$paging_nav .= \" &sdot; \" .\n \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n@@ -7550,7 +7588,8 @@ sub git_commit {\n \t\t\t} @$parents ) .\n \t\t\t')';\n \t}\n-\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n+\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n+\t\t!gitweb_check_feature('email-privacy')) {\n \t\t$formats_nav .= \" | \" .\n \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n \t\t\t\t\"patch\");\n@@ -7863,7 +7902,8 @@ sub git_commitdiff {\n \t\t$formats_nav =\n \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n \t\t\t        \"raw\");\n-\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n+\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n+\t\t\t!gitweb_check_feature('email-privacy')) {\n \t\t\t$formats_nav .= \" | \" .\n \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n \t\t\t\t\t\"patch\");\ndiff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh\nindex 1f32ca66ea51..77fc1298d4c6 100644\n--- a/t/lib-gitweb.sh\n+++ b/t/lib-gitweb.sh\n@@ -67,6 +67,9 @@ gitweb_run () {\n \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n \texport GITWEB_CONFIG\n \n+\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n+\texport PERL5LIB\n+\n \t# some of git commands write to STDERR on error, but this is not\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n\nbase-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n-- \ngitgitgadget\n"},{"id":"419967","messageId":"xmqqlfaf6nu9.fsf@gitster.g","threadId":"55358","inReplyTo":"pull.910.v4.git.1616396267010.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-22T18:32:46Z","receivedAt":"2021-03-22T18:33:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n[note to other reviewers. input from those who are more familiar\nwith gitweb and Perl is very much appreciated on this patch].\n\n> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>\n> Gitweb extracts content from the Git log and makes it accessible\n> over HTTP. As a result, e-mail addresses found in commits are\n> exposed to web crawlers and they may not respect robots.txt.\n> This may result in unsolicited messages.\n\n\"... are exposed to web crawlers, which spammers may use.\" would be\nsufficient as a problem description.\n\nAfter giving a problem description, it is customery to describe the\nsolution as if you are ordering the codebase to \"be like so\", so\ninstead of this ...\n\n> This is a feature for redacting e-mail addresses\n> from the generated HTML, etc. content.\n    \n... we may say something like\n\n    Introduce an 'email-privacy' feature, which defaults to false,\n    that redacts e-mail addresses that appear as author/committer\n    info and in log messages from the generated HTML content.\n\n> This feature does not prevent someone from downloading the\n> unredacted commit log, e.g., by cloning the repository, and\n> extracting information from it.\n> It aims to hinder the low-effort bulk collection of e-mail\n> addresses by web crawlers.\n\nAnd this is a good thing to add.  Overall, nicely written.\n\n> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n> ---\n>     gitweb: redacted e-mail addresses feature.\n>     \n>     Gitweb extracts content from the Git log and makes it accessible over\n>     HTTP. As a result, e-mail addresses found in commits are exposed to web\n>     crawlers and they may not respect robots.txt. This may result in\n>     unsolicited messages. This is a feature for redacting e-mail addresses\n>     from the generated HTML, etc. content.\n>     \n>     This feature does not prevent someone from downloading the unredacted\n>     commit log, e.g., by cloning the repository, and extracting information\n>     from it. It aims to hinder the low-effort bulk collection of e-mail\n>     addresses by web crawlers.\n\nYou do not need to repeat the above, which is in the log message above.\n\n>     Changes since v1:\n>     \n>      * Turned off the feature by default.\n>      * Removed duplicate code.\n>      * Added note about Gitweb consumers receiving redacted logs.\n>     \n>     Changes since v2:\n>     \n>      * The feature can be set on a per-project basis. ('override' => 1)\n>     \n>     Changes since v3:\n>     \n>      * Renamed feature to \"email-privacy\" and improved documentation.\n>      * Removed UI elements for git-format-patch since it won't be redacted.\n>      * Simplified calls to the address redaction logic.\n>      * Mail::Address is now used to reduce false-positive redactions.\n\nHaving these under the --- line like this is very helpful.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0959a782eccb..6630c76d92fd 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -21,6 +21,7 @@\n>  use File::Basename qw(basename);\n>  use Time::HiRes qw(gettimeofday tv_interval);\n>  use Digest::MD5 qw(md5_hex);\n> +use Git::LoadCPAN::Mail::Address;\n\nI'll defer to others who are more familiar with gitweb and Perl\necosystem if this is warranted, but I have a feeling that importing\nand using Mail::Address->parse() only because we want to see if a\ngiven \"<string>\" is an address is a bit overkill and it might be\nsufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i\n\n> +\t# Redact e-mail addresses.\n> +\n> +\t# To enable system wide have in $GITWEB_CONFIG\n> +\t# $feature{'email-privacy'}{'default'} = [1];\n> +\t'email-privacy' => {\n> +\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n> +\t\t'override' => 1,\n> +\t\t'default' => [0]},\n>  );\n>  \n>  sub gitweb_get_feature {\n> @@ -3449,6 +3459,32 @@ sub parse_date {\n>  \treturn %date;\n>  }\n>  \n> +sub is_mailaddr {\n> +\tmy @addrs = Mail::Address->parse(shift);\n> +\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n> +\t\treturn 0;\n> +\t}\n> +\treturn 1;\n> +}\n> +\n> +sub hide_mailaddrs_if_private {\n> +\tmy $line = shift;\n> +\treturn $line unless gitweb_check_feature('email-privacy');\n> +\twhile ($line =~ m/(<[^>]+>)/g) {\n> +\t\tmy $match = $1;\n> +\t\tif (!is_mailaddr($match)) {\n> +\t\t\tnext;\n> +\t\t}\n> +\t\tmy $offset = pos $line;\n> +\t\tmy $head = substr $line, 0, $offset - length($match);\n> +\t\tmy $redaction = \"<redacted>\";\n> +\t\tmy $tail = substr $line, $offset;\n> +\t\t$line = $head . $redaction . $tail;\n> +\t\tpos $line = length($head) + length($redaction);\n\nHmmmm, Perl suggestions from others?  It looks quite strange to see\nthat s/// operator is not used and replacement is done manually with\nbyte position in a Perl script.\n\n>  sub parse_tag {\n>  \tmy $tag_id = shift;\n>  \tmy %tag;\n> @@ -3465,7 +3501,7 @@ sub parse_tag {\n>  \t\t} elsif ($line =~ m/^tag (.+)$/) {\n>  \t\t\t$tag{'name'} = $1;\n>  \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n> -\t\t\t$tag{'author'} = $1;\n> +\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n>  \t\t\t$tag{'author_epoch'} = $2;\n>  \t\t\t$tag{'author_tz'} = $3;\n>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n\nThis (and the others that follow the same pattern) looks sensible.\n\n> @@ -7489,7 +7526,8 @@ sub git_log_generic {\n>  \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n>  \t}\n>  \tmy $patch_max = gitweb_get_feature('patches');\n> -\tif ($patch_max && !defined $file_name) {\n> +\tif ($patch_max && !defined $file_name &&\n> +\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n>  \t\t\t$paging_nav .= \" &sdot; \" .\n>  \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n> @@ -7550,7 +7588,8 @@ sub git_commit {\n>  \t\t\t} @$parents ) .\n>  \t\t\t')';\n>  \t}\n> -\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n> +\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n> +\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\t$formats_nav .= \" | \" .\n>  \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>  \t\t\t\t\"patch\");\n> @@ -7863,7 +7902,8 @@ sub git_commitdiff {\n>  \t\t$formats_nav =\n>  \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n>  \t\t\t        \"raw\");\n> -\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n> +\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n> +\t\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\t\t$formats_nav .= \" | \" .\n>  \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>  \t\t\t\t\t\"patch\");\n\nI wouldn't have expected to see the above three hunks in the\n\"patch\" codepath.  Rather, I was hoping that you'd do something\nlike this at startup when you find out that the privacy feature\nis enabled:\n\n\t$feature{'patches'}{'default'} = [0]\n\t\tif gitweb_get_feature('email-privacy');\n\nso that nothing related to the 'patches' has to be modified.\nThat way, even if there were fourth place that can leak an e-mail\naddress in the 'patch' codepath that above three hunks in this patch\nmissed, crawlers won't be able to get at it, no?\n\n> diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh\n> index 1f32ca66ea51..77fc1298d4c6 100644\n> --- a/t/lib-gitweb.sh\n> +++ b/t/lib-gitweb.sh\n> @@ -67,6 +67,9 @@ gitweb_run () {\n>  \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n>  \texport GITWEB_CONFIG\n>  \n> +\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n> +\texport PERL5LIB\n> +\n\nWhy is this change suddenly become necessary?  This addition is only\nabout tests---do we need to do something similar in the runtime\nenvironment when the updated gitweb that requires Mail::Address gets\ndeployed, or is that covered already somewhere else?\n\nThanks.\n"},{"id":"419971","messageId":"6f656f62497fb7c9322432b5eb151b86.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"xmqqlfaf6nu9.fsf@gitster.g","subject":"Re: [PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-22T18:59:26Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> \"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> [note to other reviewers. input from those who are more familiar\n> with gitweb and Perl is very much appreciated on this patch].\n>\n>> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers and they may not respect robots.txt.\n>> This may result in unsolicited messages.\n>\n> \"... are exposed to web crawlers, which spammers may use.\" would be\n> sufficient as a problem description.\n>\nUsing robots.txt was suggested earlier in this review so\nI want to make sure people don't rely on it instead.\n\n> After giving a problem description, it is customery to describe the\n> solution as if you are ordering the codebase to \"be like so\", so\n> instead of this ...\n>\n>> This is a feature for redacting e-mail addresses\n>> from the generated HTML, etc. content.\n>\n> ... we may say something like\n>\n>     Introduce an 'email-privacy' feature, which defaults to false,\n>     that redacts e-mail addresses that appear as author/committer\n>     info and in log messages from the generated HTML content.\n>\nWill rephrase.\n\n>> This feature does not prevent someone from downloading the\n>> unredacted commit log, e.g., by cloning the repository, and\n>> extracting information from it.\n>> It aims to hinder the low-effort bulk collection of e-mail\n>> addresses by web crawlers.\n>\n> And this is a good thing to add.  Overall, nicely written.\n>\n>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>> ---\n>>     gitweb: redacted e-mail addresses feature.\n>>\n>>     Gitweb extracts content from the Git log and makes it accessible\n>> over\n>>     HTTP. As a result, e-mail addresses found in commits are exposed to\n>> web\n>>     crawlers and they may not respect robots.txt. This may result in\n>>     unsolicited messages. This is a feature for redacting e-mail\n>> addresses\n>>     from the generated HTML, etc. content.\n>>\n>>     This feature does not prevent someone from downloading the\n>> unredacted\n>>     commit log, e.g., by cloning the repository, and extracting\n>> information\n>>     from it. It aims to hinder the low-effort bulk collection of e-mail\n>>     addresses by web crawlers.\n>\n> You do not need to repeat the above, which is in the log message above.\n>\n>>     Changes since v1:\n>>\n>>      * Turned off the feature by default.\n>>      * Removed duplicate code.\n>>      * Added note about Gitweb consumers receiving redacted logs.\n>>\n>>     Changes since v2:\n>>\n>>      * The feature can be set on a per-project basis. ('override' => 1)\n>>\n>>     Changes since v3:\n>>\n>>      * Renamed feature to \"email-privacy\" and improved documentation.\n>>      * Removed UI elements for git-format-patch since it won't be\n>> redacted.\n>>      * Simplified calls to the address redaction logic.\n>>      * Mail::Address is now used to reduce false-positive redactions.\n>\n> Having these under the --- line like this is very helpful.\n>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 0959a782eccb..6630c76d92fd 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -21,6 +21,7 @@\n>>  use File::Basename qw(basename);\n>>  use Time::HiRes qw(gettimeofday tv_interval);\n>>  use Digest::MD5 qw(md5_hex);\n>> +use Git::LoadCPAN::Mail::Address;\n>\n> I'll defer to others who are more familiar with gitweb and Perl\n> ecosystem if this is warranted, but I have a feeling that importing\n> and using Mail::Address->parse() only because we want to see if a\n> given \"<string>\" is an address is a bit overkill and it might be\n> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i\n>\nI think Mail::Address->parse() pretty much does the same thing.\nIt was suggested earlier in the review to use that function,\nsimilar to what git-send-mail does.\nI'm assuming because the maintainers of the Mail package are already\ninvested in the (accurate) parsing of e-mail addresses.\n\n>> +\t# Redact e-mail addresses.\n>> +\n>> +\t# To enable system wide have in $GITWEB_CONFIG\n>> +\t# $feature{'email-privacy'}{'default'} = [1];\n>> +\t'email-privacy' => {\n>> +\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n>> +\t\t'override' => 1,\n>> +\t\t'default' => [0]},\n>>  );\n>>\n>>  sub gitweb_get_feature {\n>> @@ -3449,6 +3459,32 @@ sub parse_date {\n>>  \treturn %date;\n>>  }\n>>\n>> +sub is_mailaddr {\n>> +\tmy @addrs = Mail::Address->parse(shift);\n>> +\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n>> +\t\treturn 0;\n>> +\t}\n>> +\treturn 1;\n>> +}\n>> +\n>> +sub hide_mailaddrs_if_private {\n>> +\tmy $line = shift;\n>> +\treturn $line unless gitweb_check_feature('email-privacy');\n>> +\twhile ($line =~ m/(<[^>]+>)/g) {\n>> +\t\tmy $match = $1;\n>> +\t\tif (!is_mailaddr($match)) {\n>> +\t\t\tnext;\n>> +\t\t}\n>> +\t\tmy $offset = pos $line;\n>> +\t\tmy $head = substr $line, 0, $offset - length($match);\n>> +\t\tmy $redaction = \"<redacted>\";\n>> +\t\tmy $tail = substr $line, $offset;\n>> +\t\t$line = $head . $redaction . $tail;\n>> +\t\tpos $line = length($head) + length($redaction);\n>\n> Hmmmm, Perl suggestions from others?  It looks quite strange to see\n> that s/// operator is not used and replacement is done manually with\n> byte position in a Perl script.\n>\nIf there's a more elegant way to do the above we can certain do that instead.\n\n>>  sub parse_tag {\n>>  \tmy $tag_id = shift;\n>>  \tmy %tag;\n>> @@ -3465,7 +3501,7 @@ sub parse_tag {\n>>  \t\t} elsif ($line =~ m/^tag (.+)$/) {\n>>  \t\t\t$tag{'name'} = $1;\n>>  \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n>> -\t\t\t$tag{'author'} = $1;\n>> +\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n>>  \t\t\t$tag{'author_epoch'} = $2;\n>>  \t\t\t$tag{'author_tz'} = $3;\n>>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>\n> This (and the others that follow the same pattern) looks sensible.\n>\n>> @@ -7489,7 +7526,8 @@ sub git_log_generic {\n>>  \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n>>  \t}\n>>  \tmy $patch_max = gitweb_get_feature('patches');\n>> -\tif ($patch_max && !defined $file_name) {\n>> +\tif ($patch_max && !defined $file_name &&\n>> +\t\t!gitweb_check_feature('email-privacy')) {\n>>  \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n>>  \t\t\t$paging_nav .= \" &sdot; \" .\n>>  \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n>> @@ -7550,7 +7588,8 @@ sub git_commit {\n>>  \t\t\t} @$parents ) .\n>>  \t\t\t')';\n>>  \t}\n>> -\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n>> +\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n>> +\t\t!gitweb_check_feature('email-privacy')) {\n>>  \t\t$formats_nav .= \" | \" .\n>>  \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>>  \t\t\t\t\"patch\");\n>> @@ -7863,7 +7902,8 @@ sub git_commitdiff {\n>>  \t\t$formats_nav =\n>>  \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n>>  \t\t\t        \"raw\");\n>> -\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n>> +\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n>> +\t\t\t!gitweb_check_feature('email-privacy')) {\n>>  \t\t\t$formats_nav .= \" | \" .\n>>  \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>>  \t\t\t\t\t\"patch\");\n>\n> I wouldn't have expected to see the above three hunks in the\n> \"patch\" codepath.  Rather, I was hoping that you'd do something\n> like this at startup when you find out that the privacy feature\n> is enabled:\n>\n> \t$feature{'patches'}{'default'} = [0]\n> \t\tif gitweb_get_feature('email-privacy');\n>\n> so that nothing related to the 'patches' has to be modified.\n> That way, even if there were fourth place that can leak an e-mail\n> address in the 'patch' codepath that above three hunks in this patch\n> missed, crawlers won't be able to get at it, no?\n>\nOr there's multiple feature flags / triggers that lead to the \"patch\"\ncodepath.\nEither way is fine with me. (disabling the \"patches\" feature or preventing\nthe\ngeneration of UI elements)\nLike you said, someone who regularly reviews Gitweb code may have an opinion\non how do this.\n\n>> diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh\n>> index 1f32ca66ea51..77fc1298d4c6 100644\n>> --- a/t/lib-gitweb.sh\n>> +++ b/t/lib-gitweb.sh\n>> @@ -67,6 +67,9 @@ gitweb_run () {\n>>  \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n>>  \texport GITWEB_CONFIG\n>>\n>> +\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n>> +\texport PERL5LIB\n>> +\n>\n> Why is this change suddenly become necessary?  This addition is only\n> about tests---do we need to do something similar in the runtime\n> environment when the updated gitweb that requires Mail::Address gets\n> deployed, or is that covered already somewhere else?\n>\nThis is necessary because osx-clang, osx-gcc tests were failing.\nIf there's a better way to address this I'm open to suggestions.\n\nI haven't been able to experience deployment on osx but on linux\ni didn't have to change anything.\n\n> Thanks.\n>\n\n\n"},{"id":"419983","messageId":"f604de775736756180d382d2f54e8b1c.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"xmqqlfaf6nu9.fsf@gitster.g","subject":"Re: [PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-23T04:27:51Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> \"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> [note to other reviewers. input from those who are more familiar\n> with gitweb and Perl is very much appreciated on this patch].\n>\n>> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers and they may not respect robots.txt.\n>> This may result in unsolicited messages.\n>\n> \"... are exposed to web crawlers, which spammers may use.\" would be\n> sufficient as a problem description.\n>\n> After giving a problem description, it is customery to describe the\n> solution as if you are ordering the codebase to \"be like so\", so\n> instead of this ...\n>\n>> This is a feature for redacting e-mail addresses\n>> from the generated HTML, etc. content.\n>\n> ... we may say something like\n>\n>     Introduce an 'email-privacy' feature, which defaults to false,\n>     that redacts e-mail addresses that appear as author/committer\n>     info and in log messages from the generated HTML content.\n>\n>> This feature does not prevent someone from downloading the\n>> unredacted commit log, e.g., by cloning the repository, and\n>> extracting information from it.\n>> It aims to hinder the low-effort bulk collection of e-mail\n>> addresses by web crawlers.\n>\n> And this is a good thing to add.  Overall, nicely written.\n>\n>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n>> ---\n>>     gitweb: redacted e-mail addresses feature.\n>>\n>>     Gitweb extracts content from the Git log and makes it accessible\n>> over\n>>     HTTP. As a result, e-mail addresses found in commits are exposed to\n>> web\n>>     crawlers and they may not respect robots.txt. This may result in\n>>     unsolicited messages. This is a feature for redacting e-mail\n>> addresses\n>>     from the generated HTML, etc. content.\n>>\n>>     This feature does not prevent someone from downloading the\n>> unredacted\n>>     commit log, e.g., by cloning the repository, and extracting\n>> information\n>>     from it. It aims to hinder the low-effort bulk collection of e-mail\n>>     addresses by web crawlers.\n>\n> You do not need to repeat the above, which is in the log message above.\n>\n>>     Changes since v1:\n>>\n>>      * Turned off the feature by default.\n>>      * Removed duplicate code.\n>>      * Added note about Gitweb consumers receiving redacted logs.\n>>\n>>     Changes since v2:\n>>\n>>      * The feature can be set on a per-project basis. ('override' => 1)\n>>\n>>     Changes since v3:\n>>\n>>      * Renamed feature to \"email-privacy\" and improved documentation.\n>>      * Removed UI elements for git-format-patch since it won't be\n>> redacted.\n>>      * Simplified calls to the address redaction logic.\n>>      * Mail::Address is now used to reduce false-positive redactions.\n>\n> Having these under the --- line like this is very helpful.\n>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 0959a782eccb..6630c76d92fd 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -21,6 +21,7 @@\n>>  use File::Basename qw(basename);\n>>  use Time::HiRes qw(gettimeofday tv_interval);\n>>  use Digest::MD5 qw(md5_hex);\n>> +use Git::LoadCPAN::Mail::Address;\n>\n> I'll defer to others who are more familiar with gitweb and Perl\n> ecosystem if this is warranted, but I have a feeling that importing\n> and using Mail::Address->parse() only because we want to see if a\n> given \"<string>\" is an address is a bit overkill and it might be\n> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i\n>\n>> +\t# Redact e-mail addresses.\n>> +\n>> +\t# To enable system wide have in $GITWEB_CONFIG\n>> +\t# $feature{'email-privacy'}{'default'} = [1];\n>> +\t'email-privacy' => {\n>> +\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n>> +\t\t'override' => 1,\n>> +\t\t'default' => [0]},\n>>  );\n>>\n>>  sub gitweb_get_feature {\n>> @@ -3449,6 +3459,32 @@ sub parse_date {\n>>  \treturn %date;\n>>  }\n>>\n>> +sub is_mailaddr {\n>> +\tmy @addrs = Mail::Address->parse(shift);\n>> +\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n>> +\t\treturn 0;\n>> +\t}\n>> +\treturn 1;\n>> +}\n>> +\n>> +sub hide_mailaddrs_if_private {\n>> +\tmy $line = shift;\n>> +\treturn $line unless gitweb_check_feature('email-privacy');\n>> +\twhile ($line =~ m/(<[^>]+>)/g) {\n>> +\t\tmy $match = $1;\n>> +\t\tif (!is_mailaddr($match)) {\n>> +\t\t\tnext;\n>> +\t\t}\n>> +\t\tmy $offset = pos $line;\n>> +\t\tmy $head = substr $line, 0, $offset - length($match);\n>> +\t\tmy $redaction = \"<redacted>\";\n>> +\t\tmy $tail = substr $line, $offset;\n>> +\t\t$line = $head . $redaction . $tail;\n>> +\t\tpos $line = length($head) + length($redaction);\n>\n> Hmmmm, Perl suggestions from others?  It looks quite strange to see\n> that s/// operator is not used and replacement is done manually with\n> byte position in a Perl script.\n>\n>>  sub parse_tag {\n>>  \tmy $tag_id = shift;\n>>  \tmy %tag;\n>> @@ -3465,7 +3501,7 @@ sub parse_tag {\n>>  \t\t} elsif ($line =~ m/^tag (.+)$/) {\n>>  \t\t\t$tag{'name'} = $1;\n>>  \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n>> -\t\t\t$tag{'author'} = $1;\n>> +\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n>>  \t\t\t$tag{'author_epoch'} = $2;\n>>  \t\t\t$tag{'author_tz'} = $3;\n>>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n>\n> This (and the others that follow the same pattern) looks sensible.\n>\n>> @@ -7489,7 +7526,8 @@ sub git_log_generic {\n>>  \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n>>  \t}\n>>  \tmy $patch_max = gitweb_get_feature('patches');\n>> -\tif ($patch_max && !defined $file_name) {\n>> +\tif ($patch_max && !defined $file_name &&\n>> +\t\t!gitweb_check_feature('email-privacy')) {\n>>  \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n>>  \t\t\t$paging_nav .= \" &sdot; \" .\n>>  \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n>> @@ -7550,7 +7588,8 @@ sub git_commit {\n>>  \t\t\t} @$parents ) .\n>>  \t\t\t')';\n>>  \t}\n>> -\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n>> +\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n>> +\t\t!gitweb_check_feature('email-privacy')) {\n>>  \t\t$formats_nav .= \" | \" .\n>>  \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>>  \t\t\t\t\"patch\");\n>> @@ -7863,7 +7902,8 @@ sub git_commitdiff {\n>>  \t\t$formats_nav =\n>>  \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n>>  \t\t\t        \"raw\");\n>> -\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n>> +\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n>> +\t\t\t!gitweb_check_feature('email-privacy')) {\n>>  \t\t\t$formats_nav .= \" | \" .\n>>  \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>>  \t\t\t\t\t\"patch\");\n>\n> I wouldn't have expected to see the above three hunks in the\n> \"patch\" codepath.  Rather, I was hoping that you'd do something\n> like this at startup when you find out that the privacy feature\n> is enabled:\n>\n> \t$feature{'patches'}{'default'} = [0]\n> \t\tif gitweb_get_feature('email-privacy');\n>\n> so that nothing related to the 'patches' has to be modified.\n> That way, even if there were fourth place that can leak an e-mail\n> address in the 'patch' codepath that above three hunks in this patch\n> missed, crawlers won't be able to get at it, no?\n>\nI forgot to mention one of the reasons for doing it this way.\nIf someone knows this URL exists and is relying on it (e.g., in a script)\nthey can still call it with \"email-privacy\" enabled.\nThe only thing that's changing is that the UI element (link) that a web\ncrawler would follow just because it's there is now gone.\n\nBut, as mentioned before, I don't feel strongly about either approach.\nLet me know.\n\n>> diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh\n>> index 1f32ca66ea51..77fc1298d4c6 100644\n>> --- a/t/lib-gitweb.sh\n>> +++ b/t/lib-gitweb.sh\n>> @@ -67,6 +67,9 @@ gitweb_run () {\n>>  \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n>>  \texport GITWEB_CONFIG\n>>\n>> +\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n>> +\texport PERL5LIB\n>> +\n>\n> Why is this change suddenly become necessary?  This addition is only\n> about tests---do we need to do something similar in the runtime\n> environment when the updated gitweb that requires Mail::Address gets\n> deployed, or is that covered already somewhere else?\n>\n> Thanks.\n>\n\n\n"},{"id":"420292","messageId":"pull.910.v5.git.1616817387441.gitgitgadget@gmail.com","threadId":"55358","inReplyTo":"pull.910.v4.git.1616396267010.gitgitgadget@gmail.com","subject":"[PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-27T03:56:26Z","receivedAt":"2021-03-27T03:57:15Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"From: Georgios Kontaxis <geko1702+commits@99rst.org>\n\nGitweb extracts content from the Git log and makes it accessible\nover HTTP. As a result, e-mail addresses found in commits are\nexposed to web crawlers and they may not respect robots.txt.\nThis can result in unsolicited messages.\n\nIntroduce an 'email-privacy' feature which redacts e-mail addresses\nfrom the generated HTML content. Specifically, obscure addresses\nretrieved from the the author/committer and comment sections of the\nGit log. The feature is off by default.\n\nThis feature does not prevent someone from downloading the\nunredacted commit log, e.g., by cloning the repository, and\nextracting information from it. It aims to hinder the low-\neffort, bulk collection of e-mail addresses by web crawlers.\n\nSigned-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n---\n    gitweb: redacted e-mail addresses feature.\n    \n    Changes since v1:\n    \n     * Turned off the feature by default.\n     * Removed duplicate code.\n     * Added note about Gitweb consumers receiving redacted logs.\n    \n    Changes since v2:\n    \n     * The feature can be set on a per-project basis. ('override' => 1)\n    \n    Changes since v3:\n    \n     * Renamed feature to \"email-privacy\" and improved documentation.\n     * Removed UI elements for git-format-patch since it won't be redacted.\n     * Simplified calls to the address redaction logic.\n     * Mail::Address is now used to reduce false-positive redactions.\n    \n    Changes since v4:\n    \n     * Rephrased the commit comment.\n     * hide_mailaddrs_if_private is slighly more compact.\n    \n    Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/910\n\nRange-diff vs v4:\n\n 1:  03a3f41c37ef ! 1:  1427231f9db5 gitweb: redacted e-mail addresses feature.\n     @@ Commit message\n          Gitweb extracts content from the Git log and makes it accessible\n          over HTTP. As a result, e-mail addresses found in commits are\n          exposed to web crawlers and they may not respect robots.txt.\n     -    This may result in unsolicited messages.\n     -    This is a feature for redacting e-mail addresses\n     -    from the generated HTML, etc. content.\n     +    This can result in unsolicited messages.\n     +\n     +    Introduce an 'email-privacy' feature which redacts e-mail addresses\n     +    from the generated HTML content. Specifically, obscure addresses\n     +    retrieved from the the author/committer and comment sections of the\n     +    Git log. The feature is off by default.\n      \n          This feature does not prevent someone from downloading the\n          unredacted commit log, e.g., by cloning the repository, and\n     -    extracting information from it.\n     -    It aims to hinder the low-effort bulk collection of e-mail\n     -    addresses by web crawlers.\n     +    extracting information from it. It aims to hinder the low-\n     +    effort, bulk collection of e-mail addresses by web crawlers.\n      \n          Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n      \n     @@ Documentation/gitweb.conf.txt: default font sizes or lineheights are changed (e.\n       \n      +email-privacy::\n      +\tRedact e-mail addresses from the generated HTML, etc. content.\n     -+\tThis hides e-mail addresses found in the commit log from HTTP clients.\n     ++\tThis obscures e-mail addresses retrieved from the author/committer\n     ++\tand comment sections of the Git log.\n      +\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n      +\tSuch crawlers may not respect robots.txt.\n     -+\tNote that users and user tools also see the addresses redacted.\n     ++\tNote that users and user tools also see the addresses as redacted.\n      +\tIf Gitweb is not the final step in a workflow then subsequent steps\n      +\tmay misbehave because of the redacted information they receive.\n      +\tDisabled by default.\n     @@ gitweb/gitweb.perl: sub parse_date {\n      +\t\tif (!is_mailaddr($match)) {\n      +\t\t\tnext;\n      +\t\t}\n     -+\t\tmy $offset = pos $line;\n     -+\t\tmy $head = substr $line, 0, $offset - length($match);\n     ++\t\tmy $match_offset = pos($line) - length($match);\n     ++\t\tpos $line = $match_offset;\n     ++\n      +\t\tmy $redaction = \"<redacted>\";\n     -+\t\tmy $tail = substr $line, $offset;\n     -+\t\t$line = $head . $redaction . $tail;\n     -+\t\tpos $line = length($head) + length($redaction);\n     ++\t\t$line =~ s/\\G(<[^>]+>)/$redaction/;\n     ++\n     ++\t\tpos $line = $match_offset + length($redaction);\n      +\t}\n      +\treturn $line;\n      +}\n\n\n Documentation/gitweb.conf.txt | 11 +++++++\n gitweb/gitweb.perl            | 55 ++++++++++++++++++++++++++++++-----\n t/lib-gitweb.sh               |  3 ++\n 3 files changed, 62 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba98b..34b1d6e22435 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -751,6 +751,17 @@ default font sizes or lineheights are changed (e.g. via adding extra\n CSS stylesheet in `@stylesheets`), it may be appropriate to change\n these values.\n \n+email-privacy::\n+\tRedact e-mail addresses from the generated HTML, etc. content.\n+\tThis obscures e-mail addresses retrieved from the author/committer\n+\tand comment sections of the Git log.\n+\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n+\tSuch crawlers may not respect robots.txt.\n+\tNote that users and user tools also see the addresses as redacted.\n+\tIf Gitweb is not the final step in a workflow then subsequent steps\n+\tmay misbehave because of the redacted information they receive.\n+\tDisabled by default.\n+\n highlight::\n \tServer-side syntax highlight support in \"blob\" view.  It requires\n \t`$highlight_bin` program to be available (see the description of\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782eccb..fe1dbc266ea7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -21,6 +21,7 @@\n use File::Basename qw(basename);\n use Time::HiRes qw(gettimeofday tv_interval);\n use Digest::MD5 qw(md5_hex);\n+use Git::LoadCPAN::Mail::Address;\n \n binmode STDOUT, ':utf8';\n \n@@ -569,6 +570,15 @@ sub evaluate_uri {\n \t\t'sub' => \\&feature_extra_branch_refs,\n \t\t'override' => 0,\n \t\t'default' => []},\n+\n+\t# Redact e-mail addresses.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'email-privacy'}{'default'} = [1];\n+\t'email-privacy' => {\n+\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n+\t\t'override' => 1,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -3449,6 +3459,33 @@ sub parse_date {\n \treturn %date;\n }\n \n+sub is_mailaddr {\n+\tmy @addrs = Mail::Address->parse(shift);\n+\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub hide_mailaddrs_if_private {\n+\tmy $line = shift;\n+\treturn $line unless gitweb_check_feature('email-privacy');\n+\twhile ($line =~ m/(<[^>]+>)/g) {\n+\t\tmy $match = $1;\n+\t\tif (!is_mailaddr($match)) {\n+\t\t\tnext;\n+\t\t}\n+\t\tmy $match_offset = pos($line) - length($match);\n+\t\tpos $line = $match_offset;\n+\n+\t\tmy $redaction = \"<redacted>\";\n+\t\t$line =~ s/\\G(<[^>]+>)/$redaction/;\n+\n+\t\tpos $line = $match_offset + length($redaction);\n+\t}\n+\treturn $line;\n+}\n+\n sub parse_tag {\n \tmy $tag_id = shift;\n \tmy %tag;\n@@ -3465,7 +3502,7 @@ sub parse_tag {\n \t\t} elsif ($line =~ m/^tag (.+)$/) {\n \t\t\t$tag{'name'} = $1;\n \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$tag{'author'} = $1;\n+\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n \t\t\t$tag{'author_epoch'} = $2;\n \t\t\t$tag{'author_tz'} = $3;\n \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3513,7 +3550,7 @@ sub parse_commit_text {\n \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {\n \t\t\tpush @parents, $1;\n \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'author'} = to_utf8($1);\n+\t\t\t$co{'author'} = hide_mailaddrs_if_private(to_utf8($1));\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3523,7 +3560,7 @@ sub parse_commit_text {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'committer'} = to_utf8($1);\n+\t\t\t$co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3568,9 +3605,10 @@ sub parse_commit_text {\n \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n \t}\n-\t# remove added spaces\n+\t# remove added spaces, redact e-mail addresses if applicable.\n \tforeach my $line (@commit_lines) {\n \t\t$line =~ s/^    //;\n+\t\t$line = hide_mailaddrs_if_private($line);\n \t}\n \t$co{'comment'} = \\@commit_lines;\n \n@@ -7489,7 +7527,8 @@ sub git_log_generic {\n \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n \t}\n \tmy $patch_max = gitweb_get_feature('patches');\n-\tif ($patch_max && !defined $file_name) {\n+\tif ($patch_max && !defined $file_name &&\n+\t\t!gitweb_check_feature('email-privacy')) {\n \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n \t\t\t$paging_nav .= \" &sdot; \" .\n \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n@@ -7550,7 +7589,8 @@ sub git_commit {\n \t\t\t} @$parents ) .\n \t\t\t')';\n \t}\n-\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n+\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n+\t\t!gitweb_check_feature('email-privacy')) {\n \t\t$formats_nav .= \" | \" .\n \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n \t\t\t\t\"patch\");\n@@ -7863,7 +7903,8 @@ sub git_commitdiff {\n \t\t$formats_nav =\n \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n \t\t\t        \"raw\");\n-\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n+\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n+\t\t\t!gitweb_check_feature('email-privacy')) {\n \t\t\t$formats_nav .= \" | \" .\n \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n \t\t\t\t\t\"patch\");\ndiff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh\nindex 1f32ca66ea51..77fc1298d4c6 100644\n--- a/t/lib-gitweb.sh\n+++ b/t/lib-gitweb.sh\n@@ -67,6 +67,9 @@ gitweb_run () {\n \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n \texport GITWEB_CONFIG\n \n+\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n+\texport PERL5LIB\n+\n \t# some of git commands write to STDERR on error, but this is not\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n\nbase-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n-- \ngitgitgadget\n"},{"id":"420329","messageId":"xmqqczvkoxzx.fsf@gitster.g","threadId":"55358","inReplyTo":"6f656f62497fb7c9322432b5eb151b86.squirrel@mail.kodaksys.org","subject":"Re: [PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-28T01:41:54Z","receivedAt":"2021-03-28T01:42:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n\n>> I'll defer to others who are more familiar with gitweb and Perl\n>> ecosystem if this is warranted, but I have a feeling that importing\n>> and using Mail::Address->parse() only because we want to see if a\n>> given \"<string>\" is an address is a bit overkill and it might be\n>> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i\n>> ...\n>>> +\twhile ($line =~ m/(<[^>]+>)/g) {\n>>> +\t\tmy $match = $1;\n>>> +\t\tif (!is_mailaddr($match)) {\n>>> +\t\t\tnext;\n>>> +\t\t}\n>>> +\t\tmy $offset = pos $line;\n>>> +\t\tmy $head = substr $line, 0, $offset - length($match);\n>>> +\t\tmy $redaction = \"<redacted>\";\n>>> +\t\tmy $tail = substr $line, $offset;\n>>> +\t\t$line = $head . $redaction . $tail;\n>>> +\t\tpos $line = length($head) + length($redaction);\n>>\n>> Hmmmm, Perl suggestions from others?  It looks quite strange to see\n>> that s/// operator is not used and replacement is done manually with\n>> byte position in a Perl script.\n>>\n> If there's a more elegant way to do the above we can certain do that instead.\n\nFor example, if we do not insist on using overkill Mail::Address->parse(),\nwe could do something silly like this:\n\n\t$line =~ s/<[^@>]+@[a-z0-9-.]+>/<redacted@address>/ig;\n\nno?\n"},{"id":"420422","messageId":"14ebc48b8b7bee23dbcf19942f1f9029.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"xmqqczvkoxzx.fsf@gitster.g","subject":"Re: [PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-28T21:43:51Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> \"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n>\n>>> I'll defer to others who are more familiar with gitweb and Perl\n>>> ecosystem if this is warranted, but I have a feeling that importing\n>>> and using Mail::Address->parse() only because we want to see if a\n>>> given \"<string>\" is an address is a bit overkill and it might be\n>>> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i\n>>> ...\n>>>> +\twhile ($line =~ m/(<[^>]+>)/g) {\n>>>> +\t\tmy $match = $1;\n>>>> +\t\tif (!is_mailaddr($match)) {\n>>>> +\t\t\tnext;\n>>>> +\t\t}\n>>>> +\t\tmy $offset = pos $line;\n>>>> +\t\tmy $head = substr $line, 0, $offset - length($match);\n>>>> +\t\tmy $redaction = \"<redacted>\";\n>>>> +\t\tmy $tail = substr $line, $offset;\n>>>> +\t\t$line = $head . $redaction . $tail;\n>>>> +\t\tpos $line = length($head) + length($redaction);\n>>>\n>>> Hmmmm, Perl suggestions from others?  It looks quite strange to see\n>>> that s/// operator is not used and replacement is done manually with\n>>> byte position in a Perl script.\n>>>\n>> If there's a more elegant way to do the above we can certain do that\n>> instead.\n>\n> For example, if we do not insist on using overkill Mail::Address->parse(),\n> we could do something silly like this:\n>\n> \t$line =~ s/<[^@>]+@[a-z0-9-.]+>/<redacted@address>/ig;\n>\n> no?\n>\nIt's not clear if you think it's overkill because we have to depend on an\nexternal module or because we don't need accurate parsing.\n\nThe above expression seems to get the job done but it isn't the correct\nway to parse an e-mail address.\nBut if it's good enough for the reviewers I don't feel strongly about it.\n\nIf we prefer accurate parsing but don't like depending on Mail::Address,\nit's easy to write complete expressions ourselves. (In a separate,\ninternal, Perl module)\n\nThoughts?\n\n\n"},{"id":"420423","messageId":"xmqqft0elxei.fsf@gitster.g","threadId":"55358","inReplyTo":"14ebc48b8b7bee23dbcf19942f1f9029.squirrel@mail.kodaksys.org","subject":"Re: [PATCH v4] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-28T22:35:17Z","receivedAt":"2021-03-28T22:36:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis\" <geko1702+commits@99rst.org> writes:\n\n> It's not clear if you think it's overkill because we have to depend on an\n> external module or because we don't need accurate parsing.\n\nIt is neither; what we need to parse is not exactly 'e-mail\naddresses' as known to Mail::Address.\n\nThe thing is, unlike send-email that needs to interact with the\nreal-world MTAs and e-mail addresses, the codepaths we are talking\nabout are mostly about author/committer ident, where the definition\nis quite narrower than the Mail::Address's \"has to cover all the\npossible ways to spell e-mail addresses under the sun\" requirement.\n\nHaving said all that ...\n\n> If we prefer accurate parsing but don't like depending on Mail::Address,\n> it's easy to write complete expressions ourselves. (In a separate,\n> internal, Perl module)\n\n... I do not have strong opinions either way.  I won't have the\nfinal say on the way things are done in Perl and what is done to\nGitweb.\n\nIt was just that what I saw earlier with the offsets and manual\nparsing instead of s///g operator did not smell not like a Perl\nprogram to me, and the message you are responding to was my reaction\nto it.\n\nThanks.\n"},{"id":"420425","messageId":"pull.910.v6.git.1616973963862.gitgitgadget@gmail.com","threadId":"55358","inReplyTo":"pull.910.v5.git.1616817387441.gitgitgadget@gmail.com","subject":"[PATCH v6] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-28T23:26:03Z","receivedAt":"2021-03-28T23:27:41Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"From: Georgios Kontaxis <geko1702+commits@99rst.org>\n\nGitweb extracts content from the Git log and makes it accessible\nover HTTP. As a result, e-mail addresses found in commits are\nexposed to web crawlers and they may not respect robots.txt.\nThis can result in unsolicited messages.\n\nIntroduce an 'email-privacy' feature which redacts e-mail addresses\nfrom the generated HTML content. Specifically, obscure addresses\nretrieved from the the author/committer and comment sections of the\nGit log. The feature is off by default.\n\nThis feature does not prevent someone from downloading the\nunredacted commit log, e.g., by cloning the repository, and\nextracting information from it. It aims to hinder the low-\neffort, bulk collection of e-mail addresses by web crawlers.\n\nSigned-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n---\n    gitweb: redacted e-mail addresses feature.\n    \n    Changes since v1:\n    \n     * Turned off the feature by default.\n     * Removed duplicate code.\n     * Added note about Gitweb consumers receiving redacted logs.\n    \n    Changes since v2:\n    \n     * The feature can be set on a per-project basis. ('override' => 1)\n    \n    Changes since v3:\n    \n     * Renamed feature to \"email-privacy\" and improved documentation.\n     * Removed UI elements for git-format-patch since it won't be redacted.\n     * Simplified calls to the address redaction logic.\n     * Mail::Address is now used to reduce false-positive redactions.\n    \n    Changes since v4:\n    \n     * Rephrased the commit comment.\n     * hide_mailaddrs_if_private is slighly more compact.\n    \n    Changes since v5:\n    \n     * A simple <local@domain> filter is used instead of Mail::Address to\n       identify addresses.\n    \n    Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/910\n\nRange-diff vs v5:\n\n 1:  1427231f9db5 ! 1:  245cfed8ad58 gitweb: redacted e-mail addresses feature.\n     @@ Documentation/gitweb.conf.txt: default font sizes or lineheights are changed (e.\n       \t`$highlight_bin` program to be available (see the description of\n      \n       ## gitweb/gitweb.perl ##\n     -@@\n     - use File::Basename qw(basename);\n     - use Time::HiRes qw(gettimeofday tv_interval);\n     - use Digest::MD5 qw(md5_hex);\n     -+use Git::LoadCPAN::Mail::Address;\n     - \n     - binmode STDOUT, ':utf8';\n     - \n      @@ gitweb/gitweb.perl: sub evaluate_uri {\n       \t\t'sub' => \\&feature_extra_branch_refs,\n       \t\t'override' => 0,\n     @@ gitweb/gitweb.perl: sub parse_date {\n       \treturn %date;\n       }\n       \n     -+sub is_mailaddr {\n     -+\tmy @addrs = Mail::Address->parse(shift);\n     -+\tif (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {\n     -+\t\treturn 0;\n     -+\t}\n     -+\treturn 1;\n     -+}\n     -+\n      +sub hide_mailaddrs_if_private {\n      +\tmy $line = shift;\n      +\treturn $line unless gitweb_check_feature('email-privacy');\n     -+\twhile ($line =~ m/(<[^>]+>)/g) {\n     -+\t\tmy $match = $1;\n     -+\t\tif (!is_mailaddr($match)) {\n     -+\t\t\tnext;\n     -+\t\t}\n     -+\t\tmy $match_offset = pos($line) - length($match);\n     -+\t\tpos $line = $match_offset;\n     -+\n     -+\t\tmy $redaction = \"<redacted>\";\n     -+\t\t$line =~ s/\\G(<[^>]+>)/$redaction/;\n     -+\n     -+\t\tpos $line = $match_offset + length($redaction);\n     -+\t}\n     ++\t$line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;\n      +\treturn $line;\n      +}\n      +\n     @@ gitweb/gitweb.perl: sub git_commitdiff {\n       \t\t\t$formats_nav .= \" | \" .\n       \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n       \t\t\t\t\t\"patch\");\n     -\n     - ## t/lib-gitweb.sh ##\n     -@@ t/lib-gitweb.sh: gitweb_run () {\n     - \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n     - \texport GITWEB_CONFIG\n     - \n     -+\tPERL5LIB=\"$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN\"\n     -+\texport PERL5LIB\n     -+\n     - \t# some of git commands write to STDERR on error, but this is not\n     - \t# written to web server logs, so we are not interested in that:\n     - \t# we are interested only in properly formatted errors/warnings\n\n\n Documentation/gitweb.conf.txt | 11 +++++++++++\n gitweb/gitweb.perl            | 34 +++++++++++++++++++++++++++-------\n 2 files changed, 38 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex 7963a79ba98b..34b1d6e22435 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -751,6 +751,17 @@ default font sizes or lineheights are changed (e.g. via adding extra\n CSS stylesheet in `@stylesheets`), it may be appropriate to change\n these values.\n \n+email-privacy::\n+\tRedact e-mail addresses from the generated HTML, etc. content.\n+\tThis obscures e-mail addresses retrieved from the author/committer\n+\tand comment sections of the Git log.\n+\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n+\tSuch crawlers may not respect robots.txt.\n+\tNote that users and user tools also see the addresses as redacted.\n+\tIf Gitweb is not the final step in a workflow then subsequent steps\n+\tmay misbehave because of the redacted information they receive.\n+\tDisabled by default.\n+\n highlight::\n \tServer-side syntax highlight support in \"blob\" view.  It requires\n \t`$highlight_bin` program to be available (see the description of\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0959a782eccb..01c6faf88006 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -569,6 +569,15 @@ sub evaluate_uri {\n \t\t'sub' => \\&feature_extra_branch_refs,\n \t\t'override' => 0,\n \t\t'default' => []},\n+\n+\t# Redact e-mail addresses.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'email-privacy'}{'default'} = [1];\n+\t'email-privacy' => {\n+\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n+\t\t'override' => 1,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -3449,6 +3458,13 @@ sub parse_date {\n \treturn %date;\n }\n \n+sub hide_mailaddrs_if_private {\n+\tmy $line = shift;\n+\treturn $line unless gitweb_check_feature('email-privacy');\n+\t$line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;\n+\treturn $line;\n+}\n+\n sub parse_tag {\n \tmy $tag_id = shift;\n \tmy %tag;\n@@ -3465,7 +3481,7 @@ sub parse_tag {\n \t\t} elsif ($line =~ m/^tag (.+)$/) {\n \t\t\t$tag{'name'} = $1;\n \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$tag{'author'} = $1;\n+\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n \t\t\t$tag{'author_epoch'} = $2;\n \t\t\t$tag{'author_tz'} = $3;\n \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3513,7 +3529,7 @@ sub parse_commit_text {\n \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {\n \t\t\tpush @parents, $1;\n \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'author'} = to_utf8($1);\n+\t\t\t$co{'author'} = hide_mailaddrs_if_private(to_utf8($1));\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3523,7 +3539,7 @@ sub parse_commit_text {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'committer'} = to_utf8($1);\n+\t\t\t$co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -3568,9 +3584,10 @@ sub parse_commit_text {\n \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n \t}\n-\t# remove added spaces\n+\t# remove added spaces, redact e-mail addresses if applicable.\n \tforeach my $line (@commit_lines) {\n \t\t$line =~ s/^    //;\n+\t\t$line = hide_mailaddrs_if_private($line);\n \t}\n \t$co{'comment'} = \\@commit_lines;\n \n@@ -7489,7 +7506,8 @@ sub git_log_generic {\n \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n \t}\n \tmy $patch_max = gitweb_get_feature('patches');\n-\tif ($patch_max && !defined $file_name) {\n+\tif ($patch_max && !defined $file_name &&\n+\t\t!gitweb_check_feature('email-privacy')) {\n \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n \t\t\t$paging_nav .= \" &sdot; \" .\n \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n@@ -7550,7 +7568,8 @@ sub git_commit {\n \t\t\t} @$parents ) .\n \t\t\t')';\n \t}\n-\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n+\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n+\t\t!gitweb_check_feature('email-privacy')) {\n \t\t$formats_nav .= \" | \" .\n \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n \t\t\t\t\"patch\");\n@@ -7863,7 +7882,8 @@ sub git_commitdiff {\n \t\t$formats_nav =\n \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n \t\t\t        \"raw\");\n-\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n+\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n+\t\t\t!gitweb_check_feature('email-privacy')) {\n \t\t\t$formats_nav .= \" | \" .\n \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n \t\t\t\t\t\"patch\");\n\nbase-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f\n-- \ngitgitgadget\n"},{"id":"420426","messageId":"20210329014744.GA2813@dcvr","threadId":"55358","inReplyTo":"pull.910.v5.git.1616817387441.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2021-03-29T01:47:44Z","receivedAt":"2021-03-29T01:48:19Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:\n> Gitweb extracts content from the Git log and makes it accessible\n> over HTTP. As a result, e-mail addresses found in commits are\n> exposed to web crawlers and they may not respect robots.txt.\n> This can result in unsolicited messages.\n\n> Introduce an 'email-privacy' feature which redacts e-mail addresses\n> from the generated HTML content\n\nA general reply to the topic: have you considered munging\naddresses in a way that is still human readable, but obviously\nobfuscated?\n\nOn some other project, I settled on HTML \"&#8226;\" as a replacement\nfor '.' for admins who enable that option.  The $USER@$NO_DOT\nremains as-is for easy identification+recognition of hosts.\n\nI also considered Unicode homographs which can look identical\nto replacement characters, too; but rejected that idea since\nit would cause grief for legitimate users who would not notice\nthe homograph when pasting into their mail client.\n\nAnyways, here's the list of candidates I tried:\n\nhomograph∂80x24.org\nhomograph@80x24ͺorg\nhomograph@80x24·org\nhomograph@80x24•org\nhomograph＠80x24.org\nhomograph﹫80x24.org\n\nhttps://en.wikipedia.org/wiki/Ano_Teleia#Similar_symbols\nhttps://en.wikipedia.org/wiki/Enclosed_A\n\nhomographⒶ80x24.org\nhomograph@80x24 org\nhomograph@80x24․org\nhomograph@80x24ꓸorg\n"},{"id":"420429","messageId":"8330ef0d7195de461f961d72f90998fa.squirrel@mail.kodaksys.org","threadId":"55358","inReplyTo":"20210329014744.GA2813@dcvr","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Georgios Kontaxis","fromEmail":"geko1702+commits@99rst.org","sentAt":null,"receivedAt":"2021-03-29T03:18:24Z","isPatch":true,"sender":{"key":"geko1702+commits@99rst.org","avatar":"https://avatars.githubusercontent.com/u/6628990?v=4"},"body":"> Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers and they may not respect robots.txt.\n>> This can result in unsolicited messages.\n>\n>> Introduce an 'email-privacy' feature which redacts e-mail addresses\n>> from the generated HTML content\n>\n> A general reply to the topic: have you considered munging\n> addresses in a way that is still human readable, but obviously\n> obfuscated?\n>\n> On some other project, I settled on HTML \"&#8226;\" as a replacement\n> for '.' for admins who enable that option.  The $USER@$NO_DOT\n> remains as-is for easy identification+recognition of hosts.\n>\nThanks for the suggestion.\n\nPeople have been trying to hinder address harvesting for a while now.\nReplacing '@' with \"at\", the dot with \"dot\", adding spaces, etc.\nwas pretty common at some point. May still be.\nI would expect crawlers to have caught up and this includes\nall sorts of character encodings and unicode look-alike substitutions.\n\nAt the end of the day we are looking for something that's easy for humans\nto read but hard for scripts to parse as an e-mail address.\n(And that scripts cannot learn through an additional regex)\nI'm not aware of anything like that. (I know CAPTCHAs, etc.)\n\n> I also considered Unicode homographs which can look identical\n> to replacement characters, too; but rejected that idea since\n> it would cause grief for legitimate users who would not notice\n> the homograph when pasting into their mail client.\n>\n> Anyways, here's the list of candidates I tried:\n>\n> homograph∂80x24.org\n> homograph@80x24ͺorg\n> homograph@80x24·org\n> homograph@80x24•org\n> homograph＠80x24.org\n> homograph﹫80x24.org\n>\n> https://en.wikipedia.org/wiki/Ano_Teleia#Similar_symbols\n> https://en.wikipedia.org/wiki/Enclosed_A\n>\n> homographⒶ80x24.org\n> homograph@80x24 org\n> homograph@80x24․org\n> homograph@80x24ꓸorg\n>\n\n\n"},{"id":"420506","messageId":"xmqq5z19k9wu.fsf@gitster.g","threadId":"55358","inReplyTo":"pull.910.v6.git.1616973963862.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T20:00:17Z","receivedAt":"2021-03-29T20:01:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>\n> Gitweb extracts content from the Git log and makes it accessible\n> over HTTP. As a result, e-mail addresses found in commits are\n> exposed to web crawlers and they may not respect robots.txt.\n> This can result in unsolicited messages.\n>\n> Introduce an 'email-privacy' feature which redacts e-mail addresses\n> from the generated HTML content. Specifically, obscure addresses\n> retrieved from the the author/committer and comment sections of the\n> Git log. The feature is off by default.\n>\n> This feature does not prevent someone from downloading the\n> unredacted commit log, e.g., by cloning the repository, and\n> extracting information from it. It aims to hinder the low-\n> effort, bulk collection of e-mail addresses by web crawlers.\n>\n> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>\n> ---\n\n> @@ -751,6 +751,17 @@ default font sizes or lineheights are changed (e.g. via adding extra\n>  CSS stylesheet in `@stylesheets`), it may be appropriate to change\n>  these values.\n>  \n> +email-privacy::\n> +\tRedact e-mail addresses from the generated HTML, etc. content.\n> +\tThis obscures e-mail addresses retrieved from the author/committer\n> +\tand comment sections of the Git log.\n> +\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n> +\tSuch crawlers may not respect robots.txt.\n> +\tNote that users and user tools also see the addresses as redacted.\n> +\tIf Gitweb is not the final step in a workflow then subsequent steps\n> +\tmay misbehave because of the redacted information they receive.\n> +\tDisabled by default.\n\nOK.  I still think everything after \"Note that\" is a bit redandant\n(as any intelligent reader can read what the feature does and reach\nits natural consequence themselves), but I do not strongly oppose\nleaving it in.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0959a782eccb..01c6faf88006 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -569,6 +569,15 @@ sub evaluate_uri {\n>  \t\t'sub' => \\&feature_extra_branch_refs,\n>  \t\t'override' => 0,\n>  \t\t'default' => []},\n> +\n> +\t# Redact e-mail addresses.\n> +\n> +\t# To enable system wide have in $GITWEB_CONFIG\n> +\t# $feature{'email-privacy'}{'default'} = [1];\n> +\t'email-privacy' => {\n> +\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n> +\t\t'override' => 1,\n> +\t\t'default' => [0]},\n>  );\n\nSensible.\n\nWhile reviewing this part, I noticed a few things\n\n - A few other features explain how to toggle it system wide and\n   nothing else, like this one does.\n\n - Others explain, in addition, talk about how to toggle per\n   repository (which may necessitate explaining how to toggle the\n   override bit if the default is false).\n\nIt might be a good idea to standardise the explanation somehow.\n\nThis is just an observation (in other words, nothing needs to be\ndone in this patch).\n\n\n> @@ -3449,6 +3458,13 @@ sub parse_date {\n>  \treturn %date;\n>  }\n>  \n> +sub hide_mailaddrs_if_private {\n> +\tmy $line = shift;\n> +\treturn $line unless gitweb_check_feature('email-privacy');\n> +\t$line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;\n> +\treturn $line;\n> +}\n\nOK.  The \"this catches AUTHOR_EMAIL\" pattern is a bit embarrassing\n(my fault); \"s/<[^@>]+@[-a-z0-9.]+>/<redacted>/ig\" might be less\nembarrassing but I do not care deeply enough either way.  The version\nin this patch is probably closer to what ident.c uses anyway ;-)\n\n> @@ -7489,7 +7506,8 @@ sub git_log_generic {\n>  \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n>  \t}\n>  \tmy $patch_max = gitweb_get_feature('patches');\n> -\tif ($patch_max && !defined $file_name) {\n> +\tif ($patch_max && !defined $file_name &&\n> +\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n\nAn observation unrelated to this change.  I think checking for\nnegative patch_max is a bug in the original code.  Everywhere else,\nthe way to disable the 'patch' view is to set it to 0, not negative,\nand this block is already protected with \"if ($patch_max\".\n\n> @@ -7550,7 +7568,8 @@ sub git_commit {\n>  \t\t\t} @$parents ) .\n>  \t\t\t')';\n>  \t}\n> -\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n> +\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n> +\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\t$formats_nav .= \" | \" .\n>  \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>  \t\t\t\t\"patch\");\n> @@ -7863,7 +7882,8 @@ sub git_commitdiff {\n>  \t\t$formats_nav =\n>  \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n>  \t\t\t        \"raw\");\n> -\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n> +\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n> +\t\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\t\t$formats_nav .= \" | \" .\n>  \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>  \t\t\t\t\t\"patch\");\n\nI see your wish is \"we do not show any link to a patch page, but\nanybody who knows what a link to the patch page looks like can craft\nand ask\", but I am not sure if that is worth the above three hunks.\nIt still feels more robust to just disable the 'patches' feature\nwhen the email-privacy feature is enabled, but that may be just me.\n\nWill queue as-is.  Input from those who are more adept at Perl\nand/or interested in helping polish gitweb are still welcome, but at\nmy level of interest on the topic, this version looks as good as it\ngets ;-)\n\nThanks.\n"},{"id":"420748","messageId":"xmqqa6qj6n5g.fsf@gitster.g","threadId":"55358","inReplyTo":"xmqq5z19k9wu.fsf@gitster.g","subject":"Re: [PATCH v6] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-31T21:14:51Z","receivedAt":"2021-03-31T21:15:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Will queue as-is.  Input from those who are more adept at Perl\n> and/or interested in helping polish gitweb are still welcome, but at\n> my level of interest on the topic, this version looks as good as it\n> gets ;-)\n\nOh by the way, as nobody commented on the title, I'd have to X-<.\n\nI'll retitle it to\n\n\tgitweb: add \"e-mail privacy\" feature to redact e-mail addresses\n\nwhile queuing.\n\nThanks.\n"},{"id":"421033","messageId":"xmqq35w444dp.fsf@gitster.g","threadId":"55358","inReplyTo":"xmqq5z19k9wu.fsf@gitster.g","subject":"Re: [PATCH v6] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-06T00:56:50Z","receivedAt":"2021-04-06T00:56:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Georgios Kontaxis via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n>> From: Georgios Kontaxis <geko1702+commits@99rst.org>\n>>\n>> Gitweb extracts content from the Git log and makes it accessible\n>> over HTTP. As a result, e-mail addresses found in commits are\n>> exposed to web crawlers and they may not respect robots.txt.\n>> This can result in unsolicited messages.\n>> ...\n> Will queue as-is.  Input from those who are more adept at Perl\n> and/or interested in helping polish gitweb are still welcome, but at\n> my level of interest on the topic, this version looks as good as it\n> gets ;-)\n\nComments by anybody who is more adept at Perl and/or more well\nversed in gitweb and e-mail privacy issues?\n\nOtherwise I am inclined to merge it down to 'next' as-is in a couple\nof days.\n\nThanks.\n"},{"id":"421243","messageId":"20210408171648.GA7133@dcvr","threadId":"55358","inReplyTo":"8330ef0d7195de461f961d72f90998fa.squirrel@mail.kodaksys.org","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2021-04-08T17:16:48Z","receivedAt":"2021-04-08T17:16:50Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Georgios Kontaxis <geko1702+commits@99rst.org> wrote:\n> > Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:\n> >> Introduce an 'email-privacy' feature which redacts e-mail addresses\n> >> from the generated HTML content\n> >\n> Eric Wong wrote:\n> > A general reply to the topic: have you considered munging\n> > addresses in a way that is still human readable, but obviously\n> > obfuscated?\n> >\n> > On some other project, I settled on HTML \"&#8226;\" as a replacement\n> > for '.' for admins who enable that option.  The $USER@$NO_DOT\n> > remains as-is for easy identification+recognition of hosts.\n> >\n> Thanks for the suggestion.\n> \n> People have been trying to hinder address harvesting for a while now.\n> Replacing '@' with \"at\", the dot with \"dot\", adding spaces, etc.\n> was pretty common at some point. May still be.\n> I would expect crawlers to have caught up and this includes\n> all sorts of character encodings and unicode look-alike substitutions.\n\nI figure the crawlers hit a combinatorial explosion and\ngive up since they'd be wasting time with false-positives.\n\n> > I also considered Unicode homographs which can look identical\n> > to replacement characters, too; but rejected that idea since\n> > it would cause grief for legitimate users who would not notice\n> > the homograph when pasting into their mail client.\n\nAs a data point, none of the homograph@ candidates I posted here\non Mar 29 have attracted any attempts on my mail server.\n"},{"id":"421271","messageId":"xmqqmtu8sd2a.fsf@gitster.g","threadId":"55358","inReplyTo":"20210408171648.GA7133@dcvr","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-08T21:04:29Z","receivedAt":"2021-04-08T21:04:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <e@80x24.org> writes:\n\n> Georgios Kontaxis <geko1702+commits@99rst.org> wrote:\n>> > Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:\n>> >> Introduce an 'email-privacy' feature which redacts e-mail addresses\n>> >> from the generated HTML content\n>> >\n>> Eric Wong wrote:\n>> > A general reply to the topic: have you considered munging\n>> > addresses in a way that is still human readable, but obviously\n>> > obfuscated?\n>> ...\n>> > I also considered Unicode homographs which can look identical\n>> > to replacement characters, too; but rejected that idea since\n>> > it would cause grief for legitimate users who would not notice\n>> > the homograph when pasting into their mail client.\n>\n> As a data point, none of the homograph@ candidates I posted here\n> on Mar 29 have attracted any attempts on my mail server.\n\nThat is an interesting observation.  All homograph@ non-addresses,\nif a human corrected the funnies in their spelling, would have hit\nwhoever handles @80x24.org mailboxes.\n\nI take it to mean that as a future direction, replacing <redacted>\nwith the obfuscated-but-readable-by-humans homographs is a likely\nimprovement that would help human users while still inconveniencing\nthe crawlers.  It may however need some provision to prevent casual\nend-users from cutting-and-pasting these homographs, as you said in\nyour original mention of the homograph approach.\n\nBut other than that, does the patch look reasonable?\n\nThanks.\n"},{"id":"421273","messageId":"20210408211904.GA16486@dcvr","threadId":"55358","inReplyTo":"xmqqmtu8sd2a.fsf@gitster.g","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2021-04-08T21:19:04Z","receivedAt":"2021-04-08T21:19:06Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Wong <e@80x24.org> writes:\n> > As a data point, none of the homograph@ candidates I posted here\n> > on Mar 29 have attracted any attempts on my mail server.\n> \n> That is an interesting observation.  All homograph@ non-addresses,\n> if a human corrected the funnies in their spelling, would have hit\n> whoever handles @80x24.org mailboxes.\n> \n> I take it to mean that as a future direction, replacing <redacted>\n> with the obfuscated-but-readable-by-humans homographs is a likely\n> improvement that would help human users while still inconveniencing\n> the crawlers.  It may however need some provision to prevent casual\n> end-users from cutting-and-pasting these homographs, as you said in\n> your original mention of the homograph approach.\n\nYes, exactly.\n\n> But other than that, does the patch look reasonable?\n\nI only took a cursory glance at it, but v6 seemed fine.\n"},{"id":"421287","messageId":"87eefkieig.fsf@evledraar.gmail.com","threadId":"55358","inReplyTo":"pull.910.v6.git.1616973963862.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] gitweb: redacted e-mail addresses feature.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-08T22:43:19Z","receivedAt":"2021-04-08T22:43:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 29 2021, Georgios Kontaxis via GitGitGadget wrote:\n\n> [...]\n> +email-privacy::\n> +\tRedact e-mail addresses from the generated HTML, etc. content.\n> +\tThis obscures e-mail addresses retrieved from the author/committer\n> +\tand comment sections of the Git log.\n> +\tIt is meant to hinder web crawlers that harvest and abuse addresses.\n> +\tSuch crawlers may not respect robots.txt.\n> +\tNote that users and user tools also see the addresses as redacted.\n> +\tIf Gitweb is not the final step in a workflow then subsequent steps\n> +\tmay misbehave because of the redacted information they receive.\n> +\tDisabled by default.\n> +\n>  highlight::\n>  \tServer-side syntax highlight support in \"blob\" view.  It requires\n>  \t`$highlight_bin` program to be available (see the description of\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0959a782eccb..01c6faf88006 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -569,6 +569,15 @@ sub evaluate_uri {\n>  \t\t'sub' => \\&feature_extra_branch_refs,\n>  \t\t'override' => 0,\n>  \t\t'default' => []},\n> +\n> +\t# Redact e-mail addresses.\n> +\n> +\t# To enable system wide have in $GITWEB_CONFIG\n> +\t# $feature{'email-privacy'}{'default'} = [1];\n> +\t'email-privacy' => {\n> +\t\t'sub' => sub { feature_bool('email-privacy', @_) },\n> +\t\t'override' => 1,\n> +\t\t'default' => [0]},\n>  );\n>  \n>  sub gitweb_get_feature {\n> @@ -3449,6 +3458,13 @@ sub parse_date {\n>  \treturn %date;\n>  }\n>  \n> +sub hide_mailaddrs_if_private {\n> +\tmy $line = shift;\n> +\treturn $line unless gitweb_check_feature('email-privacy');\n> +\t$line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;\n\nThe /i here is redundant, since you have nothing that'll case-fold on\nthe LHS of the s///, doesn't harm anything either. Just a small note\nsince it's new in v6...\n\n> +\treturn $line;\n> +}\n> +\n>  sub parse_tag {\n>  \tmy $tag_id = shift;\n>  \tmy %tag;\n> @@ -3465,7 +3481,7 @@ sub parse_tag {\n>  \t\t} elsif ($line =~ m/^tag (.+)$/) {\n>  \t\t\t$tag{'name'} = $1;\n>  \t\t} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {\n> -\t\t\t$tag{'author'} = $1;\n> +\t\t\t$tag{'author'} = hide_mailaddrs_if_private($1);\n>  \t\t\t$tag{'author_epoch'} = $2;\n>  \t\t\t$tag{'author_tz'} = $3;\n>  \t\t\tif ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n> @@ -3513,7 +3529,7 @@ sub parse_commit_text {\n>  \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {\n>  \t\t\tpush @parents, $1;\n>  \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n> -\t\t\t$co{'author'} = to_utf8($1);\n> +\t\t\t$co{'author'} = hide_mailaddrs_if_private(to_utf8($1));\n>  \t\t\t$co{'author_epoch'} = $2;\n>  \t\t\t$co{'author_tz'} = $3;\n>  \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n> @@ -3523,7 +3539,7 @@ sub parse_commit_text {\n>  \t\t\t\t$co{'author_name'} = $co{'author'};\n>  \t\t\t}\n>  \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n> -\t\t\t$co{'committer'} = to_utf8($1);\n> +\t\t\t$co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));\n>  \t\t\t$co{'committer_epoch'} = $2;\n>  \t\t\t$co{'committer_tz'} = $3;\n>  \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n> @@ -3568,9 +3584,10 @@ sub parse_commit_text {\n>  \tif (! defined $co{'title'} || $co{'title'} eq \"\") {\n>  \t\t$co{'title'} = $co{'title_short'} = '(no commit message)';\n>  \t}\n> -\t# remove added spaces\n> +\t# remove added spaces, redact e-mail addresses if applicable.\n>  \tforeach my $line (@commit_lines) {\n>  \t\t$line =~ s/^    //;\n> +\t\t$line = hide_mailaddrs_if_private($line);\n>  \t}\n>  \t$co{'comment'} = \\@commit_lines;\n>  \n> @@ -7489,7 +7506,8 @@ sub git_log_generic {\n>  \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n>  \t}\n>  \tmy $patch_max = gitweb_get_feature('patches');\n> -\tif ($patch_max && !defined $file_name) {\n> +\tif ($patch_max && !defined $file_name &&\n> +\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\tif ($patch_max < 0 || @commitlist <= $patch_max) {\n>  \t\t\t$paging_nav .= \" &sdot; \" .\n>  \t\t\t\t$cgi->a({-href => href(action=>\"patches\", -replay=>1)},\n> @@ -7550,7 +7568,8 @@ sub git_commit {\n>  \t\t\t} @$parents ) .\n>  \t\t\t')';\n>  \t}\n> -\tif (gitweb_check_feature('patches') && @$parents <= 1) {\n> +\tif (gitweb_check_feature('patches') && @$parents <= 1 &&\n> +\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\t$formats_nav .= \" | \" .\n>  \t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>  \t\t\t\t\"patch\");\n> @@ -7863,7 +7882,8 @@ sub git_commitdiff {\n>  \t\t$formats_nav =\n>  \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n>  \t\t\t        \"raw\");\n> -\t\tif ($patch_max && @{$co{'parents'}} <= 1) {\n> +\t\tif ($patch_max && @{$co{'parents'}} <= 1 &&\n> +\t\t\t!gitweb_check_feature('email-privacy')) {\n>  \t\t\t$formats_nav .= \" | \" .\n>  \t\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n>  \t\t\t\t\t\"patch\");\n\nI didn't run this, and hadn't kept up for a few rounds. I'm happy to see\nthe pos/while etc. looping gone, this LGTM.\n"},{"id":"421288","messageId":"87blaoieef.fsf@evledraar.gmail.com","threadId":"55358","inReplyTo":"20210408211904.GA16486@dcvr","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-08T22:45:44Z","receivedAt":"2021-04-08T22:45:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 08 2021, Eric Wong wrote:\n\n> Junio C Hamano <gitster@pobox.com> wrote:\n>> Eric Wong <e@80x24.org> writes:\n>> > As a data point, none of the homograph@ candidates I posted here\n>> > on Mar 29 have attracted any attempts on my mail server.\n>> \n>> That is an interesting observation.  All homograph@ non-addresses,\n>> if a human corrected the funnies in their spelling, would have hit\n>> whoever handles @80x24.org mailboxes.\n>> \n>> I take it to mean that as a future direction, replacing <redacted>\n>> with the obfuscated-but-readable-by-humans homographs is a likely\n>> improvement that would help human users while still inconveniencing\n>> the crawlers.  It may however need some provision to prevent casual\n>> end-users from cutting-and-pasting these homographs, as you said in\n>> your original mention of the homograph approach.\n>\n> Yes, exactly.\n>\n>> But other than that, does the patch look reasonable?\n>\n> I only took a cursory glance at it, but v6 seemed fine.\n\nDitto, I left a small nit comment about a needless /i in a regex, but I\ndon't think that needs a re-roll.\n"},{"id":"421290","messageId":"xmqqblaoqtjf.fsf@gitster.g","threadId":"55358","inReplyTo":"87eefkieig.fsf@evledraar.gmail.com","subject":"Re: [PATCH v6] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-08T22:51:32Z","receivedAt":"2021-04-08T22:51:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> +sub hide_mailaddrs_if_private {\n>> +\tmy $line = shift;\n>> +\treturn $line unless gitweb_check_feature('email-privacy');\n>> +\t$line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;\n>\n> The /i here is redundant, since you have nothing that'll case-fold on\n> the LHS of the s///, doesn't harm anything either. Just a small note\n> since it's new in v6...\n\nTrue.  If it were left original version that was suggested during\nthe review, e.g.\n\n\ts/<[^@]+@[-.a-z0-9]+>/<red@cted>/ig;\n\nthe /i/ would have been needed, but without the \"after the @-sign\nshould be a run of DNS-valid characters\", I agree that there is no\nneed.\n\nThanks.  Will locally tweak.\n"},{"id":"421291","messageId":"xmqq7dlcqteq.fsf@gitster.g","threadId":"55358","inReplyTo":"87blaoieef.fsf@evledraar.gmail.com","subject":"Re: [PATCH v5] gitweb: redacted e-mail addresses feature.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-08T22:54:21Z","receivedAt":"2021-04-08T22:54:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Thu, Apr 08 2021, Eric Wong wrote:\n>\n>> Junio C Hamano <gitster@pobox.com> wrote:\n>>> Eric Wong <e@80x24.org> writes:\n>>> > As a data point, none of the homograph@ candidates I posted here\n>>> > on Mar 29 have attracted any attempts on my mail server.\n>>> \n>>> That is an interesting observation.  All homograph@ non-addresses,\n>>> if a human corrected the funnies in their spelling, would have hit\n>>> whoever handles @80x24.org mailboxes.\n>>> \n>>> I take it to mean that as a future direction, replacing <redacted>\n>>> with the obfuscated-but-readable-by-humans homographs is a likely\n>>> improvement that would help human users while still inconveniencing\n>>> the crawlers.  It may however need some provision to prevent casual\n>>> end-users from cutting-and-pasting these homographs, as you said in\n>>> your original mention of the homograph approach.\n>>\n>> Yes, exactly.\n>>\n>>> But other than that, does the patch look reasonable?\n>>\n>> I only took a cursory glance at it, but v6 seemed fine.\n>\n> Ditto, I left a small nit comment about a needless /i in a regex, but I\n> don't think that needs a re-roll.\n\nThanks, both.\n\nWill tweak the /i out, and re-queue with acked-by from you two.\n"}]}