Merge lp:~edb/quam-plures/plugins_use_long_desc into lp:quam-plures

[EdB]

http://forums.quamplures.net/viewtopic.php?f=6&t=900

item #1: Use long description for the credits page. Discussed elsewhere, seemingly no reason why not.

[Tilman Blumenbach (tblue)]

Easy review.

review: Approve

[lp:~edb/quam-plures/plugins_use_long_desc]

7620. By EdB on 2011-09-01

core to 7620

7621. By EdB on 2011-09-01

core to 7624

[Lee Turner (leeturner)]

Downloaded and tested. Works fine.

One question - not a reason not to approve though as it was still a question when the short desc was being used but do we have any cause for concern with links, embedded JS in the short or long desc ?

review: Approve

[EdB (edb)]

OT I'm glad I broke this up into specific-focus branches :)

I am 90% sure that there will be no problem with most usual and customary HTML as long as going inside a DL is not going to be an issue for it. I currently have a UL with LIs but can't think of any links I've put in. Dunno about JS though. Never gave it a thought.

I'll test to make sure AFTER I give up debugging why "twitter with 3 accounts" won't connect to a an account if it is coming from the Blog Settings instead of a User's profile.

[Lee Turner (leeturner)]

OK, cool. I guess there is the legitimate use of html in those fields and then there is the not so legitimate where a plugin author creates a plugin just to embed a porn video into the credits page or something like that :-)

I guess this was always a possibility but now we are displaying them on the front end, the exposure would be a little bigger instead of just the people who have access to the admin interface.

L

[EdB (edb)]

I am adding core updates AND retesting everything I have in merge just to be sure, but it is taking a long time due to many conflicts this time around. Will send another comment when I got them back up to speed.

[lp:~edb/quam-plures/plugins_use_long_desc]

7622. By EdB on 2011-09-04

core to 7628

[EdB (edb)]

This one is good to go - no issues when re-testing after merging from core.

Although I still don't know about adding a script into the description fields because I don't know how to stick one in for testing purposes.

[Tilman Blumenbach (tblue)]

Just checked, and YES, it's possible to (accidentally or purposely) inject HTML in several places in the credits display handler.

There are several vulnerable places in the two files; one may argue that a template author can "inject" arbitrary HTML anyway, but even if he doesn't want to do that, he may rely on us escaping his credits and forget to do that himself, breaking the template by accident. So it's better to escape everything the right way.

Basically, everything that is being output should be wrapped by a call to format_to_output(), the first argument being the data that should be "formatted" in a more safe way; the second argument specifies how to format the data. In this case it needs to be:

- 'htmlattr' if the data is being output as an attribute of a HTML tag, e. g. '<a href="'.format_to_output( $url, 'htmlattr' ).'>'
- 'text' if we want text WITHOUT any HTML tags.
- 'htmlbody' if we want text that allows HTML tags, for example when outputting the "spiel" data values from the contributors list or the "pre" and "post" bits for the template credits.

review: Needs Fixing

[EdB (edb)]

Okay then it is a problem with the app as it is now. I might change it, I might not. The thing is this branch had nothing to do with "can we do html in a long/short description".

[Tilman Blumenbach (tblue)]

Okay, let's just make sure we don't forget to fix it.

review: Approve

=== modified file 'qp_templates/_credits.disp.php'
--- qp_templates/_credits.disp.php	2011-07-03 14:54:20 +0000
+++ qp_templates/_credits.disp.php	2011-09-04 22:28:26 +0000
@@ -71,7 +71,7 @@
 	$credits[ strtolower( $a_plugin->name ) ] = array(
 		'p_name' => $p_name,
 		'a_name' => $a_name,
-		'desc' => $a_plugin->short_desc,
+		'desc' => $a_plugin->long_desc,
 	);
 }
 

=== modified file 'qp_templates/basic/_credits.disp.php'
--- qp_templates/basic/_credits.disp.php	2011-07-12 18:10:36 +0000
+++ qp_templates/basic/_credits.disp.php	2011-09-04 22:28:26 +0000
@@ -75,7 +75,7 @@
 	$credits[ strtolower( $a_plugin->name ) ] = array(
 		'p_name' => $p_name,
 		'a_name' => $a_name,
-		'desc' => $a_plugin->short_desc,
+		'desc' => $a_plugin->long_desc,
 	);
 }