Uidisplay link handling inconsistencies / redirect.php breakage

Oliver_Ackermann 2009-08-18 18:24:12 UTC #1

Hi, I’m currently using eGroupware 1.6.001 and I’m currently testing the svn version of 1.6. I’ve noticed that the handling of web site links has changed as of 1.6.001 which leads to 1) different way that the links are displayed and 2) breaks some links (with variables). Going through the source of class.uidisplay.inc.php (located in egroupware/felamimail/inc) I’ve noticed some changes that seem to contribute to both problems mentioned above.

Both issues seems related to the code in the function &getdisplayableBody.

In version 1.6.001, the link conversion for plain/text and other e-mails are all handled within the function itself, such as:

// create links for websites
$newBody = preg_replace("/((http(s?):\/\/)|(www\.))([\w,\-,\/,\?,\=,\.,&amp;,!\n,!&gt;,\%,@,\*,#,:,~,\+]+)/ie",
"'<a href="#">displayCharset\").'\" target=\"_blank\"><font color=\"blue\">$2$4$5</font></a>'", $newBody);

whereas in the svn version, you use the functions html::activate_links (located in egroupware/phpgwapi/inc) and parseHREF (located in class.uidisplay.inc.php) to do the conversion.

And now to the differences and the issues they cause:

in 1.6.001, all the code for http link conversion include the target="_blank" attribute for the tag that is created for the link. As of the svn version, the html::activate_links functions uses the target attribute, but parseHREF does not. Thus, HTML e-mails received in the svn version of eGroupware do not open their html links in a new window, but in the e-mail window, which makes link viewing inconsistent in the svn version of eGroupware (in 1.6.001, all links opened w/target="_blank", in svn, text/plain messages have their links converted w/target="_blank", but html links do not). Is this how it should be, or does parseHREF need to be modified to include target="_blank" ?
As of svn, html links that contain variables, are broken. It looks like this may have been addressed with r27288. The reason for the breakage may be the ?incorrect? calling of the redirect.php module itself. Please note that the notes in redirect.php say:

Use this in your app:

"’

1.6.001 seems to adhere to this standard (see example above), but the svn version of uidisplay do not. The code in parseHREF for creating the link is

$comp_uri = "<a href="#"><raw>
$body = str_replace('<a href="#">

(sorry for the extra raws, looks like the two line code snippet is given the raws a fit)

and for html::activate_links is

$result = preg_replace( $Expr, "<a href="#" target=\"_blank\">$2$3$4</a>", $result )
.
.
return preg_replace( $Expr, "<a href="#" target=\"_blank\">$0</a>", $result );

Notice that neither parseHREF, nor html::activate_links use the “htmlentities(urlencode(” part to create their links and therefore, redirect.php breaks. Please note that the solution provided in r27288 works, but should redirect.php be modified because it is being called in a non standard way or should the calling functions be modified?

Thanks,

Oliver.

Leithoff_Klaus 2009-08-19 07:29:26 UTC #2

Dear Oliver,
note that redirect.php has changed since that as well (you noted that at the
end of your posting).
If you use the one which is (after checkout) stored in egroupware directory,
this problem, is
solved, as you mentioned at the end of the posting.
The “htmlentities(urlencode(…” caused different problems with the work in
progress.
I could not solve at that time. The work there is not finished yet, as it is
partly an issue of getCleanHTML.
It is intended to switch to htmlpurifier instead of kses. Some other issues
should be
resolved with that.

As people do the definitions of software, nothing stands in the way to
modify redirect.php
We should have modified the comment too. true.

The link opening issue is true: not consistent, and not intentionally.
Thanks for working out the
details. I will try to tackle this, when I am getting to the
kses/htmlpurifier issue.

Best regards
Leithoff, Klaus

Oliver_Ackermann 2009-08-19 20:41:53 UTC #3

Hi Klaus,

Thanks for taking the time to answer.

As to the modified redirect.php being available in svn, please note that (at least to me) it looks like it is only available in trunk, but not in branch 1.6 (note my issues were w/branch 1.6, not trunk). I manually applied the trunk updates to the 1.6 svn copy (and it works).
The different behavior from 1.6.001 to svn (branch 1.6) for html anchors in html messages currently stops me from updating 1.6.001 to 1.6.002 (from what I can tell, uidisplay is the same for 1.6.002 and current svn of 1.6 branch). If it were just me, I probably could live w/it, but it will be my users that will get confused and wonder what is going on (they are finally getting used to eGroupware after coming from Groupwise 5.5EP. Any new “glitches” in eGroupware would just frustrate them). Since 1.6.002 (and svn) seem to have several security fixes, I might try to tackle the problem w/the link creation of parseHREF and the missing target="_blank" (even though it looks a tad more complicated than the link creation in 1.6.001).
Thanks for pointing out htmlpurifier. I currently have to modify some parameters in class.bofelamimail.inc.php in order to get my mxlogic html e-mails to show properly in felamimail and it looks like I need to study up on htmlpurifier.
If needed, I can volunteer some time to the project (especially w/felamimail, since this is used the most by the users).

Thanks and thanks for eGroupware!

Oliver.

Leithoff:

Dear Oliver,
note that redirect.php has changed since that as well (you noted that at the
end of your posting).
If you use the one which is (after checkout) stored in egroupware directory,
this problem, is
solved, as you mentioned at the end of the posting.
The “htmlentities(urlencode(…” caused different problems with the work in
progress.
I could not solve at that time. The work there is not finished yet, as it is
partly an issue of getCleanHTML.
It is intended to switch to htmlpurifier instead of kses. Some other issues
should be
resolved with that.

As people do the definitions of software, nothing stands in the way to
modify redirect.php
We should have modified the comment too. true.

The link opening issue is true: not consistent, and not intentionally.
Thanks for working out the
details. I will try to tackle this, when I am getting to the
kses/htmlpurifier issue.

Best regards
Leithoff, Klaus

----------------ursprüngliche Nachricht-----------------
Von: “Oliver Ackermann” olivera@b-belec.com
An: egroupware-developers@lists.sourceforge.net
Datum: Tue, 18 Aug 2009 11:24:12 -0700 (PDT)

Hi, I’m currently using eGroupware 1.6.001 and I’m currently testing the
svn
version of 1.6. I’ve noticed that the handling of web site links has
changed
as of 1.6.001 which leads to 1) different way that the links are displayed
and 2) breaks some links (with variables). Going through the source of
class.uidisplay.inc.php (located in egroupware/felamimail/inc) I’ve
noticed
some changes that seem to contribute to both problems mentioned above.

Both issues seems related to the code in the function &getdisplayableBody.

In version 1.6.001, the link conversion for plain/text and other e-mails
are
all handled within the function itself, such as:

// create links for websites
$newBody =

preg_replace("/((http(s?)://)|(www.))([\w,-,/,?,=,.,&,!
n,!>,%,@,*,#,:,~,+]+)/ie",
"’

“$webserverURL/redirect.php?go=’.@htmlentities(urlencode(‘http$3://
$4$5’),ENT_QUOTES,”$this-
displayCharset").’" target="_blank">$2$4$5 '", $newBody);

whereas in the svn version, you use the functions html::activate_links
(located in egroupware/phpgwapi/inc) and parseHREF (located in
class.uidisplay.inc.php) to do the conversion.

And now to the differences and the issues they cause:

in 1.6.001, all the code for http link conversion include the
target="_blank" attribute for the tag that is created for the link. As of
the svn version, the html::activate_links functions uses the target
attribute, but parseHREF does not. Thus, HTML e-mails received in the svn
version of eGroupware do not open their html links in a new window, but in
the e-mail window, which makes link viewing inconsistent in the svn
version
of eGroupware (in 1.6.001, all links opened w/target="_blank", in svn,
text/plain messages have their links converted w/target="_blank", but html
links do not). Is this how it should be, or does parseHREF need to be
modified to include target="_blank" ?

As of svn, html links that contain variables, are broken. It looks like
this may have been addressed with r27288. The reason for the breakage may
be
the ?incorrect? calling of the redirect.php module itself. Please note
that
the notes in redirect.php say:

Use this in your app:

"

“$webserverURL/redirect.php?go=”.htmlentities(urlencode(‘http://www
.egroupware.org’)).’"
’

1.6.001 seems to adhere to this standard (see example above), but the svn
version of uidisplay do not. The code in parseHREF for creating the link
is

$comp_uri = " “$webserverURL/redirect.php?go=”.$link; $body =
str_replace(’ "’.$link, $comp_uri, $body);

(sorry for the extra raws, looks like the two line code snippet is given
the
raws a fit)

and for html::activate_links is

$result = preg_replace( $Expr, " “$0” $2$3$4 ", $result )
.
.
return preg_replace( $Expr, " “http://$0” $0 ", $result );

Notice that neither parseHREF, nor html::activate_links use the
"htmlentities(urlencode(" part to create their links and therefore,
redirect.php breaks. Please note that the solution provided in r27288
works,
but should redirect.php be modified because it is being called in a non
standard way or should the calling functions be modified?

Thanks,

Oliver.

View this message in context:
http://www.nabble.com/uidisplay-link-handling-inconsistencies---redi
rect.php-breakage-tp25030760s3741p25030760.html
Sent from the egroupware-developers mailing list archive at Nabble.com.

Let Crystal Reports handle the reporting - Free Crystal Reports 2008
30-Day
trial. Simplify your report design, integration and deployment - and focus
on
what you do best, core application coding. Discover what’s new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july

eGroupWare-developers mailing list
eGroupWare-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/egroupware-developers

–
Stylite GmbH
[ open style of IT ]
Morschheimer Strasse 15
67292 Kirchheimbolanden

fon 06352 . 70629-0
fax 06352 . 70629-30
www.stylite.de

Geschäftsführer: Andre Keller, Gudrun Müller, Nigel Vickers, Ralf Becker
Handelsregister Kaiserslautern HRB 30575
USt-ID: DE214280951
SteuerNr. 44/667/1114/3

Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what’s new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july

eGroupWare-developers mailing list
eGroupWare-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/egroupware-developers

Leithoff_Klaus 2009-08-26 11:56:45 UTC #4

Hi Oliver,
too true; I missed that on commiting.

A great improvement would be, to get htmlpurifier to work with felamimail
and produce at least the same kind of result, as the usage of kses does.
I had a quick start with it in trunk and dropped it, since other stuff was
more pressing.
It could resolve some of the link issues altogether, as we may want to trust

htmlpurifier “more”, and may have no special need of linkhandling anymore.

best regards
Klaus

Hans-Jurgen_Tappe 2009-08-26 13:52:44 UTC #5

Hi Klaus!

Reading your change in redirect.php, I think, it’s a security hole that
it does not check whether the resulting link will stay inside the domain.

Take a phishing mail which advertises GREAT groupware software but takes
your company’s name to redirect to some unrelated domain:

http://www.stylite.de/egw/redirect.php?go=http%3A%2F%2Fgoogle.de/search?q=eGroupware
(replace eGroupware with any competitor’s groupware)

You should definitely add a check (see lines 25 in
sitemgr/sitemgr-site/index.php) and the in_array function with some
permissions for relative paths…

Thanks and best regards,
Hans-Jürgen

Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what’s new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july

eGroupWare-developers mailing list
eGroupWare-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/egroupware-developers