html_quote escapes @ causing mailto links to not be processed

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
Creating/Changing Bugs
--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: kevin, Assigned: Frédéric Buclin)

Tracking

({regression})

3.6.5
Bugzilla 4.0
regression
Bug Flags:
approval +
approval4.0 +

Details

Attachments

(1 attachment)

704 bytes, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
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

user@example.com 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)) &&
(Assignee)

Comment 1

7 years ago
Works in Bugzilla 3.4. Broken in 3.6 and newer.
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86 → All
Version: unspecified → 3.6.5
(Assignee)

Comment 2

7 years ago
Regression due to the merge of html() and html_quote() in bug 476305.
Depends on: 476305
Keywords: regression
Target Milestone: --- → Bugzilla 4.0
(Assignee)

Comment 3

7 years ago
Created attachment 534621 [details] [diff] [review]
patch, v1
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attachment #534621 - Flags: review?(mkanat)
(Reporter)

Comment 4

7 years ago
patch, v1 does not work for me.
(Assignee)

Comment 5

7 years ago
(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?
(Reporter)

Comment 6

7 years ago
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
(Assignee)

Comment 7

7 years ago
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).
(Reporter)

Comment 8

7 years ago
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 "kevin@example.com"
(Assignee)

Comment 9

7 years ago
(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.
(Reporter)

Comment 10

7 years ago
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

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

Looks good to me.
Attachment #534621 - Flags: review?(mkanat) → review+

Updated

7 years ago
Flags: approval4.2+
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 12

7 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Template.pm
Committed revision 7824.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Template.pm
Committed revision 7600.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Flags: approval4.2+
You need to log in before you can comment on or make changes to this bug.