Re: [PATCH v4] gitweb: redacted e-mail addresses feature.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> "Georgios Kontaxis" <geko1702+commits@xxxxxxxxx> 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?





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux