* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 17:28 ` [PATCH v3] " Georgios Kontaxis via GitGitGadget
@ 2021-03-21 18:26 ` Ævar Arnfjörð Bjarmason
2021-03-21 18:48 ` Junio C Hamano
2021-03-21 19:48 ` Georgios Kontaxis
2021-03-21 18:42 ` Junio C Hamano
2021-03-22 6:57 ` [PATCH v4] " Georgios Kontaxis via GitGitGadget
2 siblings, 2 replies; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-03-21 18:26 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, brian m. carlson, Georgios Kontaxis
On Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:
> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>
> Gitweb extracts content from the Git log and makes it accessible
> over HTTP. As a result, e-mail addresses found in commits are
> exposed to web crawlers and they may not respect robots.txt.
> This may result in unsolicited messages.
> This is a feature for redacting e-mail addresses
> from the generated HTML, etc. content.
>
> This feature does not prevent someone from downloading the
> unredacted commit log, e.g., by cloning the repository, and
> extracting information from it.
> It aims to hinder the low-effort bulk collection of e-mail
> addresses by web crawlers.
>
> Changes since v1:
> - Turned off the feature by default.
> - Removed duplicate code.
> - Added note about Gitweb consumers receiving redacted logs.
>
> Changes since v2:
> - The feature can be set on a per-project basis. ('override' => 1)
>
> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
> ---
> gitweb: Redacted e-mail addresses feature.
>
> Gitweb extracts content from the Git log and makes it accessible over
> HTTP. As a result, e-mail addresses found in commits are exposed to web
> crawlers. This may result in unsolicited messages. This is a feature for
> redacting e-mail addresses from the generated HTML content.
>
> This feature does not prevent someone from downloading the unredacted
> commit log and extracting information from it. It aims to hinder the
> low-effort bulk collection of e-mail addresses by web crawlers.
>
> Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v3
> Pull-Request: https://github.com/gitgitgadget/git/pull/910
>
> Range-diff vs v2:
>
> 1: 74af11ca8bf2 ! 1: 930cdefe7ee0 gitweb: redacted e-mail addresses feature.
> @@ Commit message
> - Removed duplicate code.
> - Added note about Gitweb consumers receiving redacted logs.
>
> + Changes since v2:
> + - The feature can be set on a per-project basis. ('override' => 1)
> +
> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>
> ## Documentation/gitweb.conf.txt ##
> @@ gitweb/gitweb.perl: sub evaluate_uri {
> + # $feature{'email_privacy'}{'default'} = [1];
> + 'email_privacy' => {
> + 'sub' => sub { feature_bool('email_privacy', @_) },
> -+ 'override' => 0,
> ++ 'override' => 1,
> + 'default' => [0]},
> );
>
>
>
> Documentation/gitweb.conf.txt | 16 +++++++++++++
> gitweb/gitweb.perl | 42 ++++++++++++++++++++++++++++++++---
> 2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
> index 7963a79ba98b..b7af3240177d 100644
> --- a/Documentation/gitweb.conf.txt
> +++ b/Documentation/gitweb.conf.txt
> @@ -896,6 +896,22 @@ same as of the snippet above:
> It is an error to specify a ref that does not pass "git check-ref-format"
> scrutiny. Duplicated values are filtered.
>
> +email_privacy::
> + Redact e-mail addresses from the generated HTML, etc. content.
> + This hides e-mail addresses found in the commit log from web crawlers.
> + Disabled by default.
> ++
> +It is highly recommended to enable this feature unless web crawlers are
> +hindered in some other way. Note that crawlers intent on harvesting e-mail
> +addresses may disregard robots.txt. You can enable this feature like so:
> ++
> +---------------------------------------------------------------------------
> +$feature{'email_privacy'}{'default'} = [1];
> +---------------------------------------------------------------------------
> ++
> +Note that if Gitweb is not the final step in a workflow then subsequent
> +steps may misbehave because of the redacted information they receive.
> +
>
> EXAMPLES
> --------
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0959a782eccb..174cc566d97d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -569,6 +569,15 @@ sub evaluate_uri {
> 'sub' => \&feature_extra_branch_refs,
> 'override' => 0,
> 'default' => []},
> +
> + # Redact e-mail addresses.
> +
> + # To enable system wide have in $GITWEB_CONFIG
> + # $feature{'email_privacy'}{'default'} = [1];
> + 'email_privacy' => {
> + 'sub' => sub { feature_bool('email_privacy', @_) },
> + 'override' => 1,
> + 'default' => [0]},
> );
>
> sub gitweb_get_feature {
> @@ -3449,6 +3458,19 @@ sub parse_date {
> return %date;
> }
>
> [snip]
So in the v1 feedback I suggested:
BEGIN QUOTE
sub maybe_hide_email {
my $email = shift;
return $email unless gitweb_check_feature('email_privacy');
return hide_email($email);
}
then:
$tag{author_email} = maybe_hide_email($2);
END QUOTE
But:
> sub parse_tag {
> my $tag_id = shift;
> my %tag;
> @@ -3471,6 +3493,10 @@ sub parse_tag {
> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
> $tag{'author_name'} = $1;
> $tag{'author_email'} = $2;
> + if (gitweb_check_feature('email_privacy')) {
> + $tag{'author_email'} = "private";
> + $tag{'author'} = hide_mailaddr($tag{'author'});
> + }
This code seems quite awkward, we've already done the regex match, but
this code:
> [snip]
> +sub hide_mailaddr_if_private {
> + my $line = shift;
> + return $line unless (gitweb_check_feature('email_privacy') &&
> + $line =~ m/^([^<]+) <([^>]*)>/);
> + return hide_mailaddr($line)
> +}
> +
> +sub hide_mailaddr {
> + my $mailaddr = shift;
> + $mailaddr =~ s/<([^>]*)>/<private>/;
> + return $mailaddr;
> +}
Is going to do it again incrementally, and then just act on a
search-replacement if we've got the feature enabled.
It seems much simpler to just turn your:
> + if (gitweb_check_feature('email_privacy')) {
> + $tag{'author_email'} = "private";
> + $tag{'author'} = hide_mailaddr($tag{'author'});
> + }
Into:
$tag{'author'} = maybe_hide_mailaddr($tag{author}, \$tag{author_email});
Using:
sub maybe_hide_email {
my ($email, $ref) = shift;
return $email unless gitweb_check_feature('email_privacy');
$$ref = "private" if $ref;
return hide_email($email);
}
Which also works for the case where you don't have a "private" hash key
to assign to. But maybe it overcomplicates things...
> } else {
> $tag{'author_name'} = $tag{'author'};
> }
> @@ -3519,6 +3545,10 @@ sub parse_commit_text {
> if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
> $co{'author_name'} = $1;
> $co{'author_email'} = $2;
> + if (gitweb_check_feature('email_privacy')) {
> + $co{'author_email'} = "private";
> + $co{'author'} = hide_mailaddr($co{'author'});
> + }
> } else {
> $co{'author_name'} = $co{'author'};
> }
> @@ -3529,6 +3559,10 @@ sub parse_commit_text {
> if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
> $co{'committer_name'} = $1;
> $co{'committer_email'} = $2;
> + if (gitweb_check_feature('email_privacy')) {
> + $co{'committer_email'} = "private";
> + $co{'committer'} = hide_mailaddr($co{'committer'});
> + }
> [...]
> } else {
> $co{'committer_name'} = $co{'committer'};
> }
> @@ -3568,9 +3602,10 @@ sub parse_commit_text {
> if (! defined $co{'title'} || $co{'title'} eq "") {
> $co{'title'} = $co{'title_short'} = '(no commit message)';
> }
> - # remove added spaces
> + # remove added spaces, redact e-mail addresses if applicable.
> foreach my $line (@commit_lines) {
> $line =~ s/^ //;
> + $line = hide_mailaddr_if_private($line);
> }
> $co{'comment'} = \@commit_lines;
>
> @@ -8060,8 +8095,9 @@ sub git_commitdiff {
> close $fd
> or print "Reading git-diff-tree failed\n";
> } elsif ($format eq 'patch') {
> - local $/ = undef;
> - print <$fd>;
> + while (my $line = <$fd>) {
> + print hide_mailaddr_if_private($line);
> + }
Urm, have you tested this? How does a while loop over a <$fd> make sense
when $/ is undef, the readline() operator will always return just one
record, so having a while loop doesn't make sense.
I'm not sure of the input here, but given that if you're expecting to
replace all e-mail addresses on all lines with this function that's not
how it'll work, the s/// doesn't have a /g, so it'll stop at the first
replacement.
> close $fd
> or print "Reading git-format-patch failed\n";
> }
>
> base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 18:26 ` Ævar Arnfjörð Bjarmason
@ 2021-03-21 18:48 ` Junio C Hamano
2021-03-21 19:48 ` Georgios Kontaxis
1 sibling, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-21 18:48 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Georgios Kontaxis via GitGitGadget, git, brian m. carlson,
Georgios Kontaxis
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> But:
>
>> sub parse_tag {
>> my $tag_id = shift;
>> my %tag;
>> @@ -3471,6 +3493,10 @@ sub parse_tag {
>> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
>> $tag{'author_name'} = $1;
>> $tag{'author_email'} = $2;
>> + if (gitweb_check_feature('email_privacy')) {
>> + $tag{'author_email'} = "private";
>> + $tag{'author'} = hide_mailaddr($tag{'author'});
>> + }
>
> This code seems quite awkward, we've already done the regex match, but
> this code:
>
>> [snip]
>> +sub hide_mailaddr_if_private {
>> + my $line = shift;
>> + return $line unless (gitweb_check_feature('email_privacy') &&
>> + $line =~ m/^([^<]+) <([^>]*)>/);
>> + return hide_mailaddr($line)
>> +}
>> +
>> +sub hide_mailaddr {
>> + my $mailaddr = shift;
>> + $mailaddr =~ s/<([^>]*)>/<private>/;
>> + return $mailaddr;
>> +}
>
> Is going to do it again incrementally, and then just act on a
> search-replacement if we've got the feature enabled.
I think you misread the patch the same way as I did initially. What
is called in parse_tag is not the "if-private" version---the caller
knows that $tag{'author'} MUST BE an address so unconditionally redact
when the feature is set by bypassing the _if_private variant.
Having said that, I do think
> sub maybe_hide_email {
> my ($email, $ref) = shift;
> return $email unless gitweb_check_feature('email_privacy');
> $$ref = "private" if $ref;
> return hide_email($email);
> }
this helper is how you would simplify these numerous callers.
>> } elsif ($format eq 'patch') {
>> - local $/ = undef;
>> - print <$fd>;
>> + while (my $line = <$fd>) {
>> + print hide_mailaddr_if_private($line);
>> + }
>
> Urm, have you tested this? How does a while loop over a <$fd> make sense
> when $/ is undef, the readline() operator will always return just one
> record, so having a while loop doesn't make sense.
>
> I'm not sure of the input here, but given that if you're expecting to
> replace all e-mail addresses on all lines with this function that's not
> how it'll work, the s/// doesn't have a /g, so it'll stop at the first
> replacement.
>
>> close $fd
>> or print "Reading git-format-patch failed\n";
>> }
All true. This one is reading from "format-patch --stdout" output,
but for the reasons I mentioned in my review, I do not think it
should touch the 'patch' output at all.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 18:26 ` Ævar Arnfjörð Bjarmason
2021-03-21 18:48 ` Junio C Hamano
@ 2021-03-21 19:48 ` Georgios Kontaxis
1 sibling, 0 replies; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-21 19:48 UTC (permalink / raw)
To: "Ævar Arnfjörð Bjarmason"
Cc: Georgios Kontaxis via GitGitGadget, git, brian m. carlson
>
> On Sun, Mar 21 2021, Georgios Kontaxis via GitGitGadget wrote:
>
>> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over HTTP. As a result, e-mail addresses found in commits are
>> exposed to web crawlers and they may not respect robots.txt.
>> This may result in unsolicited messages.
>> This is a feature for redacting e-mail addresses
>> from the generated HTML, etc. content.
>>
>> This feature does not prevent someone from downloading the
>> unredacted commit log, e.g., by cloning the repository, and
>> extracting information from it.
>> It aims to hinder the low-effort bulk collection of e-mail
>> addresses by web crawlers.
>>
>> Changes since v1:
>> - Turned off the feature by default.
>> - Removed duplicate code.
>> - Added note about Gitweb consumers receiving redacted logs.
>>
>> Changes since v2:
>> - The feature can be set on a per-project basis. ('override' => 1)
>>
>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>> ---
>> gitweb: Redacted e-mail addresses feature.
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over
>> HTTP. As a result, e-mail addresses found in commits are exposed to
>> web
>> crawlers. This may result in unsolicited messages. This is a feature
>> for
>> redacting e-mail addresses from the generated HTML content.
>>
>> This feature does not prevent someone from downloading the
>> unredacted
>> commit log and extracting information from it. It aims to hinder the
>> low-effort bulk collection of e-mail addresses by web crawlers.
>>
>> Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
>>
>> Published-As:
>> https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
>> pr-910/kontaxis/kontaxis/email_privacy-v3
>> Pull-Request: https://github.com/gitgitgadget/git/pull/910
>>
>> Range-diff vs v2:
>>
>> 1: 74af11ca8bf2 ! 1: 930cdefe7ee0 gitweb: redacted e-mail addresses
>> feature.
>> @@ Commit message
>> - Removed duplicate code.
>> - Added note about Gitweb consumers receiving redacted logs.
>>
>> + Changes since v2:
>> + - The feature can be set on a per-project basis. ('override'
>> => 1)
>> +
>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> ## Documentation/gitweb.conf.txt ##
>> @@ gitweb/gitweb.perl: sub evaluate_uri {
>> + # $feature{'email_privacy'}{'default'} = [1];
>> + 'email_privacy' => {
>> + 'sub' => sub { feature_bool('email_privacy', @_) },
>> -+ 'override' => 0,
>> ++ 'override' => 1,
>> + 'default' => [0]},
>> );
>>
>>
>>
>> Documentation/gitweb.conf.txt | 16 +++++++++++++
>> gitweb/gitweb.perl | 42 ++++++++++++++++++++++++++++++++---
>> 2 files changed, 55 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/gitweb.conf.txt
>> b/Documentation/gitweb.conf.txt
>> index 7963a79ba98b..b7af3240177d 100644
>> --- a/Documentation/gitweb.conf.txt
>> +++ b/Documentation/gitweb.conf.txt
>> @@ -896,6 +896,22 @@ same as of the snippet above:
>> It is an error to specify a ref that does not pass "git
>> check-ref-format"
>> scrutiny. Duplicated values are filtered.
>>
>> +email_privacy::
>> + Redact e-mail addresses from the generated HTML, etc. content.
>> + This hides e-mail addresses found in the commit log from web
>> crawlers.
>> + Disabled by default.
>> ++
>> +It is highly recommended to enable this feature unless web crawlers are
>> +hindered in some other way. Note that crawlers intent on harvesting
>> e-mail
>> +addresses may disregard robots.txt. You can enable this feature like
>> so:
>> ++
>> +---------------------------------------------------------------------------
>> +$feature{'email_privacy'}{'default'} = [1];
>> +---------------------------------------------------------------------------
>> ++
>> +Note that if Gitweb is not the final step in a workflow then subsequent
>> +steps may misbehave because of the redacted information they receive.
>> +
>>
>> EXAMPLES
>> --------
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 0959a782eccb..174cc566d97d 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -569,6 +569,15 @@ sub evaluate_uri {
>> 'sub' => \&feature_extra_branch_refs,
>> 'override' => 0,
>> 'default' => []},
>> +
>> + # Redact e-mail addresses.
>> +
>> + # To enable system wide have in $GITWEB_CONFIG
>> + # $feature{'email_privacy'}{'default'} = [1];
>> + 'email_privacy' => {
>> + 'sub' => sub { feature_bool('email_privacy', @_) },
>> + 'override' => 1,
>> + 'default' => [0]},
>> );
>>
>> sub gitweb_get_feature {
>> @@ -3449,6 +3458,19 @@ sub parse_date {
>> return %date;
>> }
>>
>> [snip]
>
> So in the v1 feedback I suggested:
>
> BEGIN QUOTE
> sub maybe_hide_email {
> my $email = shift;
> return $email unless gitweb_check_feature('email_privacy');
> return hide_email($email);
> }
>
> then:
>
> $tag{author_email} = maybe_hide_email($2);
> END QUOTE
>
> But:
>
>> sub parse_tag {
>> my $tag_id = shift;
>> my %tag;
>> @@ -3471,6 +3493,10 @@ sub parse_tag {
>> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
>> $tag{'author_name'} = $1;
>> $tag{'author_email'} = $2;
>> + if (gitweb_check_feature('email_privacy')) {
>> + $tag{'author_email'} = "private";
>> + $tag{'author'} = hide_mailaddr($tag{'author'});
>> + }
>
> This code seems quite awkward, we've already done the regex match, but
> this code:
>
>> [snip]
>> +sub hide_mailaddr_if_private {
>> + my $line = shift;
>> + return $line unless (gitweb_check_feature('email_privacy') &&
>> + $line =~ m/^([^<]+) <([^>]*)>/);
>> + return hide_mailaddr($line)
>> +}
>> +
>> +sub hide_mailaddr {
>> + my $mailaddr = shift;
>> + $mailaddr =~ s/<([^>]*)>/<private>/;
>> + return $mailaddr;
>> +}
>
> Is going to do it again incrementally, and then just act on a
> search-replacement if we've got the feature enabled.
>
I think hide_mailaddr_if_private (which checks the feature flag
and looks for the pattern) is only called when we haven't done so
already. In every other case we call hide_mailaddr directly.
So there's no duplication of checks.
> It seems much simpler to just turn your:
>
>> + if (gitweb_check_feature('email_privacy')) {
>> + $tag{'author_email'} = "private";
>> + $tag{'author'} = hide_mailaddr($tag{'author'});
>> + }
>
> Into:
>
> $tag{'author'} = maybe_hide_mailaddr($tag{author},
> \$tag{author_email});
>
> Using:
>
> sub maybe_hide_email {
> my ($email, $ref) = shift;
> return $email unless gitweb_check_feature('email_privacy');
> $$ref = "private" if $ref;
> return hide_email($email);
> }
>
> Which also works for the case where you don't have a "private" hash key
> to assign to. But maybe it overcomplicates things...
>
>> } else {
>> $tag{'author_name'} = $tag{'author'};
>> }
>> @@ -3519,6 +3545,10 @@ sub parse_commit_text {
>> if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
>> $co{'author_name'} = $1;
>> $co{'author_email'} = $2;
>> + if (gitweb_check_feature('email_privacy')) {
>> + $co{'author_email'} = "private";
>> + $co{'author'} = hide_mailaddr($co{'author'});
>> + }
>> } else {
>> $co{'author_name'} = $co{'author'};
>> }
>> @@ -3529,6 +3559,10 @@ sub parse_commit_text {
>> if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
>> $co{'committer_name'} = $1;
>> $co{'committer_email'} = $2;
>> + if (gitweb_check_feature('email_privacy')) {
>> + $co{'committer_email'} = "private";
>> + $co{'committer'} = hide_mailaddr($co{'committer'});
>> + }
>> [...]
>> } else {
>> $co{'committer_name'} = $co{'committer'};
>> }
>> @@ -3568,9 +3602,10 @@ sub parse_commit_text {
>> if (! defined $co{'title'} || $co{'title'} eq "") {
>> $co{'title'} = $co{'title_short'} = '(no commit message)';
>> }
>> - # remove added spaces
>> + # remove added spaces, redact e-mail addresses if applicable.
>> foreach my $line (@commit_lines) {
>> $line =~ s/^ //;
>> + $line = hide_mailaddr_if_private($line);
>> }
>> $co{'comment'} = \@commit_lines;
>>
>> @@ -8060,8 +8095,9 @@ sub git_commitdiff {
>> close $fd
>> or print "Reading git-diff-tree failed\n";
>> } elsif ($format eq 'patch') {
>> - local $/ = undef;
>> - print <$fd>;
>> + while (my $line = <$fd>) {
>> + print hide_mailaddr_if_private($line);
>> + }
>
> Urm, have you tested this? How does a while loop over a <$fd> make sense
> when $/ is undef, the readline() operator will always return just one
> record, so having a while loop doesn't make sense.
>
$/ is not undef, the opposite, it's the default value which results
in reading from <$fd> one line at a time. (where line is a sequence
terminated by CRLF or LF)
> I'm not sure of the input here, but given that if you're expecting to
> replace all e-mail addresses on all lines with this function that's not
> how it'll work, the s/// doesn't have a /g, so it'll stop at the first
> replacement.
>
So we are reading from <$fd> one line at a time and looking for the
first "expr1 <expr2>" occurrence in each line. This successfully redacts
the author, committer, signed-off-by lines.
Not doing a global replacement per line is intentional so as to avoid
false positives.
So far I haven't noticed missing any addresses that should have been
redacted.
>> close $fd
>> or print "Reading git-format-patch failed\n";
>> }
>>
>> base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 17:28 ` [PATCH v3] " Georgios Kontaxis via GitGitGadget
2021-03-21 18:26 ` Ævar Arnfjörð Bjarmason
@ 2021-03-21 18:42 ` Junio C Hamano
2021-03-21 18:57 ` Junio C Hamano
2021-03-21 20:07 ` Georgios Kontaxis
2021-03-22 6:57 ` [PATCH v4] " Georgios Kontaxis via GitGitGadget
2 siblings, 2 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-21 18:42 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
"Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>
> Gitweb extracts content from the Git log and makes it accessible
> over HTTP. As a result, e-mail addresses found in commits are
> exposed to web crawlers and they may not respect robots.txt.
> This may result in unsolicited messages.
> This is a feature for redacting e-mail addresses
> from the generated HTML, etc. content.
>
> This feature does not prevent someone from downloading the
> unredacted commit log, e.g., by cloning the repository, and
> extracting information from it.
> It aims to hinder the low-effort bulk collection of e-mail
> addresses by web crawlers.
Lines from here ...
>
> Changes since v1:
> - Turned off the feature by default.
> - Removed duplicate code.
> - Added note about Gitweb consumers receiving redacted logs.
>
> Changes since v2:
> - The feature can be set on a per-project basis. ('override' => 1)
... to here are to help reviewers on the mailing list while
iterations of the same change is being reviewed and polished.
But it is useless noise for those who only read "git log". They
simply do not have access to earlier iterations.
Such "changes between iterations" comments needs to be written after
the three-dash lines.
> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
> ---
> gitweb: Redacted e-mail addresses feature.
>
> Gitweb extracts content from the Git log and makes it accessible over
> HTTP. As a result, e-mail addresses found in commits are exposed to web
> crawlers. This may result in unsolicited messages. This is a feature for
> redacting e-mail addresses from the generated HTML content.
>
> This feature does not prevent someone from downloading the unredacted
> commit log and extracting information from it. It aims to hinder the
> low-effort bulk collection of e-mail addresses by web crawlers.
>
> Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v3
> Pull-Request: https://github.com/gitgitgadget/git/pull/910
>
> Range-diff vs v2:
>
> 1: 74af11ca8bf2 ! 1: 930cdefe7ee0 gitweb: redacted e-mail addresses feature.
> @@ Commit message
> - Removed duplicate code.
> - Added note about Gitweb consumers receiving redacted logs.
>
> + Changes since v2:
> + - The feature can be set on a per-project basis. ('override' => 1)
> +
> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>
> ## Documentation/gitweb.conf.txt ##
> @@ gitweb/gitweb.perl: sub evaluate_uri {
> + # $feature{'email_privacy'}{'default'} = [1];
> + 'email_privacy' => {
> + 'sub' => sub { feature_bool('email_privacy', @_) },
> -+ 'override' => 0,
> ++ 'override' => 1,
> + 'default' => [0]},
> );
>
>
>
> Documentation/gitweb.conf.txt | 16 +++++++++++++
> gitweb/gitweb.perl | 42 ++++++++++++++++++++++++++++++++---
> 2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
> index 7963a79ba98b..b7af3240177d 100644
> --- a/Documentation/gitweb.conf.txt
> +++ b/Documentation/gitweb.conf.txt
> @@ -896,6 +896,22 @@ same as of the snippet above:
> It is an error to specify a ref that does not pass "git check-ref-format"
> scrutiny. Duplicated values are filtered.
>
> +email_privacy::
> + Redact e-mail addresses from the generated HTML, etc. content.
> + This hides e-mail addresses found in the commit log from web crawlers.
> + Disabled by default.
> ++
> +It is highly recommended to enable this feature unless web crawlers are
> +hindered in some other way. Note that crawlers intent on harvesting e-mail
> +addresses may disregard robots.txt.
Up to this line is more-or-less OK, but with a few comments:
- "This hides ... from web crawlers"? Doesn't this hide it
indiscriminately, whether the requester is an interactive
end-user or a crawler?
- The name of the configured feature, 'email_privacy'. There is
another existing feature with underscore in its name
(remote_heads), but I think it is an odd-ball mistake we do not
want to imitate. Instead, I think the "extra branch refs" may be
a good example to follow. The feature name (written in Perl
code) is "extra-branch-refs" (downcased words with inter-word
dashes) and the corresponding configuration (I am not saying we
should add one to support this feature, if we do not have one
already) is "gitweb.extraBranchRefs" (camelCased words).
- Do not exaggerate with words like "highly", but trust the
intelligence of your readers to make the right decision when they
understand the reason why this feature exists in the first place.
- I do not think this entry should be added to the end of feature
list. How about listing it just after 'avatar', which is also
about how author/committer identity is shown?
> +---------------------------------------------------------------------------
> +$feature{'email_privacy'}{'default'} = [1];
> +---------------------------------------------------------------------------
I do not think this should be here. None of the boolean features
listed early in "Features in `%feature`" section like blame,
snapshot, grep, pickaxe, ... features tell readers how to enable a
specific feature like that.
Unless the syntax to configure a feature is one-off oddball that is
different from all other features, we shouldn't clutter the
description with an example like this.
At the beginning of the section "Configuring Gitweb Features" there
is a general description and that should be sufficient (and if it is
not sufficient, it is OK to add an example to that section).
> +Note that if Gitweb is not the final step in a workflow then subsequent
> +steps may misbehave because of the redacted information they receive.
In other words, this will break crawlers that expect email addresses
are there and want to use it for some good purpose. But that is an
natural consequence of the "feature", and should be described as
such when we said "Redact e-mail addresses", not as a "by the way"
mention in a footnote.
... after reading more, readers realize that the damage is
far worse in the current incarnation of the patch, but let's
read on ...
> @@ -3449,6 +3458,19 @@ sub parse_date {
> return %date;
> }
>
> +sub hide_mailaddr_if_private {
> + my $line = shift;
> + return $line unless (gitweb_check_feature('email_privacy') &&
> + $line =~ m/^([^<]+) <([^>]*)>/);
I find that the second line is way too deeply indented. Wouldn't
> + return $line unless (gitweb_check_feature('email_privacy') &&
> + $line =~ m/^([^<]+) <([^>]*)>/);
be enough?
> + return hide_mailaddr($line)
> +}
> +
> +sub hide_mailaddr {
> + my $mailaddr = shift;
> + $mailaddr =~ s/<([^>]*)>/<private>/;
s/private/redacted/ perhaps?
> @@ -3568,9 +3602,10 @@ sub parse_commit_text {
> if (! defined $co{'title'} || $co{'title'} eq "") {
> $co{'title'} = $co{'title_short'} = '(no commit message)';
> }
> - # remove added spaces
> + # remove added spaces, redact e-mail addresses if applicable.
> foreach my $line (@commit_lines) {
> $line =~ s/^ //;
> + $line = hide_mailaddr_if_private($line);
A commit log that ends with
Thank you, A <a@example.com> and B <b@example.com>, for discovering
this bug and quickly proposing a solution.
would recact only the first one but not the other, I suspect.
Much more problematic is that I see too many hits from:
$ git log v2.30.0..v2.31.0 | grep ' <[^>@]*>'
That is, "find a line in the commit log message that has a SP, open <bra,
run of characters that are not ket> or at@sign, closed with ket>". These
are clearly not e-mail addresses.
This "feature" butchers a commit title:
mktag doc: say <hash> not <sha1>
to
mktag doc: say <private> not <sha1>
doesn't it?
> @@ -8060,8 +8095,9 @@ sub git_commitdiff {
> close $fd
> or print "Reading git-diff-tree failed\n";
> } elsif ($format eq 'patch') {
> - local $/ = undef;
> - print <$fd>;
> + while (my $line = <$fd>) {
> + print hide_mailaddr_if_private($line);
> + }
And this is even worse.
$ git log -p --no-merges v2.30.0..v2.31.0 | grep ' <[^>@]*>' | wc -l
gives me ~4700 hits. Because the patch text as well as the log
message is munged, the "feature" makes the output utterly unreliable
as an input to "git am" by munging too much. Interesting examples
are like these:
- for (i = 0; i < pairs->nr; ++i) {
'git <command> [<revision>...] -- [<file>...]'
+ for (i = split; i < geometry->pack_nr; i++) {
Now, I am *NOT* saying that you should tighten the e-mail address
syntax detection and keep munging the patch text. The lines that
begin with minus and SP in a patch must match the preimage text,
so munging out existing e-mail addresses from them will make the
patch fail to find the part that needs to be modified. And
replacing an e-mail address on a line that begins with plus would
redact it from the source---if they wrote an address, they must have
meant it to be available to those who consume the source, so I doubt
the wisdom of munging the patch part at all.
I may be sympathetic to the cause of the patch, but, I do not agree
with its execution in this iteration of the patch.
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 18:42 ` Junio C Hamano
@ 2021-03-21 18:57 ` Junio C Hamano
2021-03-21 19:05 ` Junio C Hamano
2021-03-21 20:07 ` Georgios Kontaxis
1 sibling, 1 reply; 41+ messages in thread
From: Junio C Hamano @ 2021-03-21 18:57 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
Junio C Hamano <gitster@pobox.com> writes:
>> @@ -8060,8 +8095,9 @@ sub git_commitdiff {
>> close $fd
>> or print "Reading git-diff-tree failed\n";
>> } elsif ($format eq 'patch') {
>> - local $/ = undef;
>> - print <$fd>;
>> + while (my $line = <$fd>) {
>> + print hide_mailaddr_if_private($line);
>> + }
>
> And this is even worse.
And also Ævar is right. The original just has a format-patch output
in $_ and prints it to the output stream in one go. It won't make
any sense to attempt reading from that stream, which the original
code used to write to, at all.
In any case, I've already said that I do not think it is a good idea
to touch the 'patch' codepath at all, so this code won't do any
damage in the end by reading from the output pipe connected to
end-user's browser ;-)
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 18:57 ` Junio C Hamano
@ 2021-03-21 19:05 ` Junio C Hamano
0 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-21 19:05 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
> And also Ævar is right. The original just has a format-patch output
> in $_ and prints it to the output stream in one go.
Sorry but I misread the patch. By dropping the $/ = undef, it wants
to read from $fd (which is "format-patch --stdout" output) one line
at a time. So unless a line has two addresses, this would "work" as
intended.
Again, as I said, I do not agree with the definition of "work" here,
though ;-).
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 18:42 ` Junio C Hamano
2021-03-21 18:57 ` Junio C Hamano
@ 2021-03-21 20:07 ` Georgios Kontaxis
2021-03-21 22:17 ` Junio C Hamano
1 sibling, 1 reply; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-21 20:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
> "Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over HTTP. As a result, e-mail addresses found in commits are
>> exposed to web crawlers and they may not respect robots.txt.
>> This may result in unsolicited messages.
>> This is a feature for redacting e-mail addresses
>> from the generated HTML, etc. content.
>>
>> This feature does not prevent someone from downloading the
>> unredacted commit log, e.g., by cloning the repository, and
>> extracting information from it.
>> It aims to hinder the low-effort bulk collection of e-mail
>> addresses by web crawlers.
>
> Lines from here ...
>
>>
>> Changes since v1:
>> - Turned off the feature by default.
>> - Removed duplicate code.
>> - Added note about Gitweb consumers receiving redacted logs.
>>
>> Changes since v2:
>> - The feature can be set on a per-project basis. ('override' => 1)
>
> ... to here are to help reviewers on the mailing list while
> iterations of the same change is being reviewed and polished.
>
> But it is useless noise for those who only read "git log". They
> simply do not have access to earlier iterations.
>
> Such "changes between iterations" comments needs to be written after
> the three-dash lines.
>
>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>> ---
>> gitweb: Redacted e-mail addresses feature.
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over
>> HTTP. As a result, e-mail addresses found in commits are exposed to
>> web
>> crawlers. This may result in unsolicited messages. This is a feature
>> for
>> redacting e-mail addresses from the generated HTML content.
>>
>> This feature does not prevent someone from downloading the
>> unredacted
>> commit log and extracting information from it. It aims to hinder the
>> low-effort bulk collection of e-mail addresses by web crawlers.
>>
>> Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
>>
>> Published-As:
>> https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v3
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
>> pr-910/kontaxis/kontaxis/email_privacy-v3
>> Pull-Request: https://github.com/gitgitgadget/git/pull/910
>>
>> Range-diff vs v2:
>>
>> 1: 74af11ca8bf2 ! 1: 930cdefe7ee0 gitweb: redacted e-mail addresses
>> feature.
>> @@ Commit message
>> - Removed duplicate code.
>> - Added note about Gitweb consumers receiving redacted logs.
>>
>> + Changes since v2:
>> + - The feature can be set on a per-project basis. ('override'
>> => 1)
>> +
>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> ## Documentation/gitweb.conf.txt ##
>> @@ gitweb/gitweb.perl: sub evaluate_uri {
>> + # $feature{'email_privacy'}{'default'} = [1];
>> + 'email_privacy' => {
>> + 'sub' => sub { feature_bool('email_privacy', @_) },
>> -+ 'override' => 0,
>> ++ 'override' => 1,
>> + 'default' => [0]},
>> );
>>
>>
>>
>> Documentation/gitweb.conf.txt | 16 +++++++++++++
>> gitweb/gitweb.perl | 42 ++++++++++++++++++++++++++++++++---
>> 2 files changed, 55 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/gitweb.conf.txt
>> b/Documentation/gitweb.conf.txt
>> index 7963a79ba98b..b7af3240177d 100644
>> --- a/Documentation/gitweb.conf.txt
>> +++ b/Documentation/gitweb.conf.txt
>> @@ -896,6 +896,22 @@ same as of the snippet above:
>> It is an error to specify a ref that does not pass "git
>> check-ref-format"
>> scrutiny. Duplicated values are filtered.
>>
>> +email_privacy::
>> + Redact e-mail addresses from the generated HTML, etc. content.
>> + This hides e-mail addresses found in the commit log from web
>> crawlers.
>> + Disabled by default.
>> ++
>> +It is highly recommended to enable this feature unless web crawlers are
>> +hindered in some other way. Note that crawlers intent on harvesting
>> e-mail
>> +addresses may disregard robots.txt.
>
> Up to this line is more-or-less OK, but with a few comments:
>
> - "This hides ... from web crawlers"? Doesn't this hide it
> indiscriminately, whether the requester is an interactive
> end-user or a crawler?
>
Correct. It doesn't say "it only hides from".
I can see how it may be confusing. I'll rephrase.
> - The name of the configured feature, 'email_privacy'. There is
> another existing feature with underscore in its name
> (remote_heads), but I think it is an odd-ball mistake we do not
> want to imitate. Instead, I think the "extra branch refs" may be
> a good example to follow. The feature name (written in Perl
> code) is "extra-branch-refs" (downcased words with inter-word
> dashes) and the corresponding configuration (I am not saying we
> should add one to support this feature, if we do not have one
> already) is "gitweb.extraBranchRefs" (camelCased words).
>
I'll rename the feature.
> - Do not exaggerate with words like "highly", but trust the
> intelligence of your readers to make the right decision when they
> understand the reason why this feature exists in the first place.
>
Will remove.
> - I do not think this entry should be added to the end of feature
> list. How about listing it just after 'avatar', which is also
> about how author/committer identity is shown?
>
Will do.
>> +---------------------------------------------------------------------------
>> +$feature{'email_privacy'}{'default'} = [1];
>> +---------------------------------------------------------------------------
>
> I do not think this should be here. None of the boolean features
> listed early in "Features in `%feature`" section like blame,
> snapshot, grep, pickaxe, ... features tell readers how to enable a
> specific feature like that.
>
> Unless the syntax to configure a feature is one-off oddball that is
> different from all other features, we shouldn't clutter the
> description with an example like this.
>
> At the beginning of the section "Configuring Gitweb Features" there
> is a general description and that should be sufficient (and if it is
> not sufficient, it is OK to add an example to that section).
>
Will remove.
>> +Note that if Gitweb is not the final step in a workflow then subsequent
>> +steps may misbehave because of the redacted information they receive.
>
> In other words, this will break crawlers that expect email addresses
> are there and want to use it for some good purpose. But that is an
> natural consequence of the "feature", and should be described as
> such when we said "Redact e-mail addresses", not as a "by the way"
> mention in a footnote.
>
> ... after reading more, readers realize that the damage is
> far worse in the current incarnation of the patch, but let's
> read on ...
>
Will move this out of the footnote.
>> @@ -3449,6 +3458,19 @@ sub parse_date {
>> return %date;
>> }
>>
>> +sub hide_mailaddr_if_private {
>> + my $line = shift;
>> + return $line unless (gitweb_check_feature('email_privacy') &&
>> + $line =~ m/^([^<]+) <([^>]*)>/);
>
> I find that the second line is way too deeply indented. Wouldn't
>
>> + return $line unless (gitweb_check_feature('email_privacy') &&
>> + $line =~ m/^([^<]+) <([^>]*)>/);
>
> be enough?
>
Will adjust.
>> + return hide_mailaddr($line)
>> +}
>> +
>> +sub hide_mailaddr {
>> + my $mailaddr = shift;
>> + $mailaddr =~ s/<([^>]*)>/<private>/;
>
> s/private/redacted/ perhaps?
>
Will adjust.
>> @@ -3568,9 +3602,10 @@ sub parse_commit_text {
>> if (! defined $co{'title'} || $co{'title'} eq "") {
>> $co{'title'} = $co{'title_short'} = '(no commit message)';
>> }
>> - # remove added spaces
>> + # remove added spaces, redact e-mail addresses if applicable.
>> foreach my $line (@commit_lines) {
>> $line =~ s/^ //;
>> + $line = hide_mailaddr_if_private($line);
>
> A commit log that ends with
>
> Thank you, A <a@example.com> and B <b@example.com>, for discovering
> this bug and quickly proposing a solution.
>
> would recact only the first one but not the other, I suspect.
>
> Much more problematic is that I see too many hits from:
>
> $ git log v2.30.0..v2.31.0 | grep ' <[^>@]*>'
>
> That is, "find a line in the commit log message that has a SP, open <bra,
> run of characters that are not ket> or at@sign, closed with ket>". These
> are clearly not e-mail addresses.
>
> This "feature" butchers a commit title:
>
> mktag doc: say <hash> not <sha1>
>
> to
>
> mktag doc: say <private> not <sha1>
>
> doesn't it?
>
True. Thanks for pointing these out.
Sounds like the pattern we're looking for needs to be more specific.
>> @@ -8060,8 +8095,9 @@ sub git_commitdiff {
>> close $fd
>> or print "Reading git-diff-tree failed\n";
>> } elsif ($format eq 'patch') {
>> - local $/ = undef;
>> - print <$fd>;
>> + while (my $line = <$fd>) {
>> + print hide_mailaddr_if_private($line);
>> + }
>
> And this is even worse.
>
> $ git log -p --no-merges v2.30.0..v2.31.0 | grep ' <[^>@]*>' | wc -l
>
> gives me ~4700 hits. Because the patch text as well as the log
> message is munged, the "feature" makes the output utterly unreliable
> as an input to "git am" by munging too much. Interesting examples
> are like these:
>
> - for (i = 0; i < pairs->nr; ++i) {
> 'git <command> [<revision>...] -- [<file>...]'
> + for (i = split; i < geometry->pack_nr; i++) {
>
> Now, I am *NOT* saying that you should tighten the e-mail address
> syntax detection and keep munging the patch text. The lines that
> begin with minus and SP in a patch must match the preimage text,
> so munging out existing e-mail addresses from them will make the
> patch fail to find the part that needs to be modified. And
> replacing an e-mail address on a line that begins with plus would
> redact it from the source---if they wrote an address, they must have
> meant it to be available to those who consume the source, so I doubt
> the wisdom of munging the patch part at all.
>
> I may be sympathetic to the cause of the patch, but, I do not agree
> with its execution in this iteration of the patch.
>
I see your point.
It seems hiding e-mail addresses should be limited to the commit message,
i.e., stop at the "---" line.
> Thanks.
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 20:07 ` Georgios Kontaxis
@ 2021-03-21 22:17 ` Junio C Hamano
2021-03-21 23:14 ` Georgios Kontaxis
0 siblings, 1 reply; 41+ messages in thread
From: Junio C Hamano @ 2021-03-21 22:17 UTC (permalink / raw)
To: Georgios Kontaxis
Cc: Georgios Kontaxis via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, brian m. carlson
"Georgios Kontaxis" <geko1702+commits@99rst.org> writes:
>> ... so I doubt
>> the wisdom of munging the patch part at all.
>>
>> I may be sympathetic to the cause of the patch, but, I do not agree
>> with its execution in this iteration of the patch.
>>
> I see your point.
>
> It seems hiding e-mail addresses should be limited to the commit message,
> i.e., stop at the "---" line.
I doubt it makes sense to redact anything in the 'patch' view at
all, actually. What kind of URL does the crawler need to formulate
and what pieces of information (like commit object names or branch
names) does it need to fill in the URL to get a series of patches
out of gitweb? As long as it takes more effort than running "git
clone" against the repository, the crawler would not have much
incentive to crawl and harvest addresses from the 'patch' pages, and
even in the log message part, the downsides of butchering the
payload would outweigh the "privacy benefit", I would have to say.
Quite honestly, if a site claims to offer a 'patch' download UI but
returns corrupt data back, I would say it is much worse than not
offering the service at all. Perhaps disabling the 'patch' feature
in repositories that enable 'privacy' feature may be a much better
approach.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 22:17 ` Junio C Hamano
@ 2021-03-21 23:14 ` Georgios Kontaxis
2021-03-22 4:25 ` Junio C Hamano
0 siblings, 1 reply; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-21 23:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
> "Georgios Kontaxis" <geko1702+commits@99rst.org> writes:
>
>>> ... so I doubt
>>> the wisdom of munging the patch part at all.
>>>
>>> I may be sympathetic to the cause of the patch, but, I do not agree
>>> with its execution in this iteration of the patch.
>>>
>> I see your point.
>>
>> It seems hiding e-mail addresses should be limited to the commit
>> message,
>> i.e., stop at the "---" line.
>
> I doubt it makes sense to redact anything in the 'patch' view at
> all, actually. What kind of URL does the crawler need to formulate
> and what pieces of information (like commit object names or branch
> names) does it need to fill in the URL to get a series of patches
> out of gitweb? As long as it takes more effort than running "git
> clone" against the repository, the crawler would not have much
> incentive to crawl and harvest addresses from the 'patch' pages, and
> even in the log message part, the downsides of butchering the
> payload would outweigh the "privacy benefit", I would have to say.
>
No effort at all I would say.
E..g, somehow the web crawler gets to git.kernel.org.
It then follows every link, eventually arriving at a commitdiff page.
It then follows every link, which includes the URL for the patch output.
See how "wget --mirror" behaves for instance.
Just to clarify, my goal is not to stop someone who wants to extract
e-mail address from git.kernel.org specifically.
They can just "git clone" the repositories and grep through the logs.
My goal is to stop generic crawlers (pretty much "wget --mirror | grep"
scripts) from making their way to the logs.
> Quite honestly, if a site claims to offer a 'patch' download UI but
> returns corrupt data back, I would say it is much worse than not
> offering the service at all. Perhaps disabling the 'patch' feature
> in repositories that enable 'privacy' feature may be a much better
> approach.
>
Good point. I think I'll try that.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3] gitweb: redacted e-mail addresses feature.
2021-03-21 23:14 ` Georgios Kontaxis
@ 2021-03-22 4:25 ` Junio C Hamano
0 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-22 4:25 UTC (permalink / raw)
To: Georgios Kontaxis
Cc: Georgios Kontaxis via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, brian m. carlson
"Georgios Kontaxis" <geko1702+commits@99rst.org> writes:
>> Quite honestly, if a site claims to offer a 'patch' download UI but
>> returns corrupt data back, I would say it is much worse than not
>> offering the service at all. Perhaps disabling the 'patch' feature
>> in repositories that enable 'privacy' feature may be a much better
>> approach.
>>
> Good point. I think I'll try that.
Alternatively, we could add 'captcha' to stop crawlers, perhaps?
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-21 17:28 ` [PATCH v3] " Georgios Kontaxis via GitGitGadget
2021-03-21 18:26 ` Ævar Arnfjörð Bjarmason
2021-03-21 18:42 ` Junio C Hamano
@ 2021-03-22 6:57 ` Georgios Kontaxis via GitGitGadget
2021-03-22 18:32 ` Junio C Hamano
2021-03-27 3:56 ` [PATCH v5] " Georgios Kontaxis via GitGitGadget
2 siblings, 2 replies; 41+ messages in thread
From: Georgios Kontaxis via GitGitGadget @ 2021-03-22 6:57 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis, Georgios Kontaxis
From: Georgios Kontaxis <geko1702+commits@99rst.org>
Gitweb extracts content from the Git log and makes it accessible
over HTTP. As a result, e-mail addresses found in commits are
exposed to web crawlers and they may not respect robots.txt.
This may result in unsolicited messages.
This is a feature for redacting e-mail addresses
from the generated HTML, etc. content.
This feature does not prevent someone from downloading the
unredacted commit log, e.g., by cloning the repository, and
extracting information from it.
It aims to hinder the low-effort bulk collection of e-mail
addresses by web crawlers.
Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
---
gitweb: redacted e-mail addresses feature.
Gitweb extracts content from the Git log and makes it accessible over
HTTP. As a result, e-mail addresses found in commits are exposed to web
crawlers and they may not respect robots.txt. This may result in
unsolicited messages. This is a feature for redacting e-mail addresses
from the generated HTML, etc. content.
This feature does not prevent someone from downloading the unredacted
commit log, e.g., by cloning the repository, and extracting information
from it. It aims to hinder the low-effort bulk collection of e-mail
addresses by web crawlers.
Changes since v1:
* Turned off the feature by default.
* Removed duplicate code.
* Added note about Gitweb consumers receiving redacted logs.
Changes since v2:
* The feature can be set on a per-project basis. ('override' => 1)
Changes since v3:
* Renamed feature to "email-privacy" and improved documentation.
* Removed UI elements for git-format-patch since it won't be redacted.
* Simplified calls to the address redaction logic.
* Mail::Address is now used to reduce false-positive redactions.
Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/910
Range-diff vs v3:
1: 930cdefe7ee0 ! 1: 03a3f41c37ef gitweb: redacted e-mail addresses feature.
@@ Commit message
It aims to hinder the low-effort bulk collection of e-mail
addresses by web crawlers.
- Changes since v1:
- - Turned off the feature by default.
- - Removed duplicate code.
- - Added note about Gitweb consumers receiving redacted logs.
-
- Changes since v2:
- - The feature can be set on a per-project basis. ('override' => 1)
-
Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
## Documentation/gitweb.conf.txt ##
-@@ Documentation/gitweb.conf.txt: same as of the snippet above:
- It is an error to specify a ref that does not pass "git check-ref-format"
- scrutiny. Duplicated values are filtered.
+@@ Documentation/gitweb.conf.txt: default font sizes or lineheights are changed (e.g. via adding extra
+ CSS stylesheet in `@stylesheets`), it may be appropriate to change
+ these values.
-+email_privacy::
-+ Redact e-mail addresses from the generated HTML, etc. content.
-+ This hides e-mail addresses found in the commit log from web crawlers.
-+ Disabled by default.
-++
-+It is highly recommended to enable this feature unless web crawlers are
-+hindered in some other way. Note that crawlers intent on harvesting e-mail
-+addresses may disregard robots.txt. You can enable this feature like so:
-++
-+---------------------------------------------------------------------------
-+$feature{'email_privacy'}{'default'} = [1];
-+---------------------------------------------------------------------------
-++
-+Note that if Gitweb is not the final step in a workflow then subsequent
-+steps may misbehave because of the redacted information they receive.
++email-privacy::
++ Redact e-mail addresses from the generated HTML, etc. content.
++ This hides e-mail addresses found in the commit log from HTTP clients.
++ It is meant to hinder web crawlers that harvest and abuse addresses.
++ Such crawlers may not respect robots.txt.
++ Note that users and user tools also see the addresses redacted.
++ If Gitweb is not the final step in a workflow then subsequent steps
++ may misbehave because of the redacted information they receive.
++ Disabled by default.
+
-
- EXAMPLES
- --------
+ highlight::
+ Server-side syntax highlight support in "blob" view. It requires
+ `$highlight_bin` program to be available (see the description of
## gitweb/gitweb.perl ##
+@@
+ use File::Basename qw(basename);
+ use Time::HiRes qw(gettimeofday tv_interval);
+ use Digest::MD5 qw(md5_hex);
++use Git::LoadCPAN::Mail::Address;
+
+ binmode STDOUT, ':utf8';
+
@@ gitweb/gitweb.perl: sub evaluate_uri {
'sub' => \&feature_extra_branch_refs,
'override' => 0,
@@ gitweb/gitweb.perl: sub evaluate_uri {
+ # Redact e-mail addresses.
+
+ # To enable system wide have in $GITWEB_CONFIG
-+ # $feature{'email_privacy'}{'default'} = [1];
-+ 'email_privacy' => {
-+ 'sub' => sub { feature_bool('email_privacy', @_) },
++ # $feature{'email-privacy'}{'default'} = [1];
++ 'email-privacy' => {
++ 'sub' => sub { feature_bool('email-privacy', @_) },
+ 'override' => 1,
+ 'default' => [0]},
);
@@ gitweb/gitweb.perl: sub parse_date {
return %date;
}
-+sub hide_mailaddr_if_private {
-+ my $line = shift;
-+ return $line unless (gitweb_check_feature('email_privacy') &&
-+ $line =~ m/^([^<]+) <([^>]*)>/);
-+ return hide_mailaddr($line)
++sub is_mailaddr {
++ my @addrs = Mail::Address->parse(shift);
++ if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
++ return 0;
++ }
++ return 1;
+}
+
-+sub hide_mailaddr {
-+ my $mailaddr = shift;
-+ $mailaddr =~ s/<([^>]*)>/<private>/;
-+ return $mailaddr;
++sub hide_mailaddrs_if_private {
++ my $line = shift;
++ return $line unless gitweb_check_feature('email-privacy');
++ while ($line =~ m/(<[^>]+>)/g) {
++ my $match = $1;
++ if (!is_mailaddr($match)) {
++ next;
++ }
++ my $offset = pos $line;
++ my $head = substr $line, 0, $offset - length($match);
++ my $redaction = "<redacted>";
++ my $tail = substr $line, $offset;
++ $line = $head . $redaction . $tail;
++ pos $line = length($head) + length($redaction);
++ }
++ return $line;
+}
+
sub parse_tag {
my $tag_id = shift;
my %tag;
@@ gitweb/gitweb.perl: sub parse_tag {
+ } elsif ($line =~ m/^tag (.+)$/) {
+ $tag{'name'} = $1;
+ } elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
+- $tag{'author'} = $1;
++ $tag{'author'} = hide_mailaddrs_if_private($1);
+ $tag{'author_epoch'} = $2;
+ $tag{'author_tz'} = $3;
if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
- $tag{'author_name'} = $1;
- $tag{'author_email'} = $2;
-+ if (gitweb_check_feature('email_privacy')) {
-+ $tag{'author_email'} = "private";
-+ $tag{'author'} = hide_mailaddr($tag{'author'});
-+ }
- } else {
- $tag{'author_name'} = $tag{'author'};
- }
@@ gitweb/gitweb.perl: sub parse_commit_text {
+ } elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {
+ push @parents, $1;
+ } elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
+- $co{'author'} = to_utf8($1);
++ $co{'author'} = hide_mailaddrs_if_private(to_utf8($1));
+ $co{'author_epoch'} = $2;
+ $co{'author_tz'} = $3;
if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
- $co{'author_name'} = $1;
- $co{'author_email'} = $2;
-+ if (gitweb_check_feature('email_privacy')) {
-+ $co{'author_email'} = "private";
-+ $co{'author'} = hide_mailaddr($co{'author'});
-+ }
- } else {
+@@ gitweb/gitweb.perl: sub parse_commit_text {
$co{'author_name'} = $co{'author'};
}
-@@ gitweb/gitweb.perl: sub parse_commit_text {
+ } elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {
+- $co{'committer'} = to_utf8($1);
++ $co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));
+ $co{'committer_epoch'} = $2;
+ $co{'committer_tz'} = $3;
if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
- $co{'committer_name'} = $1;
- $co{'committer_email'} = $2;
-+ if (gitweb_check_feature('email_privacy')) {
-+ $co{'committer_email'} = "private";
-+ $co{'committer'} = hide_mailaddr($co{'committer'});
-+ }
- } else {
- $co{'committer_name'} = $co{'committer'};
- }
@@ gitweb/gitweb.perl: sub parse_commit_text {
if (! defined $co{'title'} || $co{'title'} eq "") {
$co{'title'} = $co{'title_short'} = '(no commit message)';
@@ gitweb/gitweb.perl: sub parse_commit_text {
+ # remove added spaces, redact e-mail addresses if applicable.
foreach my $line (@commit_lines) {
$line =~ s/^ //;
-+ $line = hide_mailaddr_if_private($line);
++ $line = hide_mailaddrs_if_private($line);
}
$co{'comment'} = \@commit_lines;
-@@ gitweb/gitweb.perl: sub git_commitdiff {
- close $fd
- or print "Reading git-diff-tree failed\n";
- } elsif ($format eq 'patch') {
-- local $/ = undef;
-- print <$fd>;
-+ while (my $line = <$fd>) {
-+ print hide_mailaddr_if_private($line);
-+ }
- close $fd
- or print "Reading git-format-patch failed\n";
+@@ gitweb/gitweb.perl: sub git_log_generic {
+ -accesskey => "n", -title => "Alt-n"}, "next");
+ }
+ my $patch_max = gitweb_get_feature('patches');
+- if ($patch_max && !defined $file_name) {
++ if ($patch_max && !defined $file_name &&
++ !gitweb_check_feature('email-privacy')) {
+ if ($patch_max < 0 || @commitlist <= $patch_max) {
+ $paging_nav .= " ⋅ " .
+ $cgi->a({-href => href(action=>"patches", -replay=>1)},
+@@ gitweb/gitweb.perl: sub git_commit {
+ } @$parents ) .
+ ')';
}
+- if (gitweb_check_feature('patches') && @$parents <= 1) {
++ if (gitweb_check_feature('patches') && @$parents <= 1 &&
++ !gitweb_check_feature('email-privacy')) {
+ $formats_nav .= " | " .
+ $cgi->a({-href => href(action=>"patch", -replay=>1)},
+ "patch");
+@@ gitweb/gitweb.perl: sub git_commitdiff {
+ $formats_nav =
+ $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
+ "raw");
+- if ($patch_max && @{$co{'parents'}} <= 1) {
++ if ($patch_max && @{$co{'parents'}} <= 1 &&
++ !gitweb_check_feature('email-privacy')) {
+ $formats_nav .= " | " .
+ $cgi->a({-href => href(action=>"patch", -replay=>1)},
+ "patch");
+
+ ## t/lib-gitweb.sh ##
+@@ t/lib-gitweb.sh: gitweb_run () {
+ GITWEB_CONFIG=$(pwd)/gitweb_config.perl
+ export GITWEB_CONFIG
+
++ PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
++ export PERL5LIB
++
+ # some of git commands write to STDERR on error, but this is not
+ # written to web server logs, so we are not interested in that:
+ # we are interested only in properly formatted errors/warnings
Documentation/gitweb.conf.txt | 10 +++++++
gitweb/gitweb.perl | 54 ++++++++++++++++++++++++++++++-----
t/lib-gitweb.sh | 3 ++
3 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index 7963a79ba98b..47d85717ccaa 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -751,6 +751,16 @@ default font sizes or lineheights are changed (e.g. via adding extra
CSS stylesheet in `@stylesheets`), it may be appropriate to change
these values.
+email-privacy::
+ Redact e-mail addresses from the generated HTML, etc. content.
+ This hides e-mail addresses found in the commit log from HTTP clients.
+ It is meant to hinder web crawlers that harvest and abuse addresses.
+ Such crawlers may not respect robots.txt.
+ Note that users and user tools also see the addresses redacted.
+ If Gitweb is not the final step in a workflow then subsequent steps
+ may misbehave because of the redacted information they receive.
+ Disabled by default.
+
highlight::
Server-side syntax highlight support in "blob" view. It requires
`$highlight_bin` program to be available (see the description of
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0959a782eccb..6630c76d92fd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -21,6 +21,7 @@
use File::Basename qw(basename);
use Time::HiRes qw(gettimeofday tv_interval);
use Digest::MD5 qw(md5_hex);
+use Git::LoadCPAN::Mail::Address;
binmode STDOUT, ':utf8';
@@ -569,6 +570,15 @@ sub evaluate_uri {
'sub' => \&feature_extra_branch_refs,
'override' => 0,
'default' => []},
+
+ # Redact e-mail addresses.
+
+ # To enable system wide have in $GITWEB_CONFIG
+ # $feature{'email-privacy'}{'default'} = [1];
+ 'email-privacy' => {
+ 'sub' => sub { feature_bool('email-privacy', @_) },
+ 'override' => 1,
+ 'default' => [0]},
);
sub gitweb_get_feature {
@@ -3449,6 +3459,32 @@ sub parse_date {
return %date;
}
+sub is_mailaddr {
+ my @addrs = Mail::Address->parse(shift);
+ if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
+ return 0;
+ }
+ return 1;
+}
+
+sub hide_mailaddrs_if_private {
+ my $line = shift;
+ return $line unless gitweb_check_feature('email-privacy');
+ while ($line =~ m/(<[^>]+>)/g) {
+ my $match = $1;
+ if (!is_mailaddr($match)) {
+ next;
+ }
+ my $offset = pos $line;
+ my $head = substr $line, 0, $offset - length($match);
+ my $redaction = "<redacted>";
+ my $tail = substr $line, $offset;
+ $line = $head . $redaction . $tail;
+ pos $line = length($head) + length($redaction);
+ }
+ return $line;
+}
+
sub parse_tag {
my $tag_id = shift;
my %tag;
@@ -3465,7 +3501,7 @@ sub parse_tag {
} elsif ($line =~ m/^tag (.+)$/) {
$tag{'name'} = $1;
} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
- $tag{'author'} = $1;
+ $tag{'author'} = hide_mailaddrs_if_private($1);
$tag{'author_epoch'} = $2;
$tag{'author_tz'} = $3;
if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3513,7 +3549,7 @@ sub parse_commit_text {
} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {
push @parents, $1;
} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
- $co{'author'} = to_utf8($1);
+ $co{'author'} = hide_mailaddrs_if_private(to_utf8($1));
$co{'author_epoch'} = $2;
$co{'author_tz'} = $3;
if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3523,7 +3559,7 @@ sub parse_commit_text {
$co{'author_name'} = $co{'author'};
}
} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {
- $co{'committer'} = to_utf8($1);
+ $co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));
$co{'committer_epoch'} = $2;
$co{'committer_tz'} = $3;
if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3568,9 +3604,10 @@ sub parse_commit_text {
if (! defined $co{'title'} || $co{'title'} eq "") {
$co{'title'} = $co{'title_short'} = '(no commit message)';
}
- # remove added spaces
+ # remove added spaces, redact e-mail addresses if applicable.
foreach my $line (@commit_lines) {
$line =~ s/^ //;
+ $line = hide_mailaddrs_if_private($line);
}
$co{'comment'} = \@commit_lines;
@@ -7489,7 +7526,8 @@ sub git_log_generic {
-accesskey => "n", -title => "Alt-n"}, "next");
}
my $patch_max = gitweb_get_feature('patches');
- if ($patch_max && !defined $file_name) {
+ if ($patch_max && !defined $file_name &&
+ !gitweb_check_feature('email-privacy')) {
if ($patch_max < 0 || @commitlist <= $patch_max) {
$paging_nav .= " ⋅ " .
$cgi->a({-href => href(action=>"patches", -replay=>1)},
@@ -7550,7 +7588,8 @@ sub git_commit {
} @$parents ) .
')';
}
- if (gitweb_check_feature('patches') && @$parents <= 1) {
+ if (gitweb_check_feature('patches') && @$parents <= 1 &&
+ !gitweb_check_feature('email-privacy')) {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
@@ -7863,7 +7902,8 @@ sub git_commitdiff {
$formats_nav =
$cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
"raw");
- if ($patch_max && @{$co{'parents'}} <= 1) {
+ if ($patch_max && @{$co{'parents'}} <= 1 &&
+ !gitweb_check_feature('email-privacy')) {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh
index 1f32ca66ea51..77fc1298d4c6 100644
--- a/t/lib-gitweb.sh
+++ b/t/lib-gitweb.sh
@@ -67,6 +67,9 @@ gitweb_run () {
GITWEB_CONFIG=$(pwd)/gitweb_config.perl
export GITWEB_CONFIG
+ PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
+ export PERL5LIB
+
# some of git commands write to STDERR on error, but this is not
# written to web server logs, so we are not interested in that:
# we are interested only in properly formatted errors/warnings
base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-22 6:57 ` [PATCH v4] " Georgios Kontaxis via GitGitGadget
@ 2021-03-22 18:32 ` Junio C Hamano
2021-03-22 18:58 ` Georgios Kontaxis
2021-03-23 4:27 ` Georgios Kontaxis
2021-03-27 3:56 ` [PATCH v5] " Georgios Kontaxis via GitGitGadget
1 sibling, 2 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-22 18:32 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
"Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
writes:
[note to other reviewers. input from those who are more familiar
with gitweb and Perl is very much appreciated on this patch].
> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>
> Gitweb extracts content from the Git log and makes it accessible
> over HTTP. As a result, e-mail addresses found in commits are
> exposed to web crawlers and they may not respect robots.txt.
> This may result in unsolicited messages.
"... are exposed to web crawlers, which spammers may use." would be
sufficient as a problem description.
After giving a problem description, it is customery to describe the
solution as if you are ordering the codebase to "be like so", so
instead of this ...
> This is a feature for redacting e-mail addresses
> from the generated HTML, etc. content.
... we may say something like
Introduce an 'email-privacy' feature, which defaults to false,
that redacts e-mail addresses that appear as author/committer
info and in log messages from the generated HTML content.
> This feature does not prevent someone from downloading the
> unredacted commit log, e.g., by cloning the repository, and
> extracting information from it.
> It aims to hinder the low-effort bulk collection of e-mail
> addresses by web crawlers.
And this is a good thing to add. Overall, nicely written.
> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
> ---
> gitweb: redacted e-mail addresses feature.
>
> Gitweb extracts content from the Git log and makes it accessible over
> HTTP. As a result, e-mail addresses found in commits are exposed to web
> crawlers and they may not respect robots.txt. This may result in
> unsolicited messages. This is a feature for redacting e-mail addresses
> from the generated HTML, etc. content.
>
> This feature does not prevent someone from downloading the unredacted
> commit log, e.g., by cloning the repository, and extracting information
> from it. It aims to hinder the low-effort bulk collection of e-mail
> addresses by web crawlers.
You do not need to repeat the above, which is in the log message above.
> Changes since v1:
>
> * Turned off the feature by default.
> * Removed duplicate code.
> * Added note about Gitweb consumers receiving redacted logs.
>
> Changes since v2:
>
> * The feature can be set on a per-project basis. ('override' => 1)
>
> Changes since v3:
>
> * Renamed feature to "email-privacy" and improved documentation.
> * Removed UI elements for git-format-patch since it won't be redacted.
> * Simplified calls to the address redaction logic.
> * Mail::Address is now used to reduce false-positive redactions.
Having these under the --- line like this is very helpful.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0959a782eccb..6630c76d92fd 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -21,6 +21,7 @@
> use File::Basename qw(basename);
> use Time::HiRes qw(gettimeofday tv_interval);
> use Digest::MD5 qw(md5_hex);
> +use Git::LoadCPAN::Mail::Address;
I'll defer to others who are more familiar with gitweb and Perl
ecosystem if this is warranted, but I have a feeling that importing
and using Mail::Address->parse() only because we want to see if a
given "<string>" is an address is a bit overkill and it might be
sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i
> + # Redact e-mail addresses.
> +
> + # To enable system wide have in $GITWEB_CONFIG
> + # $feature{'email-privacy'}{'default'} = [1];
> + 'email-privacy' => {
> + 'sub' => sub { feature_bool('email-privacy', @_) },
> + 'override' => 1,
> + 'default' => [0]},
> );
>
> sub gitweb_get_feature {
> @@ -3449,6 +3459,32 @@ sub parse_date {
> return %date;
> }
>
> +sub is_mailaddr {
> + my @addrs = Mail::Address->parse(shift);
> + if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
> + return 0;
> + }
> + return 1;
> +}
> +
> +sub hide_mailaddrs_if_private {
> + my $line = shift;
> + return $line unless gitweb_check_feature('email-privacy');
> + while ($line =~ m/(<[^>]+>)/g) {
> + my $match = $1;
> + if (!is_mailaddr($match)) {
> + next;
> + }
> + my $offset = pos $line;
> + my $head = substr $line, 0, $offset - length($match);
> + my $redaction = "<redacted>";
> + my $tail = substr $line, $offset;
> + $line = $head . $redaction . $tail;
> + pos $line = length($head) + length($redaction);
Hmmmm, Perl suggestions from others? It looks quite strange to see
that s/// operator is not used and replacement is done manually with
byte position in a Perl script.
> sub parse_tag {
> my $tag_id = shift;
> my %tag;
> @@ -3465,7 +3501,7 @@ sub parse_tag {
> } elsif ($line =~ m/^tag (.+)$/) {
> $tag{'name'} = $1;
> } elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
> - $tag{'author'} = $1;
> + $tag{'author'} = hide_mailaddrs_if_private($1);
> $tag{'author_epoch'} = $2;
> $tag{'author_tz'} = $3;
> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
This (and the others that follow the same pattern) looks sensible.
> @@ -7489,7 +7526,8 @@ sub git_log_generic {
> -accesskey => "n", -title => "Alt-n"}, "next");
> }
> my $patch_max = gitweb_get_feature('patches');
> - if ($patch_max && !defined $file_name) {
> + if ($patch_max && !defined $file_name &&
> + !gitweb_check_feature('email-privacy')) {
> if ($patch_max < 0 || @commitlist <= $patch_max) {
> $paging_nav .= " ⋅ " .
> $cgi->a({-href => href(action=>"patches", -replay=>1)},
> @@ -7550,7 +7588,8 @@ sub git_commit {
> } @$parents ) .
> ')';
> }
> - if (gitweb_check_feature('patches') && @$parents <= 1) {
> + if (gitweb_check_feature('patches') && @$parents <= 1 &&
> + !gitweb_check_feature('email-privacy')) {
> $formats_nav .= " | " .
> $cgi->a({-href => href(action=>"patch", -replay=>1)},
> "patch");
> @@ -7863,7 +7902,8 @@ sub git_commitdiff {
> $formats_nav =
> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
> "raw");
> - if ($patch_max && @{$co{'parents'}} <= 1) {
> + if ($patch_max && @{$co{'parents'}} <= 1 &&
> + !gitweb_check_feature('email-privacy')) {
> $formats_nav .= " | " .
> $cgi->a({-href => href(action=>"patch", -replay=>1)},
> "patch");
I wouldn't have expected to see the above three hunks in the
"patch" codepath. Rather, I was hoping that you'd do something
like this at startup when you find out that the privacy feature
is enabled:
$feature{'patches'}{'default'} = [0]
if gitweb_get_feature('email-privacy');
so that nothing related to the 'patches' has to be modified.
That way, even if there were fourth place that can leak an e-mail
address in the 'patch' codepath that above three hunks in this patch
missed, crawlers won't be able to get at it, no?
> diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh
> index 1f32ca66ea51..77fc1298d4c6 100644
> --- a/t/lib-gitweb.sh
> +++ b/t/lib-gitweb.sh
> @@ -67,6 +67,9 @@ gitweb_run () {
> GITWEB_CONFIG=$(pwd)/gitweb_config.perl
> export GITWEB_CONFIG
>
> + PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
> + export PERL5LIB
> +
Why is this change suddenly become necessary? This addition is only
about tests---do we need to do something similar in the runtime
environment when the updated gitweb that requires Mail::Address gets
deployed, or is that covered already somewhere else?
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-22 18:32 ` Junio C Hamano
@ 2021-03-22 18:58 ` Georgios Kontaxis
2021-03-28 1:41 ` Junio C Hamano
2021-03-23 4:27 ` Georgios Kontaxis
1 sibling, 1 reply; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-22 18:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
> "Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> [note to other reviewers. input from those who are more familiar
> with gitweb and Perl is very much appreciated on this patch].
>
>> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over HTTP. As a result, e-mail addresses found in commits are
>> exposed to web crawlers and they may not respect robots.txt.
>> This may result in unsolicited messages.
>
> "... are exposed to web crawlers, which spammers may use." would be
> sufficient as a problem description.
>
Using robots.txt was suggested earlier in this review so
I want to make sure people don't rely on it instead.
> After giving a problem description, it is customery to describe the
> solution as if you are ordering the codebase to "be like so", so
> instead of this ...
>
>> This is a feature for redacting e-mail addresses
>> from the generated HTML, etc. content.
>
> ... we may say something like
>
> Introduce an 'email-privacy' feature, which defaults to false,
> that redacts e-mail addresses that appear as author/committer
> info and in log messages from the generated HTML content.
>
Will rephrase.
>> This feature does not prevent someone from downloading the
>> unredacted commit log, e.g., by cloning the repository, and
>> extracting information from it.
>> It aims to hinder the low-effort bulk collection of e-mail
>> addresses by web crawlers.
>
> And this is a good thing to add. Overall, nicely written.
>
>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>> ---
>> gitweb: redacted e-mail addresses feature.
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over
>> HTTP. As a result, e-mail addresses found in commits are exposed to
>> web
>> crawlers and they may not respect robots.txt. This may result in
>> unsolicited messages. This is a feature for redacting e-mail
>> addresses
>> from the generated HTML, etc. content.
>>
>> This feature does not prevent someone from downloading the
>> unredacted
>> commit log, e.g., by cloning the repository, and extracting
>> information
>> from it. It aims to hinder the low-effort bulk collection of e-mail
>> addresses by web crawlers.
>
> You do not need to repeat the above, which is in the log message above.
>
>> Changes since v1:
>>
>> * Turned off the feature by default.
>> * Removed duplicate code.
>> * Added note about Gitweb consumers receiving redacted logs.
>>
>> Changes since v2:
>>
>> * The feature can be set on a per-project basis. ('override' => 1)
>>
>> Changes since v3:
>>
>> * Renamed feature to "email-privacy" and improved documentation.
>> * Removed UI elements for git-format-patch since it won't be
>> redacted.
>> * Simplified calls to the address redaction logic.
>> * Mail::Address is now used to reduce false-positive redactions.
>
> Having these under the --- line like this is very helpful.
>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 0959a782eccb..6630c76d92fd 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -21,6 +21,7 @@
>> use File::Basename qw(basename);
>> use Time::HiRes qw(gettimeofday tv_interval);
>> use Digest::MD5 qw(md5_hex);
>> +use Git::LoadCPAN::Mail::Address;
>
> I'll defer to others who are more familiar with gitweb and Perl
> ecosystem if this is warranted, but I have a feeling that importing
> and using Mail::Address->parse() only because we want to see if a
> given "<string>" is an address is a bit overkill and it might be
> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i
>
I think Mail::Address->parse() pretty much does the same thing.
It was suggested earlier in the review to use that function,
similar to what git-send-mail does.
I'm assuming because the maintainers of the Mail package are already
invested in the (accurate) parsing of e-mail addresses.
>> + # Redact e-mail addresses.
>> +
>> + # To enable system wide have in $GITWEB_CONFIG
>> + # $feature{'email-privacy'}{'default'} = [1];
>> + 'email-privacy' => {
>> + 'sub' => sub { feature_bool('email-privacy', @_) },
>> + 'override' => 1,
>> + 'default' => [0]},
>> );
>>
>> sub gitweb_get_feature {
>> @@ -3449,6 +3459,32 @@ sub parse_date {
>> return %date;
>> }
>>
>> +sub is_mailaddr {
>> + my @addrs = Mail::Address->parse(shift);
>> + if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
>> + return 0;
>> + }
>> + return 1;
>> +}
>> +
>> +sub hide_mailaddrs_if_private {
>> + my $line = shift;
>> + return $line unless gitweb_check_feature('email-privacy');
>> + while ($line =~ m/(<[^>]+>)/g) {
>> + my $match = $1;
>> + if (!is_mailaddr($match)) {
>> + next;
>> + }
>> + my $offset = pos $line;
>> + my $head = substr $line, 0, $offset - length($match);
>> + my $redaction = "<redacted>";
>> + my $tail = substr $line, $offset;
>> + $line = $head . $redaction . $tail;
>> + pos $line = length($head) + length($redaction);
>
> Hmmmm, Perl suggestions from others? It looks quite strange to see
> that s/// operator is not used and replacement is done manually with
> byte position in a Perl script.
>
If there's a more elegant way to do the above we can certain do that instead.
>> sub parse_tag {
>> my $tag_id = shift;
>> my %tag;
>> @@ -3465,7 +3501,7 @@ sub parse_tag {
>> } elsif ($line =~ m/^tag (.+)$/) {
>> $tag{'name'} = $1;
>> } elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
>> - $tag{'author'} = $1;
>> + $tag{'author'} = hide_mailaddrs_if_private($1);
>> $tag{'author_epoch'} = $2;
>> $tag{'author_tz'} = $3;
>> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
>
> This (and the others that follow the same pattern) looks sensible.
>
>> @@ -7489,7 +7526,8 @@ sub git_log_generic {
>> -accesskey => "n", -title => "Alt-n"}, "next");
>> }
>> my $patch_max = gitweb_get_feature('patches');
>> - if ($patch_max && !defined $file_name) {
>> + if ($patch_max && !defined $file_name &&
>> + !gitweb_check_feature('email-privacy')) {
>> if ($patch_max < 0 || @commitlist <= $patch_max) {
>> $paging_nav .= " ⋅ " .
>> $cgi->a({-href => href(action=>"patches", -replay=>1)},
>> @@ -7550,7 +7588,8 @@ sub git_commit {
>> } @$parents ) .
>> ')';
>> }
>> - if (gitweb_check_feature('patches') && @$parents <= 1) {
>> + if (gitweb_check_feature('patches') && @$parents <= 1 &&
>> + !gitweb_check_feature('email-privacy')) {
>> $formats_nav .= " | " .
>> $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> "patch");
>> @@ -7863,7 +7902,8 @@ sub git_commitdiff {
>> $formats_nav =
>> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
>> "raw");
>> - if ($patch_max && @{$co{'parents'}} <= 1) {
>> + if ($patch_max && @{$co{'parents'}} <= 1 &&
>> + !gitweb_check_feature('email-privacy')) {
>> $formats_nav .= " | " .
>> $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> "patch");
>
> I wouldn't have expected to see the above three hunks in the
> "patch" codepath. Rather, I was hoping that you'd do something
> like this at startup when you find out that the privacy feature
> is enabled:
>
> $feature{'patches'}{'default'} = [0]
> if gitweb_get_feature('email-privacy');
>
> so that nothing related to the 'patches' has to be modified.
> That way, even if there were fourth place that can leak an e-mail
> address in the 'patch' codepath that above three hunks in this patch
> missed, crawlers won't be able to get at it, no?
>
Or there's multiple feature flags / triggers that lead to the "patch"
codepath.
Either way is fine with me. (disabling the "patches" feature or preventing
the
generation of UI elements)
Like you said, someone who regularly reviews Gitweb code may have an opinion
on how do this.
>> diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh
>> index 1f32ca66ea51..77fc1298d4c6 100644
>> --- a/t/lib-gitweb.sh
>> +++ b/t/lib-gitweb.sh
>> @@ -67,6 +67,9 @@ gitweb_run () {
>> GITWEB_CONFIG=$(pwd)/gitweb_config.perl
>> export GITWEB_CONFIG
>>
>> + PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
>> + export PERL5LIB
>> +
>
> Why is this change suddenly become necessary? This addition is only
> about tests---do we need to do something similar in the runtime
> environment when the updated gitweb that requires Mail::Address gets
> deployed, or is that covered already somewhere else?
>
This is necessary because osx-clang, osx-gcc tests were failing.
If there's a better way to address this I'm open to suggestions.
I haven't been able to experience deployment on osx but on linux
i didn't have to change anything.
> Thanks.
>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-22 18:58 ` Georgios Kontaxis
@ 2021-03-28 1:41 ` Junio C Hamano
2021-03-28 21:43 ` Georgios Kontaxis
0 siblings, 1 reply; 41+ messages in thread
From: Junio C Hamano @ 2021-03-28 1:41 UTC (permalink / raw)
To: Georgios Kontaxis
Cc: Georgios Kontaxis via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, brian m. carlson
"Georgios Kontaxis" <geko1702+commits@99rst.org> writes:
>> I'll defer to others who are more familiar with gitweb and Perl
>> ecosystem if this is warranted, but I have a feeling that importing
>> and using Mail::Address->parse() only because we want to see if a
>> given "<string>" is an address is a bit overkill and it might be
>> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i
>> ...
>>> + while ($line =~ m/(<[^>]+>)/g) {
>>> + my $match = $1;
>>> + if (!is_mailaddr($match)) {
>>> + next;
>>> + }
>>> + my $offset = pos $line;
>>> + my $head = substr $line, 0, $offset - length($match);
>>> + my $redaction = "<redacted>";
>>> + my $tail = substr $line, $offset;
>>> + $line = $head . $redaction . $tail;
>>> + pos $line = length($head) + length($redaction);
>>
>> Hmmmm, Perl suggestions from others? It looks quite strange to see
>> that s/// operator is not used and replacement is done manually with
>> byte position in a Perl script.
>>
> If there's a more elegant way to do the above we can certain do that instead.
For example, if we do not insist on using overkill Mail::Address->parse(),
we could do something silly like this:
$line =~ s/<[^@>]+@[a-z0-9-.]+>/<redacted@address>/ig;
no?
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-28 1:41 ` Junio C Hamano
@ 2021-03-28 21:43 ` Georgios Kontaxis
2021-03-28 22:35 ` Junio C Hamano
0 siblings, 1 reply; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-28 21:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
> "Georgios Kontaxis" <geko1702+commits@99rst.org> writes:
>
>>> I'll defer to others who are more familiar with gitweb and Perl
>>> ecosystem if this is warranted, but I have a feeling that importing
>>> and using Mail::Address->parse() only because we want to see if a
>>> given "<string>" is an address is a bit overkill and it might be
>>> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i
>>> ...
>>>> + while ($line =~ m/(<[^>]+>)/g) {
>>>> + my $match = $1;
>>>> + if (!is_mailaddr($match)) {
>>>> + next;
>>>> + }
>>>> + my $offset = pos $line;
>>>> + my $head = substr $line, 0, $offset - length($match);
>>>> + my $redaction = "<redacted>";
>>>> + my $tail = substr $line, $offset;
>>>> + $line = $head . $redaction . $tail;
>>>> + pos $line = length($head) + length($redaction);
>>>
>>> Hmmmm, Perl suggestions from others? It looks quite strange to see
>>> that s/// operator is not used and replacement is done manually with
>>> byte position in a Perl script.
>>>
>> If there's a more elegant way to do the above we can certain do that
>> instead.
>
> For example, if we do not insist on using overkill Mail::Address->parse(),
> we could do something silly like this:
>
> $line =~ s/<[^@>]+@[a-z0-9-.]+>/<redacted@address>/ig;
>
> no?
>
It's not clear if you think it's overkill because we have to depend on an
external module or because we don't need accurate parsing.
The above expression seems to get the job done but it isn't the correct
way to parse an e-mail address.
But if it's good enough for the reviewers I don't feel strongly about it.
If we prefer accurate parsing but don't like depending on Mail::Address,
it's easy to write complete expressions ourselves. (In a separate,
internal, Perl module)
Thoughts?
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-28 21:43 ` Georgios Kontaxis
@ 2021-03-28 22:35 ` Junio C Hamano
0 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-28 22:35 UTC (permalink / raw)
To: Georgios Kontaxis
Cc: Georgios Kontaxis via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, brian m. carlson
"Georgios Kontaxis" <geko1702+commits@99rst.org> writes:
> It's not clear if you think it's overkill because we have to depend on an
> external module or because we don't need accurate parsing.
It is neither; what we need to parse is not exactly 'e-mail
addresses' as known to Mail::Address.
The thing is, unlike send-email that needs to interact with the
real-world MTAs and e-mail addresses, the codepaths we are talking
about are mostly about author/committer ident, where the definition
is quite narrower than the Mail::Address's "has to cover all the
possible ways to spell e-mail addresses under the sun" requirement.
Having said all that ...
> If we prefer accurate parsing but don't like depending on Mail::Address,
> it's easy to write complete expressions ourselves. (In a separate,
> internal, Perl module)
... I do not have strong opinions either way. I won't have the
final say on the way things are done in Perl and what is done to
Gitweb.
It was just that what I saw earlier with the offsets and manual
parsing instead of s///g operator did not smell not like a Perl
program to me, and the message you are responding to was my reaction
to it.
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4] gitweb: redacted e-mail addresses feature.
2021-03-22 18:32 ` Junio C Hamano
2021-03-22 18:58 ` Georgios Kontaxis
@ 2021-03-23 4:27 ` Georgios Kontaxis
1 sibling, 0 replies; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-23 4:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
> "Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> [note to other reviewers. input from those who are more familiar
> with gitweb and Perl is very much appreciated on this patch].
>
>> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over HTTP. As a result, e-mail addresses found in commits are
>> exposed to web crawlers and they may not respect robots.txt.
>> This may result in unsolicited messages.
>
> "... are exposed to web crawlers, which spammers may use." would be
> sufficient as a problem description.
>
> After giving a problem description, it is customery to describe the
> solution as if you are ordering the codebase to "be like so", so
> instead of this ...
>
>> This is a feature for redacting e-mail addresses
>> from the generated HTML, etc. content.
>
> ... we may say something like
>
> Introduce an 'email-privacy' feature, which defaults to false,
> that redacts e-mail addresses that appear as author/committer
> info and in log messages from the generated HTML content.
>
>> This feature does not prevent someone from downloading the
>> unredacted commit log, e.g., by cloning the repository, and
>> extracting information from it.
>> It aims to hinder the low-effort bulk collection of e-mail
>> addresses by web crawlers.
>
> And this is a good thing to add. Overall, nicely written.
>
>> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
>> ---
>> gitweb: redacted e-mail addresses feature.
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over
>> HTTP. As a result, e-mail addresses found in commits are exposed to
>> web
>> crawlers and they may not respect robots.txt. This may result in
>> unsolicited messages. This is a feature for redacting e-mail
>> addresses
>> from the generated HTML, etc. content.
>>
>> This feature does not prevent someone from downloading the
>> unredacted
>> commit log, e.g., by cloning the repository, and extracting
>> information
>> from it. It aims to hinder the low-effort bulk collection of e-mail
>> addresses by web crawlers.
>
> You do not need to repeat the above, which is in the log message above.
>
>> Changes since v1:
>>
>> * Turned off the feature by default.
>> * Removed duplicate code.
>> * Added note about Gitweb consumers receiving redacted logs.
>>
>> Changes since v2:
>>
>> * The feature can be set on a per-project basis. ('override' => 1)
>>
>> Changes since v3:
>>
>> * Renamed feature to "email-privacy" and improved documentation.
>> * Removed UI elements for git-format-patch since it won't be
>> redacted.
>> * Simplified calls to the address redaction logic.
>> * Mail::Address is now used to reduce false-positive redactions.
>
> Having these under the --- line like this is very helpful.
>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 0959a782eccb..6630c76d92fd 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -21,6 +21,7 @@
>> use File::Basename qw(basename);
>> use Time::HiRes qw(gettimeofday tv_interval);
>> use Digest::MD5 qw(md5_hex);
>> +use Git::LoadCPAN::Mail::Address;
>
> I'll defer to others who are more familiar with gitweb and Perl
> ecosystem if this is warranted, but I have a feeling that importing
> and using Mail::Address->parse() only because we want to see if a
> given "<string>" is an address is a bit overkill and it might be
> sufficient to do something as crude as m/^<[^@>]+@[a-z0-9-.]+>$/i
>
>> + # Redact e-mail addresses.
>> +
>> + # To enable system wide have in $GITWEB_CONFIG
>> + # $feature{'email-privacy'}{'default'} = [1];
>> + 'email-privacy' => {
>> + 'sub' => sub { feature_bool('email-privacy', @_) },
>> + 'override' => 1,
>> + 'default' => [0]},
>> );
>>
>> sub gitweb_get_feature {
>> @@ -3449,6 +3459,32 @@ sub parse_date {
>> return %date;
>> }
>>
>> +sub is_mailaddr {
>> + my @addrs = Mail::Address->parse(shift);
>> + if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
>> + return 0;
>> + }
>> + return 1;
>> +}
>> +
>> +sub hide_mailaddrs_if_private {
>> + my $line = shift;
>> + return $line unless gitweb_check_feature('email-privacy');
>> + while ($line =~ m/(<[^>]+>)/g) {
>> + my $match = $1;
>> + if (!is_mailaddr($match)) {
>> + next;
>> + }
>> + my $offset = pos $line;
>> + my $head = substr $line, 0, $offset - length($match);
>> + my $redaction = "<redacted>";
>> + my $tail = substr $line, $offset;
>> + $line = $head . $redaction . $tail;
>> + pos $line = length($head) + length($redaction);
>
> Hmmmm, Perl suggestions from others? It looks quite strange to see
> that s/// operator is not used and replacement is done manually with
> byte position in a Perl script.
>
>> sub parse_tag {
>> my $tag_id = shift;
>> my %tag;
>> @@ -3465,7 +3501,7 @@ sub parse_tag {
>> } elsif ($line =~ m/^tag (.+)$/) {
>> $tag{'name'} = $1;
>> } elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
>> - $tag{'author'} = $1;
>> + $tag{'author'} = hide_mailaddrs_if_private($1);
>> $tag{'author_epoch'} = $2;
>> $tag{'author_tz'} = $3;
>> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
>
> This (and the others that follow the same pattern) looks sensible.
>
>> @@ -7489,7 +7526,8 @@ sub git_log_generic {
>> -accesskey => "n", -title => "Alt-n"}, "next");
>> }
>> my $patch_max = gitweb_get_feature('patches');
>> - if ($patch_max && !defined $file_name) {
>> + if ($patch_max && !defined $file_name &&
>> + !gitweb_check_feature('email-privacy')) {
>> if ($patch_max < 0 || @commitlist <= $patch_max) {
>> $paging_nav .= " ⋅ " .
>> $cgi->a({-href => href(action=>"patches", -replay=>1)},
>> @@ -7550,7 +7588,8 @@ sub git_commit {
>> } @$parents ) .
>> ')';
>> }
>> - if (gitweb_check_feature('patches') && @$parents <= 1) {
>> + if (gitweb_check_feature('patches') && @$parents <= 1 &&
>> + !gitweb_check_feature('email-privacy')) {
>> $formats_nav .= " | " .
>> $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> "patch");
>> @@ -7863,7 +7902,8 @@ sub git_commitdiff {
>> $formats_nav =
>> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
>> "raw");
>> - if ($patch_max && @{$co{'parents'}} <= 1) {
>> + if ($patch_max && @{$co{'parents'}} <= 1 &&
>> + !gitweb_check_feature('email-privacy')) {
>> $formats_nav .= " | " .
>> $cgi->a({-href => href(action=>"patch", -replay=>1)},
>> "patch");
>
> I wouldn't have expected to see the above three hunks in the
> "patch" codepath. Rather, I was hoping that you'd do something
> like this at startup when you find out that the privacy feature
> is enabled:
>
> $feature{'patches'}{'default'} = [0]
> if gitweb_get_feature('email-privacy');
>
> so that nothing related to the 'patches' has to be modified.
> That way, even if there were fourth place that can leak an e-mail
> address in the 'patch' codepath that above three hunks in this patch
> missed, crawlers won't be able to get at it, no?
>
I forgot to mention one of the reasons for doing it this way.
If someone knows this URL exists and is relying on it (e.g., in a script)
they can still call it with "email-privacy" enabled.
The only thing that's changing is that the UI element (link) that a web
crawler would follow just because it's there is now gone.
But, as mentioned before, I don't feel strongly about either approach.
Let me know.
>> diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh
>> index 1f32ca66ea51..77fc1298d4c6 100644
>> --- a/t/lib-gitweb.sh
>> +++ b/t/lib-gitweb.sh
>> @@ -67,6 +67,9 @@ gitweb_run () {
>> GITWEB_CONFIG=$(pwd)/gitweb_config.perl
>> export GITWEB_CONFIG
>>
>> + PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
>> + export PERL5LIB
>> +
>
> Why is this change suddenly become necessary? This addition is only
> about tests---do we need to do something similar in the runtime
> environment when the updated gitweb that requires Mail::Address gets
> deployed, or is that covered already somewhere else?
>
> Thanks.
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-03-22 6:57 ` [PATCH v4] " Georgios Kontaxis via GitGitGadget
2021-03-22 18:32 ` Junio C Hamano
@ 2021-03-27 3:56 ` Georgios Kontaxis via GitGitGadget
2021-03-28 23:26 ` [PATCH v6] " Georgios Kontaxis via GitGitGadget
2021-03-29 1:47 ` [PATCH v5] " Eric Wong
1 sibling, 2 replies; 41+ messages in thread
From: Georgios Kontaxis via GitGitGadget @ 2021-03-27 3:56 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis, Georgios Kontaxis
From: Georgios Kontaxis <geko1702+commits@99rst.org>
Gitweb extracts content from the Git log and makes it accessible
over HTTP. As a result, e-mail addresses found in commits are
exposed to web crawlers and they may not respect robots.txt.
This can result in unsolicited messages.
Introduce an 'email-privacy' feature which redacts e-mail addresses
from the generated HTML content. Specifically, obscure addresses
retrieved from the the author/committer and comment sections of the
Git log. The feature is off by default.
This feature does not prevent someone from downloading the
unredacted commit log, e.g., by cloning the repository, and
extracting information from it. It aims to hinder the low-
effort, bulk collection of e-mail addresses by web crawlers.
Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
---
gitweb: redacted e-mail addresses feature.
Changes since v1:
* Turned off the feature by default.
* Removed duplicate code.
* Added note about Gitweb consumers receiving redacted logs.
Changes since v2:
* The feature can be set on a per-project basis. ('override' => 1)
Changes since v3:
* Renamed feature to "email-privacy" and improved documentation.
* Removed UI elements for git-format-patch since it won't be redacted.
* Simplified calls to the address redaction logic.
* Mail::Address is now used to reduce false-positive redactions.
Changes since v4:
* Rephrased the commit comment.
* hide_mailaddrs_if_private is slighly more compact.
Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/910
Range-diff vs v4:
1: 03a3f41c37ef ! 1: 1427231f9db5 gitweb: redacted e-mail addresses feature.
@@ Commit message
Gitweb extracts content from the Git log and makes it accessible
over HTTP. As a result, e-mail addresses found in commits are
exposed to web crawlers and they may not respect robots.txt.
- This may result in unsolicited messages.
- This is a feature for redacting e-mail addresses
- from the generated HTML, etc. content.
+ This can result in unsolicited messages.
+
+ Introduce an 'email-privacy' feature which redacts e-mail addresses
+ from the generated HTML content. Specifically, obscure addresses
+ retrieved from the the author/committer and comment sections of the
+ Git log. The feature is off by default.
This feature does not prevent someone from downloading the
unredacted commit log, e.g., by cloning the repository, and
- extracting information from it.
- It aims to hinder the low-effort bulk collection of e-mail
- addresses by web crawlers.
+ extracting information from it. It aims to hinder the low-
+ effort, bulk collection of e-mail addresses by web crawlers.
Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
@@ Documentation/gitweb.conf.txt: default font sizes or lineheights are changed (e.
+email-privacy::
+ Redact e-mail addresses from the generated HTML, etc. content.
-+ This hides e-mail addresses found in the commit log from HTTP clients.
++ This obscures e-mail addresses retrieved from the author/committer
++ and comment sections of the Git log.
+ It is meant to hinder web crawlers that harvest and abuse addresses.
+ Such crawlers may not respect robots.txt.
-+ Note that users and user tools also see the addresses redacted.
++ Note that users and user tools also see the addresses as redacted.
+ If Gitweb is not the final step in a workflow then subsequent steps
+ may misbehave because of the redacted information they receive.
+ Disabled by default.
@@ gitweb/gitweb.perl: sub parse_date {
+ if (!is_mailaddr($match)) {
+ next;
+ }
-+ my $offset = pos $line;
-+ my $head = substr $line, 0, $offset - length($match);
++ my $match_offset = pos($line) - length($match);
++ pos $line = $match_offset;
++
+ my $redaction = "<redacted>";
-+ my $tail = substr $line, $offset;
-+ $line = $head . $redaction . $tail;
-+ pos $line = length($head) + length($redaction);
++ $line =~ s/\G(<[^>]+>)/$redaction/;
++
++ pos $line = $match_offset + length($redaction);
+ }
+ return $line;
+}
Documentation/gitweb.conf.txt | 11 +++++++
gitweb/gitweb.perl | 55 ++++++++++++++++++++++++++++++-----
t/lib-gitweb.sh | 3 ++
3 files changed, 62 insertions(+), 7 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index 7963a79ba98b..34b1d6e22435 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -751,6 +751,17 @@ default font sizes or lineheights are changed (e.g. via adding extra
CSS stylesheet in `@stylesheets`), it may be appropriate to change
these values.
+email-privacy::
+ Redact e-mail addresses from the generated HTML, etc. content.
+ This obscures e-mail addresses retrieved from the author/committer
+ and comment sections of the Git log.
+ It is meant to hinder web crawlers that harvest and abuse addresses.
+ Such crawlers may not respect robots.txt.
+ Note that users and user tools also see the addresses as redacted.
+ If Gitweb is not the final step in a workflow then subsequent steps
+ may misbehave because of the redacted information they receive.
+ Disabled by default.
+
highlight::
Server-side syntax highlight support in "blob" view. It requires
`$highlight_bin` program to be available (see the description of
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0959a782eccb..fe1dbc266ea7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -21,6 +21,7 @@
use File::Basename qw(basename);
use Time::HiRes qw(gettimeofday tv_interval);
use Digest::MD5 qw(md5_hex);
+use Git::LoadCPAN::Mail::Address;
binmode STDOUT, ':utf8';
@@ -569,6 +570,15 @@ sub evaluate_uri {
'sub' => \&feature_extra_branch_refs,
'override' => 0,
'default' => []},
+
+ # Redact e-mail addresses.
+
+ # To enable system wide have in $GITWEB_CONFIG
+ # $feature{'email-privacy'}{'default'} = [1];
+ 'email-privacy' => {
+ 'sub' => sub { feature_bool('email-privacy', @_) },
+ 'override' => 1,
+ 'default' => [0]},
);
sub gitweb_get_feature {
@@ -3449,6 +3459,33 @@ sub parse_date {
return %date;
}
+sub is_mailaddr {
+ my @addrs = Mail::Address->parse(shift);
+ if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
+ return 0;
+ }
+ return 1;
+}
+
+sub hide_mailaddrs_if_private {
+ my $line = shift;
+ return $line unless gitweb_check_feature('email-privacy');
+ while ($line =~ m/(<[^>]+>)/g) {
+ my $match = $1;
+ if (!is_mailaddr($match)) {
+ next;
+ }
+ my $match_offset = pos($line) - length($match);
+ pos $line = $match_offset;
+
+ my $redaction = "<redacted>";
+ $line =~ s/\G(<[^>]+>)/$redaction/;
+
+ pos $line = $match_offset + length($redaction);
+ }
+ return $line;
+}
+
sub parse_tag {
my $tag_id = shift;
my %tag;
@@ -3465,7 +3502,7 @@ sub parse_tag {
} elsif ($line =~ m/^tag (.+)$/) {
$tag{'name'} = $1;
} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
- $tag{'author'} = $1;
+ $tag{'author'} = hide_mailaddrs_if_private($1);
$tag{'author_epoch'} = $2;
$tag{'author_tz'} = $3;
if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3513,7 +3550,7 @@ sub parse_commit_text {
} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {
push @parents, $1;
} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
- $co{'author'} = to_utf8($1);
+ $co{'author'} = hide_mailaddrs_if_private(to_utf8($1));
$co{'author_epoch'} = $2;
$co{'author_tz'} = $3;
if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3523,7 +3560,7 @@ sub parse_commit_text {
$co{'author_name'} = $co{'author'};
}
} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {
- $co{'committer'} = to_utf8($1);
+ $co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));
$co{'committer_epoch'} = $2;
$co{'committer_tz'} = $3;
if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3568,9 +3605,10 @@ sub parse_commit_text {
if (! defined $co{'title'} || $co{'title'} eq "") {
$co{'title'} = $co{'title_short'} = '(no commit message)';
}
- # remove added spaces
+ # remove added spaces, redact e-mail addresses if applicable.
foreach my $line (@commit_lines) {
$line =~ s/^ //;
+ $line = hide_mailaddrs_if_private($line);
}
$co{'comment'} = \@commit_lines;
@@ -7489,7 +7527,8 @@ sub git_log_generic {
-accesskey => "n", -title => "Alt-n"}, "next");
}
my $patch_max = gitweb_get_feature('patches');
- if ($patch_max && !defined $file_name) {
+ if ($patch_max && !defined $file_name &&
+ !gitweb_check_feature('email-privacy')) {
if ($patch_max < 0 || @commitlist <= $patch_max) {
$paging_nav .= " ⋅ " .
$cgi->a({-href => href(action=>"patches", -replay=>1)},
@@ -7550,7 +7589,8 @@ sub git_commit {
} @$parents ) .
')';
}
- if (gitweb_check_feature('patches') && @$parents <= 1) {
+ if (gitweb_check_feature('patches') && @$parents <= 1 &&
+ !gitweb_check_feature('email-privacy')) {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
@@ -7863,7 +7903,8 @@ sub git_commitdiff {
$formats_nav =
$cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
"raw");
- if ($patch_max && @{$co{'parents'}} <= 1) {
+ if ($patch_max && @{$co{'parents'}} <= 1 &&
+ !gitweb_check_feature('email-privacy')) {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
diff --git a/t/lib-gitweb.sh b/t/lib-gitweb.sh
index 1f32ca66ea51..77fc1298d4c6 100644
--- a/t/lib-gitweb.sh
+++ b/t/lib-gitweb.sh
@@ -67,6 +67,9 @@ gitweb_run () {
GITWEB_CONFIG=$(pwd)/gitweb_config.perl
export GITWEB_CONFIG
+ PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
+ export PERL5LIB
+
# some of git commands write to STDERR on error, but this is not
# written to web server logs, so we are not interested in that:
# we are interested only in properly formatted errors/warnings
base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v6] gitweb: redacted e-mail addresses feature.
2021-03-27 3:56 ` [PATCH v5] " Georgios Kontaxis via GitGitGadget
@ 2021-03-28 23:26 ` Georgios Kontaxis via GitGitGadget
2021-03-29 20:00 ` Junio C Hamano
2021-04-08 22:43 ` Ævar Arnfjörð Bjarmason
2021-03-29 1:47 ` [PATCH v5] " Eric Wong
1 sibling, 2 replies; 41+ messages in thread
From: Georgios Kontaxis via GitGitGadget @ 2021-03-28 23:26 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
From: Georgios Kontaxis <geko1702+commits@99rst.org>
Gitweb extracts content from the Git log and makes it accessible
over HTTP. As a result, e-mail addresses found in commits are
exposed to web crawlers and they may not respect robots.txt.
This can result in unsolicited messages.
Introduce an 'email-privacy' feature which redacts e-mail addresses
from the generated HTML content. Specifically, obscure addresses
retrieved from the the author/committer and comment sections of the
Git log. The feature is off by default.
This feature does not prevent someone from downloading the
unredacted commit log, e.g., by cloning the repository, and
extracting information from it. It aims to hinder the low-
effort, bulk collection of e-mail addresses by web crawlers.
Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
---
gitweb: redacted e-mail addresses feature.
Changes since v1:
* Turned off the feature by default.
* Removed duplicate code.
* Added note about Gitweb consumers receiving redacted logs.
Changes since v2:
* The feature can be set on a per-project basis. ('override' => 1)
Changes since v3:
* Renamed feature to "email-privacy" and improved documentation.
* Removed UI elements for git-format-patch since it won't be redacted.
* Simplified calls to the address redaction logic.
* Mail::Address is now used to reduce false-positive redactions.
Changes since v4:
* Rephrased the commit comment.
* hide_mailaddrs_if_private is slighly more compact.
Changes since v5:
* A simple <local@domain> filter is used instead of Mail::Address to
identify addresses.
Signed-off-by: Georgios Kontaxis geko1702+commits@99rst.org
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-910%2Fkontaxis%2Fkontaxis%2Femail_privacy-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-910/kontaxis/kontaxis/email_privacy-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/910
Range-diff vs v5:
1: 1427231f9db5 ! 1: 245cfed8ad58 gitweb: redacted e-mail addresses feature.
@@ Documentation/gitweb.conf.txt: default font sizes or lineheights are changed (e.
`$highlight_bin` program to be available (see the description of
## gitweb/gitweb.perl ##
-@@
- use File::Basename qw(basename);
- use Time::HiRes qw(gettimeofday tv_interval);
- use Digest::MD5 qw(md5_hex);
-+use Git::LoadCPAN::Mail::Address;
-
- binmode STDOUT, ':utf8';
-
@@ gitweb/gitweb.perl: sub evaluate_uri {
'sub' => \&feature_extra_branch_refs,
'override' => 0,
@@ gitweb/gitweb.perl: sub parse_date {
return %date;
}
-+sub is_mailaddr {
-+ my @addrs = Mail::Address->parse(shift);
-+ if (!@addrs || !$addrs[0]->host || !$addrs[0]->user) {
-+ return 0;
-+ }
-+ return 1;
-+}
-+
+sub hide_mailaddrs_if_private {
+ my $line = shift;
+ return $line unless gitweb_check_feature('email-privacy');
-+ while ($line =~ m/(<[^>]+>)/g) {
-+ my $match = $1;
-+ if (!is_mailaddr($match)) {
-+ next;
-+ }
-+ my $match_offset = pos($line) - length($match);
-+ pos $line = $match_offset;
-+
-+ my $redaction = "<redacted>";
-+ $line =~ s/\G(<[^>]+>)/$redaction/;
-+
-+ pos $line = $match_offset + length($redaction);
-+ }
++ $line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;
+ return $line;
+}
+
@@ gitweb/gitweb.perl: sub git_commitdiff {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
-
- ## t/lib-gitweb.sh ##
-@@ t/lib-gitweb.sh: gitweb_run () {
- GITWEB_CONFIG=$(pwd)/gitweb_config.perl
- export GITWEB_CONFIG
-
-+ PERL5LIB="$GIT_BUILD_DIR/perl:$GIT_BUILD_DIR/perl/FromCPAN"
-+ export PERL5LIB
-+
- # some of git commands write to STDERR on error, but this is not
- # written to web server logs, so we are not interested in that:
- # we are interested only in properly formatted errors/warnings
Documentation/gitweb.conf.txt | 11 +++++++++++
gitweb/gitweb.perl | 34 +++++++++++++++++++++++++++-------
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index 7963a79ba98b..34b1d6e22435 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -751,6 +751,17 @@ default font sizes or lineheights are changed (e.g. via adding extra
CSS stylesheet in `@stylesheets`), it may be appropriate to change
these values.
+email-privacy::
+ Redact e-mail addresses from the generated HTML, etc. content.
+ This obscures e-mail addresses retrieved from the author/committer
+ and comment sections of the Git log.
+ It is meant to hinder web crawlers that harvest and abuse addresses.
+ Such crawlers may not respect robots.txt.
+ Note that users and user tools also see the addresses as redacted.
+ If Gitweb is not the final step in a workflow then subsequent steps
+ may misbehave because of the redacted information they receive.
+ Disabled by default.
+
highlight::
Server-side syntax highlight support in "blob" view. It requires
`$highlight_bin` program to be available (see the description of
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0959a782eccb..01c6faf88006 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -569,6 +569,15 @@ sub evaluate_uri {
'sub' => \&feature_extra_branch_refs,
'override' => 0,
'default' => []},
+
+ # Redact e-mail addresses.
+
+ # To enable system wide have in $GITWEB_CONFIG
+ # $feature{'email-privacy'}{'default'} = [1];
+ 'email-privacy' => {
+ 'sub' => sub { feature_bool('email-privacy', @_) },
+ 'override' => 1,
+ 'default' => [0]},
);
sub gitweb_get_feature {
@@ -3449,6 +3458,13 @@ sub parse_date {
return %date;
}
+sub hide_mailaddrs_if_private {
+ my $line = shift;
+ return $line unless gitweb_check_feature('email-privacy');
+ $line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;
+ return $line;
+}
+
sub parse_tag {
my $tag_id = shift;
my %tag;
@@ -3465,7 +3481,7 @@ sub parse_tag {
} elsif ($line =~ m/^tag (.+)$/) {
$tag{'name'} = $1;
} elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
- $tag{'author'} = $1;
+ $tag{'author'} = hide_mailaddrs_if_private($1);
$tag{'author_epoch'} = $2;
$tag{'author_tz'} = $3;
if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3513,7 +3529,7 @@ sub parse_commit_text {
} elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {
push @parents, $1;
} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
- $co{'author'} = to_utf8($1);
+ $co{'author'} = hide_mailaddrs_if_private(to_utf8($1));
$co{'author_epoch'} = $2;
$co{'author_tz'} = $3;
if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3523,7 +3539,7 @@ sub parse_commit_text {
$co{'author_name'} = $co{'author'};
}
} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {
- $co{'committer'} = to_utf8($1);
+ $co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));
$co{'committer_epoch'} = $2;
$co{'committer_tz'} = $3;
if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
@@ -3568,9 +3584,10 @@ sub parse_commit_text {
if (! defined $co{'title'} || $co{'title'} eq "") {
$co{'title'} = $co{'title_short'} = '(no commit message)';
}
- # remove added spaces
+ # remove added spaces, redact e-mail addresses if applicable.
foreach my $line (@commit_lines) {
$line =~ s/^ //;
+ $line = hide_mailaddrs_if_private($line);
}
$co{'comment'} = \@commit_lines;
@@ -7489,7 +7506,8 @@ sub git_log_generic {
-accesskey => "n", -title => "Alt-n"}, "next");
}
my $patch_max = gitweb_get_feature('patches');
- if ($patch_max && !defined $file_name) {
+ if ($patch_max && !defined $file_name &&
+ !gitweb_check_feature('email-privacy')) {
if ($patch_max < 0 || @commitlist <= $patch_max) {
$paging_nav .= " ⋅ " .
$cgi->a({-href => href(action=>"patches", -replay=>1)},
@@ -7550,7 +7568,8 @@ sub git_commit {
} @$parents ) .
')';
}
- if (gitweb_check_feature('patches') && @$parents <= 1) {
+ if (gitweb_check_feature('patches') && @$parents <= 1 &&
+ !gitweb_check_feature('email-privacy')) {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
@@ -7863,7 +7882,8 @@ sub git_commitdiff {
$formats_nav =
$cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
"raw");
- if ($patch_max && @{$co{'parents'}} <= 1) {
+ if ($patch_max && @{$co{'parents'}} <= 1 &&
+ !gitweb_check_feature('email-privacy')) {
$formats_nav .= " | " .
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v6] gitweb: redacted e-mail addresses feature.
2021-03-28 23:26 ` [PATCH v6] " Georgios Kontaxis via GitGitGadget
@ 2021-03-29 20:00 ` Junio C Hamano
2021-03-31 21:14 ` Junio C Hamano
2021-04-06 0:56 ` Junio C Hamano
2021-04-08 22:43 ` Ævar Arnfjörð Bjarmason
1 sibling, 2 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-29 20:00 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
"Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>
> Gitweb extracts content from the Git log and makes it accessible
> over HTTP. As a result, e-mail addresses found in commits are
> exposed to web crawlers and they may not respect robots.txt.
> This can result in unsolicited messages.
>
> Introduce an 'email-privacy' feature which redacts e-mail addresses
> from the generated HTML content. Specifically, obscure addresses
> retrieved from the the author/committer and comment sections of the
> Git log. The feature is off by default.
>
> This feature does not prevent someone from downloading the
> unredacted commit log, e.g., by cloning the repository, and
> extracting information from it. It aims to hinder the low-
> effort, bulk collection of e-mail addresses by web crawlers.
>
> Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
> ---
> @@ -751,6 +751,17 @@ default font sizes or lineheights are changed (e.g. via adding extra
> CSS stylesheet in `@stylesheets`), it may be appropriate to change
> these values.
>
> +email-privacy::
> + Redact e-mail addresses from the generated HTML, etc. content.
> + This obscures e-mail addresses retrieved from the author/committer
> + and comment sections of the Git log.
> + It is meant to hinder web crawlers that harvest and abuse addresses.
> + Such crawlers may not respect robots.txt.
> + Note that users and user tools also see the addresses as redacted.
> + If Gitweb is not the final step in a workflow then subsequent steps
> + may misbehave because of the redacted information they receive.
> + Disabled by default.
OK. I still think everything after "Note that" is a bit redandant
(as any intelligent reader can read what the feature does and reach
its natural consequence themselves), but I do not strongly oppose
leaving it in.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0959a782eccb..01c6faf88006 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -569,6 +569,15 @@ sub evaluate_uri {
> 'sub' => \&feature_extra_branch_refs,
> 'override' => 0,
> 'default' => []},
> +
> + # Redact e-mail addresses.
> +
> + # To enable system wide have in $GITWEB_CONFIG
> + # $feature{'email-privacy'}{'default'} = [1];
> + 'email-privacy' => {
> + 'sub' => sub { feature_bool('email-privacy', @_) },
> + 'override' => 1,
> + 'default' => [0]},
> );
Sensible.
While reviewing this part, I noticed a few things
- A few other features explain how to toggle it system wide and
nothing else, like this one does.
- Others explain, in addition, talk about how to toggle per
repository (which may necessitate explaining how to toggle the
override bit if the default is false).
It might be a good idea to standardise the explanation somehow.
This is just an observation (in other words, nothing needs to be
done in this patch).
> @@ -3449,6 +3458,13 @@ sub parse_date {
> return %date;
> }
>
> +sub hide_mailaddrs_if_private {
> + my $line = shift;
> + return $line unless gitweb_check_feature('email-privacy');
> + $line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;
> + return $line;
> +}
OK. The "this catches AUTHOR_EMAIL" pattern is a bit embarrassing
(my fault); "s/<[^@>]+@[-a-z0-9.]+>/<redacted>/ig" might be less
embarrassing but I do not care deeply enough either way. The version
in this patch is probably closer to what ident.c uses anyway ;-)
> @@ -7489,7 +7506,8 @@ sub git_log_generic {
> -accesskey => "n", -title => "Alt-n"}, "next");
> }
> my $patch_max = gitweb_get_feature('patches');
> - if ($patch_max && !defined $file_name) {
> + if ($patch_max && !defined $file_name &&
> + !gitweb_check_feature('email-privacy')) {
> if ($patch_max < 0 || @commitlist <= $patch_max) {
An observation unrelated to this change. I think checking for
negative patch_max is a bug in the original code. Everywhere else,
the way to disable the 'patch' view is to set it to 0, not negative,
and this block is already protected with "if ($patch_max".
> @@ -7550,7 +7568,8 @@ sub git_commit {
> } @$parents ) .
> ')';
> }
> - if (gitweb_check_feature('patches') && @$parents <= 1) {
> + if (gitweb_check_feature('patches') && @$parents <= 1 &&
> + !gitweb_check_feature('email-privacy')) {
> $formats_nav .= " | " .
> $cgi->a({-href => href(action=>"patch", -replay=>1)},
> "patch");
> @@ -7863,7 +7882,8 @@ sub git_commitdiff {
> $formats_nav =
> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
> "raw");
> - if ($patch_max && @{$co{'parents'}} <= 1) {
> + if ($patch_max && @{$co{'parents'}} <= 1 &&
> + !gitweb_check_feature('email-privacy')) {
> $formats_nav .= " | " .
> $cgi->a({-href => href(action=>"patch", -replay=>1)},
> "patch");
I see your wish is "we do not show any link to a patch page, but
anybody who knows what a link to the patch page looks like can craft
and ask", but I am not sure if that is worth the above three hunks.
It still feels more robust to just disable the 'patches' feature
when the email-privacy feature is enabled, but that may be just me.
Will queue as-is. Input from those who are more adept at Perl
and/or interested in helping polish gitweb are still welcome, but at
my level of interest on the topic, this version looks as good as it
gets ;-)
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v6] gitweb: redacted e-mail addresses feature.
2021-03-29 20:00 ` Junio C Hamano
@ 2021-03-31 21:14 ` Junio C Hamano
2021-04-06 0:56 ` Junio C Hamano
1 sibling, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-03-31 21:14 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
Junio C Hamano <gitster@pobox.com> writes:
> Will queue as-is. Input from those who are more adept at Perl
> and/or interested in helping polish gitweb are still welcome, but at
> my level of interest on the topic, this version looks as good as it
> gets ;-)
Oh by the way, as nobody commented on the title, I'd have to X-<.
I'll retitle it to
gitweb: add "e-mail privacy" feature to redact e-mail addresses
while queuing.
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6] gitweb: redacted e-mail addresses feature.
2021-03-29 20:00 ` Junio C Hamano
2021-03-31 21:14 ` Junio C Hamano
@ 2021-04-06 0:56 ` Junio C Hamano
1 sibling, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-04-06 0:56 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
Junio C Hamano <gitster@pobox.com> writes:
> "Georgios Kontaxis via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> From: Georgios Kontaxis <geko1702+commits@99rst.org>
>>
>> Gitweb extracts content from the Git log and makes it accessible
>> over HTTP. As a result, e-mail addresses found in commits are
>> exposed to web crawlers and they may not respect robots.txt.
>> This can result in unsolicited messages.
>> ...
> Will queue as-is. Input from those who are more adept at Perl
> and/or interested in helping polish gitweb are still welcome, but at
> my level of interest on the topic, this version looks as good as it
> gets ;-)
Comments by anybody who is more adept at Perl and/or more well
versed in gitweb and e-mail privacy issues?
Otherwise I am inclined to merge it down to 'next' as-is in a couple
of days.
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v6] gitweb: redacted e-mail addresses feature.
2021-03-28 23:26 ` [PATCH v6] " Georgios Kontaxis via GitGitGadget
2021-03-29 20:00 ` Junio C Hamano
@ 2021-04-08 22:43 ` Ævar Arnfjörð Bjarmason
2021-04-08 22:51 ` Junio C Hamano
1 sibling, 1 reply; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-04-08 22:43 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, brian m. carlson, Georgios Kontaxis
On Mon, Mar 29 2021, Georgios Kontaxis via GitGitGadget wrote:
> [...]
> +email-privacy::
> + Redact e-mail addresses from the generated HTML, etc. content.
> + This obscures e-mail addresses retrieved from the author/committer
> + and comment sections of the Git log.
> + It is meant to hinder web crawlers that harvest and abuse addresses.
> + Such crawlers may not respect robots.txt.
> + Note that users and user tools also see the addresses as redacted.
> + If Gitweb is not the final step in a workflow then subsequent steps
> + may misbehave because of the redacted information they receive.
> + Disabled by default.
> +
> highlight::
> Server-side syntax highlight support in "blob" view. It requires
> `$highlight_bin` program to be available (see the description of
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0959a782eccb..01c6faf88006 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -569,6 +569,15 @@ sub evaluate_uri {
> 'sub' => \&feature_extra_branch_refs,
> 'override' => 0,
> 'default' => []},
> +
> + # Redact e-mail addresses.
> +
> + # To enable system wide have in $GITWEB_CONFIG
> + # $feature{'email-privacy'}{'default'} = [1];
> + 'email-privacy' => {
> + 'sub' => sub { feature_bool('email-privacy', @_) },
> + 'override' => 1,
> + 'default' => [0]},
> );
>
> sub gitweb_get_feature {
> @@ -3449,6 +3458,13 @@ sub parse_date {
> return %date;
> }
>
> +sub hide_mailaddrs_if_private {
> + my $line = shift;
> + return $line unless gitweb_check_feature('email-privacy');
> + $line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;
The /i here is redundant, since you have nothing that'll case-fold on
the LHS of the s///, doesn't harm anything either. Just a small note
since it's new in v6...
> + return $line;
> +}
> +
> sub parse_tag {
> my $tag_id = shift;
> my %tag;
> @@ -3465,7 +3481,7 @@ sub parse_tag {
> } elsif ($line =~ m/^tag (.+)$/) {
> $tag{'name'} = $1;
> } elsif ($line =~ m/^tagger (.*) ([0-9]+) (.*)$/) {
> - $tag{'author'} = $1;
> + $tag{'author'} = hide_mailaddrs_if_private($1);
> $tag{'author_epoch'} = $2;
> $tag{'author_tz'} = $3;
> if ($tag{'author'} =~ m/^([^<]+) <([^>]*)>/) {
> @@ -3513,7 +3529,7 @@ sub parse_commit_text {
> } elsif ((!defined $withparents) && ($line =~ m/^parent ($oid_regex)$/)) {
> push @parents, $1;
> } elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
> - $co{'author'} = to_utf8($1);
> + $co{'author'} = hide_mailaddrs_if_private(to_utf8($1));
> $co{'author_epoch'} = $2;
> $co{'author_tz'} = $3;
> if ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {
> @@ -3523,7 +3539,7 @@ sub parse_commit_text {
> $co{'author_name'} = $co{'author'};
> }
> } elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {
> - $co{'committer'} = to_utf8($1);
> + $co{'committer'} = hide_mailaddrs_if_private(to_utf8($1));
> $co{'committer_epoch'} = $2;
> $co{'committer_tz'} = $3;
> if ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {
> @@ -3568,9 +3584,10 @@ sub parse_commit_text {
> if (! defined $co{'title'} || $co{'title'} eq "") {
> $co{'title'} = $co{'title_short'} = '(no commit message)';
> }
> - # remove added spaces
> + # remove added spaces, redact e-mail addresses if applicable.
> foreach my $line (@commit_lines) {
> $line =~ s/^ //;
> + $line = hide_mailaddrs_if_private($line);
> }
> $co{'comment'} = \@commit_lines;
>
> @@ -7489,7 +7506,8 @@ sub git_log_generic {
> -accesskey => "n", -title => "Alt-n"}, "next");
> }
> my $patch_max = gitweb_get_feature('patches');
> - if ($patch_max && !defined $file_name) {
> + if ($patch_max && !defined $file_name &&
> + !gitweb_check_feature('email-privacy')) {
> if ($patch_max < 0 || @commitlist <= $patch_max) {
> $paging_nav .= " ⋅ " .
> $cgi->a({-href => href(action=>"patches", -replay=>1)},
> @@ -7550,7 +7568,8 @@ sub git_commit {
> } @$parents ) .
> ')';
> }
> - if (gitweb_check_feature('patches') && @$parents <= 1) {
> + if (gitweb_check_feature('patches') && @$parents <= 1 &&
> + !gitweb_check_feature('email-privacy')) {
> $formats_nav .= " | " .
> $cgi->a({-href => href(action=>"patch", -replay=>1)},
> "patch");
> @@ -7863,7 +7882,8 @@ sub git_commitdiff {
> $formats_nav =
> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
> "raw");
> - if ($patch_max && @{$co{'parents'}} <= 1) {
> + if ($patch_max && @{$co{'parents'}} <= 1 &&
> + !gitweb_check_feature('email-privacy')) {
> $formats_nav .= " | " .
> $cgi->a({-href => href(action=>"patch", -replay=>1)},
> "patch");
I didn't run this, and hadn't kept up for a few rounds. I'm happy to see
the pos/while etc. looping gone, this LGTM.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v6] gitweb: redacted e-mail addresses feature.
2021-04-08 22:43 ` Ævar Arnfjörð Bjarmason
@ 2021-04-08 22:51 ` Junio C Hamano
0 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-04-08 22:51 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Georgios Kontaxis via GitGitGadget, git, brian m. carlson,
Georgios Kontaxis
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> +sub hide_mailaddrs_if_private {
>> + my $line = shift;
>> + return $line unless gitweb_check_feature('email-privacy');
>> + $line =~ s/<[^@>]+@[^>]+>/<redacted>/ig;
>
> The /i here is redundant, since you have nothing that'll case-fold on
> the LHS of the s///, doesn't harm anything either. Just a small note
> since it's new in v6...
True. If it were left original version that was suggested during
the review, e.g.
s/<[^@]+@[-.a-z0-9]+>/<red@cted>/ig;
the /i/ would have been needed, but without the "after the @-sign
should be a run of DNS-valid characters", I agree that there is no
need.
Thanks. Will locally tweak.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-03-27 3:56 ` [PATCH v5] " Georgios Kontaxis via GitGitGadget
2021-03-28 23:26 ` [PATCH v6] " Georgios Kontaxis via GitGitGadget
@ 2021-03-29 1:47 ` Eric Wong
2021-03-29 3:17 ` Georgios Kontaxis
1 sibling, 1 reply; 41+ messages in thread
From: Eric Wong @ 2021-03-29 1:47 UTC (permalink / raw)
To: Georgios Kontaxis via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson,
Georgios Kontaxis
Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:
> Gitweb extracts content from the Git log and makes it accessible
> over HTTP. As a result, e-mail addresses found in commits are
> exposed to web crawlers and they may not respect robots.txt.
> This can result in unsolicited messages.
> Introduce an 'email-privacy' feature which redacts e-mail addresses
> from the generated HTML content
A general reply to the topic: have you considered munging
addresses in a way that is still human readable, but obviously
obfuscated?
On some other project, I settled on HTML "•" as a replacement
for '.' for admins who enable that option. The $USER@$NO_DOT
remains as-is for easy identification+recognition of hosts.
I also considered Unicode homographs which can look identical
to replacement characters, too; but rejected that idea since
it would cause grief for legitimate users who would not notice
the homograph when pasting into their mail client.
Anyways, here's the list of candidates I tried:
homograph∂80x24.org
homograph@80x24ͺorg
homograph@80x24·org
homograph@80x24•org
homograph＠80x24.org
homograph﹫80x24.org
https://en.wikipedia.org/wiki/Ano_Teleia#Similar_symbols
https://en.wikipedia.org/wiki/Enclosed_A
homographⒶ80x24.org
homograph@80x24 org
homograph@80x24․org
homograph@80x24ꓸorg
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-03-29 1:47 ` [PATCH v5] " Eric Wong
@ 2021-03-29 3:17 ` Georgios Kontaxis
2021-04-08 17:16 ` Eric Wong
0 siblings, 1 reply; 41+ messages in thread
From: Georgios Kontaxis @ 2021-03-29 3:17 UTC (permalink / raw)
To: Eric Wong
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
> Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:
>> Gitweb extracts content from the Git log and makes it accessible
>> over HTTP. As a result, e-mail addresses found in commits are
>> exposed to web crawlers and they may not respect robots.txt.
>> This can result in unsolicited messages.
>
>> Introduce an 'email-privacy' feature which redacts e-mail addresses
>> from the generated HTML content
>
> A general reply to the topic: have you considered munging
> addresses in a way that is still human readable, but obviously
> obfuscated?
>
> On some other project, I settled on HTML "•" as a replacement
> for '.' for admins who enable that option. The $USER@$NO_DOT
> remains as-is for easy identification+recognition of hosts.
>
Thanks for the suggestion.
People have been trying to hinder address harvesting for a while now.
Replacing '@' with "at", the dot with "dot", adding spaces, etc.
was pretty common at some point. May still be.
I would expect crawlers to have caught up and this includes
all sorts of character encodings and unicode look-alike substitutions.
At the end of the day we are looking for something that's easy for humans
to read but hard for scripts to parse as an e-mail address.
(And that scripts cannot learn through an additional regex)
I'm not aware of anything like that. (I know CAPTCHAs, etc.)
> I also considered Unicode homographs which can look identical
> to replacement characters, too; but rejected that idea since
> it would cause grief for legitimate users who would not notice
> the homograph when pasting into their mail client.
>
> Anyways, here's the list of candidates I tried:
>
> homograph∂80x24.org
> homograph@80x24ͺorg
> homograph@80x24·org
> homograph@80x24•org
> homograph＠80x24.org
> homograph﹫80x24.org
>
> https://en.wikipedia.org/wiki/Ano_Teleia#Similar_symbols
> https://en.wikipedia.org/wiki/Enclosed_A
>
> homographⒶ80x24.org
> homograph@80x24 org
> homograph@80x24․org
> homograph@80x24ꓸorg
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-03-29 3:17 ` Georgios Kontaxis
@ 2021-04-08 17:16 ` Eric Wong
2021-04-08 21:04 ` Junio C Hamano
0 siblings, 1 reply; 41+ messages in thread
From: Eric Wong @ 2021-04-08 17:16 UTC (permalink / raw)
To: Georgios Kontaxis
Cc: Georgios Kontaxis via GitGitGadget, git,
"Ævar Arnfjörð Bjarmason",
brian m. carlson
Georgios Kontaxis <geko1702+commits@99rst.org> wrote:
> > Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:
> >> Introduce an 'email-privacy' feature which redacts e-mail addresses
> >> from the generated HTML content
> >
> Eric Wong wrote:
> > A general reply to the topic: have you considered munging
> > addresses in a way that is still human readable, but obviously
> > obfuscated?
> >
> > On some other project, I settled on HTML "•" as a replacement
> > for '.' for admins who enable that option. The $USER@$NO_DOT
> > remains as-is for easy identification+recognition of hosts.
> >
> Thanks for the suggestion.
>
> People have been trying to hinder address harvesting for a while now.
> Replacing '@' with "at", the dot with "dot", adding spaces, etc.
> was pretty common at some point. May still be.
> I would expect crawlers to have caught up and this includes
> all sorts of character encodings and unicode look-alike substitutions.
I figure the crawlers hit a combinatorial explosion and
give up since they'd be wasting time with false-positives.
> > I also considered Unicode homographs which can look identical
> > to replacement characters, too; but rejected that idea since
> > it would cause grief for legitimate users who would not notice
> > the homograph when pasting into their mail client.
As a data point, none of the homograph@ candidates I posted here
on Mar 29 have attracted any attempts on my mail server.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-04-08 17:16 ` Eric Wong
@ 2021-04-08 21:04 ` Junio C Hamano
2021-04-08 21:19 ` Eric Wong
0 siblings, 1 reply; 41+ messages in thread
From: Junio C Hamano @ 2021-04-08 21:04 UTC (permalink / raw)
To: Eric Wong
Cc: Georgios Kontaxis, Georgios Kontaxis via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, brian m. carlson
Eric Wong <e@80x24.org> writes:
> Georgios Kontaxis <geko1702+commits@99rst.org> wrote:
>> > Georgios Kontaxis via GitGitGadget <gitgitgadget@gmail.com> wrote:
>> >> Introduce an 'email-privacy' feature which redacts e-mail addresses
>> >> from the generated HTML content
>> >
>> Eric Wong wrote:
>> > A general reply to the topic: have you considered munging
>> > addresses in a way that is still human readable, but obviously
>> > obfuscated?
>> ...
>> > I also considered Unicode homographs which can look identical
>> > to replacement characters, too; but rejected that idea since
>> > it would cause grief for legitimate users who would not notice
>> > the homograph when pasting into their mail client.
>
> As a data point, none of the homograph@ candidates I posted here
> on Mar 29 have attracted any attempts on my mail server.
That is an interesting observation. All homograph@ non-addresses,
if a human corrected the funnies in their spelling, would have hit
whoever handles @80x24.org mailboxes.
I take it to mean that as a future direction, replacing <redacted>
with the obfuscated-but-readable-by-humans homographs is a likely
improvement that would help human users while still inconveniencing
the crawlers. It may however need some provision to prevent casual
end-users from cutting-and-pasting these homographs, as you said in
your original mention of the homograph approach.
But other than that, does the patch look reasonable?
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-04-08 21:04 ` Junio C Hamano
@ 2021-04-08 21:19 ` Eric Wong
2021-04-08 22:45 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 41+ messages in thread
From: Eric Wong @ 2021-04-08 21:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Georgios Kontaxis, Georgios Kontaxis via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, brian m. carlson
Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <e@80x24.org> writes:
> > As a data point, none of the homograph@ candidates I posted here
> > on Mar 29 have attracted any attempts on my mail server.
>
> That is an interesting observation. All homograph@ non-addresses,
> if a human corrected the funnies in their spelling, would have hit
> whoever handles @80x24.org mailboxes.
>
> I take it to mean that as a future direction, replacing <redacted>
> with the obfuscated-but-readable-by-humans homographs is a likely
> improvement that would help human users while still inconveniencing
> the crawlers. It may however need some provision to prevent casual
> end-users from cutting-and-pasting these homographs, as you said in
> your original mention of the homograph approach.
Yes, exactly.
> But other than that, does the patch look reasonable?
I only took a cursory glance at it, but v6 seemed fine.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-04-08 21:19 ` Eric Wong
@ 2021-04-08 22:45 ` Ævar Arnfjörð Bjarmason
2021-04-08 22:54 ` Junio C Hamano
0 siblings, 1 reply; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-04-08 22:45 UTC (permalink / raw)
To: Eric Wong
Cc: Junio C Hamano, Georgios Kontaxis,
Georgios Kontaxis via GitGitGadget, git, brian m. carlson
On Thu, Apr 08 2021, Eric Wong wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>> Eric Wong <e@80x24.org> writes:
>> > As a data point, none of the homograph@ candidates I posted here
>> > on Mar 29 have attracted any attempts on my mail server.
>>
>> That is an interesting observation. All homograph@ non-addresses,
>> if a human corrected the funnies in their spelling, would have hit
>> whoever handles @80x24.org mailboxes.
>>
>> I take it to mean that as a future direction, replacing <redacted>
>> with the obfuscated-but-readable-by-humans homographs is a likely
>> improvement that would help human users while still inconveniencing
>> the crawlers. It may however need some provision to prevent casual
>> end-users from cutting-and-pasting these homographs, as you said in
>> your original mention of the homograph approach.
>
> Yes, exactly.
>
>> But other than that, does the patch look reasonable?
>
> I only took a cursory glance at it, but v6 seemed fine.
Ditto, I left a small nit comment about a needless /i in a regex, but I
don't think that needs a re-roll.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5] gitweb: redacted e-mail addresses feature.
2021-04-08 22:45 ` Ævar Arnfjörð Bjarmason
@ 2021-04-08 22:54 ` Junio C Hamano
0 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-04-08 22:54 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Eric Wong, Georgios Kontaxis, Georgios Kontaxis via GitGitGadget,
git, brian m. carlson
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On Thu, Apr 08 2021, Eric Wong wrote:
>
>> Junio C Hamano <gitster@pobox.com> wrote:
>>> Eric Wong <e@80x24.org> writes:
>>> > As a data point, none of the homograph@ candidates I posted here
>>> > on Mar 29 have attracted any attempts on my mail server.
>>>
>>> That is an interesting observation. All homograph@ non-addresses,
>>> if a human corrected the funnies in their spelling, would have hit
>>> whoever handles @80x24.org mailboxes.
>>>
>>> I take it to mean that as a future direction, replacing <redacted>
>>> with the obfuscated-but-readable-by-humans homographs is a likely
>>> improvement that would help human users while still inconveniencing
>>> the crawlers. It may however need some provision to prevent casual
>>> end-users from cutting-and-pasting these homographs, as you said in
>>> your original mention of the homograph approach.
>>
>> Yes, exactly.
>>
>>> But other than that, does the patch look reasonable?
>>
>> I only took a cursory glance at it, but v6 seemed fine.
>
> Ditto, I left a small nit comment about a needless /i in a regex, but I
> don't think that needs a re-roll.
Thanks, both.
Will tweak the /i out, and re-queue with acked-by from you two.
^ permalink raw reply [flat|nested] 41+ messages in thread