659185 - html_quote escapes @ causing mailto links to not be processed

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[kevin]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-us) AppleWebKit/533.21.1 (KHTML, like Gecko) Version/5.0.5 Safari/533.21.1
Build Identifier: trunk

[e-mail] in a bug comment is not "autolinkified" to a mailto: url.  
html_quote escapes the 
@
which becomes
@

so the regex doesn't match the pattern anymore.

Reproducible: Always




here's a patch off today's trunk:

=== modified file 'Bugzilla/Template.pm'
--- Bugzilla/Template.pm	2011-04-27 22:20:55 +0000
+++ Bugzilla/Template.pm	2011-05-23 22:46:40 +0000
@@ -218,6 +218,16 @@
                ("\0\0" . ($count-1) . "\0\0")
               ~egox;
 
+    # mailto:
+    # Use |<nothing> so that $1 is defined regardless
+    my $tmpb;
+    $text =~ s~\b(mailto:|)?([\w\.\-\+\=]+\@[\w\-]+(?:\.[\w\-]+)+)\b
+              ~($tmp = html_quote($2)) &&
+               ($tmpb = html_quote($3)) &&
+               ($things[$count++] = "<a href=\"mailto:$tmp\">$tmp$tmpb</a>") &&
+               ("\0\0" . ($count-1) . "\0\0")
+              ~egxi;
+
     # We have to quote now, otherwise the html itself is escaped
     # THIS MEANS THAT A LITERAL ", <, >, ' MUST BE ESCAPED FOR A MATCH
 
@@ -227,11 +237,6 @@
     $text =~ s~^(&gt;.+)$~<span class="quote">$1</span >~mg;
     $text =~ s~</span >\n<span class="quote">~\n~g;
 
-    # mailto:
-    # Use |<nothing> so that $1 is defined regardless
-    $text =~ s~\b(mailto:|)?([\w\.\-\+\=]+\@[\w\-]+(?:\.[\w\-]+)+)\b
-              ~<a href=\"mailto:$2\">$1$2</a>~igx;
-
     # attachment links
     $text =~ s~\b(attachment\s*\#?\s*(\d+)(?:\s+\[details\])?)
               ~($things[$count++] = get_attachment_link($2, $1)) &&

Comment 1

[Frédéric Buclin]

Works in Bugzilla 3.4. Broken in 3.6 and newer.

Comment 2

[Frédéric Buclin]

Regression due to the merge of html() and html_quote() in bug 476305.

Comment 3

[Frédéric Buclin]

Attachment: patch, v1

Comment 4

[kevin]

patch, v1 does not work for me.

Comment 5

[Frédéric Buclin]

(In reply to comment #4)
> patch, v1 does not work for me.

Did you back out your patch from comment 0? If yes, which version of Perl are you using?

Comment 6

[kevin]

my patch was backed out before testing patch, v1


root@bugs:/home/bugzilla# perl -V
Summary of my perl5 (revision 5 version 10 subversion 1) configuration:
   
  Platform:
    osname=linux, osvers=2.6.24-27-server, archname=i486-linux-gnu-thread-multi
    uname='linux vernadsky 2.6.24-27-server #1 smp fri mar 12 01:45:06 utc 2010 i686 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.10.1 -Dsitearch=/usr/local/lib/perl/5.10.1 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.10.1 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.3', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /usr/lib64
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.11.1.so, so=so, useshrplib=true, libperl=libperl.so.5.10.1
    gnulibc_version='2.11.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector'


Characteristics of this binary (from libperl): 
  Compile-time options: MULTIPLICITY PERL_DONT_CREATE_GVSV
                        PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP USE_ITHREADS
                        USE_LARGE_FILES USE_PERLIO USE_REENTRANT_API
  Built under linux
  Compiled at Apr 23 2010 07:36:53
  @INC:
    /etc/perl
    /usr/local/lib/perl/5.10.1
    /usr/local/share/perl/5.10.1
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.10
    /usr/share/perl/5.10
    /usr/local/lib/site_perl

Comment 7

[Frédéric Buclin]

So we use the same version of Perl. Look at the source code of the HTML page, and see how the email address is escaped. It should match my regexp (and it does in my testing).

Comment 8

[kevin]

with patch v. 1  the first line of the comment that has the header info (name, icon, date, Comment #)
has the email like this:
<a class="email" href="mailto:kevin&#64;example.com" title="Kevin Connor &lt;kevin&#64;example.com&gt;">

the email in the actual comment text no longer appears in the content - so it's being matched but not printed.  That  email address was of the form "[e-mail]"

Comment 9

[Frédéric Buclin]

(In reply to comment #8)
> the email in the actual comment text no longer appears in the content - so
> it's being matched but not printed.

Interesting. This is working fine in my local installation. And on our test installation on landfill too, see https://landfill.bugzilla.org/bugzillaqa/show_bug.cgi?id=3101#c1.

Comment 10

[kevin]

looks good to me (both on landfill and locally).  sorry for the confusion.  must have been a caching issue.  that's my story and I'm sticking to it.

Comment 11

[Max Kanat-Alexander]

Comment on attachment 534621 [details] [diff] [review]
patch, v1

Looks good to me.

Comment 12

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Template.pm
Committed revision 7824.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified Bugzilla/Template.pm
Committed revision 7600.