[Bug 18620] Improve TablePager (patch)

2009-05-03 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620


Happy-melon happy-me...@live.com changed:

   What|Removed |Added

 Blocks||16497




-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620


Roan Kattouw roan.katt...@gmail.com changed:

   What|Removed |Added

 CC||roan.katt...@gmail.com




--- Comment #1 from Roan Kattouw roan.katt...@gmail.com  2009-04-29 10:16:49 
UTC ---
(In reply to comment #0)
 Created an attachment (id=6068)
 -- (https://bugzilla.wikimedia.org/attachment.cgi?id=6068) [details]
 Patch to Pager.php, against r49962
 
 I'm currently plugging away at formatting [[Special:Allmessages]] using the
 TablePager class, from Pager.php.  In order to avoid some horrible hacks
 (returning a value of foo rowspan=2 to fill into something like td
 class=$class) 
I'm surprised that that even works: it's a sanitization bug.


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620





--- Comment #2 from Happy-melon happy-me...@live.com  2009-04-29 12:47:35 UTC 
---
You mean the hack? TBH I didn't actually try it, I knew what I needed and what
was available in TablePager, and gave up in disgust. Whoever wrote that comment
in CodeReview was right, the function wasn't particularly accessible. The
positioning of the $this-mCurrentRow=$row; line is just stupid.

Or do you mean the patch? :-S


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620





--- Comment #3 from Roan Kattouw roan.katt...@gmail.com  2009-04-29 12:58:33 
UTC ---
(In reply to comment #2)
 You mean the hack? TBH I didn't actually try it, I knew what I needed and what
 was available in TablePager, and gave up in disgust. Whoever wrote that 
 comment
 in CodeReview was right, the function wasn't particularly accessible. The
 positioning of the $this-mCurrentRow=$row; line is just stupid.
 
 Or do you mean the patch? :-S
 

No, I meant the HTML injection hack. I hadn't even looked at the patch yet. Now
that I have:

$td = Xml::openElement( 'td', $this-getCellAttrs($field,$value) );
$s .= $td $formatted /td;

You should simply use:

$s .= Xml::element( 'td', $this-getCellAttrs( $field, $value ), $formatted );

(or Xml::tags() if $formatted contains HTML and should not be escaped; I
couldn't tell from the patch).

return array( 'class' = htmlspecialchars( $this-getRowClass( $row ) ) );

You don't need to (and shouldn't) use htmlspecialchars() here: the Xml::
functions will handle proper escaping. This way, you'll probably end up
double-escaping stuff. The same applies to getCellAttrs().


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620





--- Comment #4 from Happy-melon happy-me...@live.com  2009-04-29 14:45:16 UTC 
---
I tried the first method and got PHP's version of a compile error (unexpected
character on the last line); I'm not sure either why it didn't work ($formatted
could in principle contain raw HTML, but would that really *kill* the
program?); but doing it that way definitely did fix it. 


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620


Happy-melon happy-me...@live.com changed:

   What|Removed |Added

Attachment #6068 is|0   |1
   obsolete||




--- Comment #5 from Happy-melon happy-me...@live.com  2009-04-29 14:46:07 UTC 
---
Created an attachment (id=6069)
 -- (https://bugzilla.wikimedia.org/attachment.cgi?id=6069)
Updated with Roan's htmlspecialchars() fixes


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620





--- Comment #6 from Roan Kattouw roan.katt...@gmail.com  2009-04-29 15:00:49 
UTC ---
(In reply to comment #4)
 I tried the first method and got PHP's version of a compile error (unexpected
 character on the last line); I'm not sure either why it didn't work
My fragment looks correct, I'm not sure your error is related. Please try
again, as your current patch still doesn't use Xml::element() or Xml::tags() on
that line.

 ($formatted
 could in principle contain raw HTML, but would that really *kill* the
 program?); but doing it that way definitely did fix it. 
 
No, but it would show the HTML as text in the browser rather than actually
treating it as HTML, so you'd have a bug in case $formatted is *supposed* to
contain HTML.


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620


Happy-melon happy-me...@live.com changed:

   What|Removed |Added

Attachment #6069 is|0   |1
   obsolete||




--- Comment #7 from Happy-melon happy-me...@live.com  2009-04-29 15:37:54 UTC 
---
Created an attachment (id=6070)
 -- (https://bugzilla.wikimedia.org/attachment.cgi?id=6070)
updated, use Xml::tags(); still against r49962

I guess you're right; tried and it worked fine.  It definitely needs to be
tags(); using elements escapes things like links. htmlspecialchars is the thing
to use to disable tags, right? (Not relevant for this, but will be in my
rewrite of SpecialAllmessages).


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l


[Bug 18620] Improve TablePager (patch)

2009-04-29 Thread bugzilla-daemon
https://bugzilla.wikimedia.org/show_bug.cgi?id=18620


Roan Kattouw roan.katt...@gmail.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
   Keywords|need-review |reviewed
 Resolution||FIXED




--- Comment #8 from Roan Kattouw roan.katt...@gmail.com  2009-04-29 16:00:12 
UTC ---
(In reply to comment #7)
 Created an attachment (id=6070)
 -- (https://bugzilla.wikimedia.org/attachment.cgi?id=6070) [details]
 updated, use Xml::tags(); still against r49962
 
 I guess you're right; tried and it worked fine.  It definitely needs to be
 tags(); using elements escapes things like links.
Patch looks OK, committed in r50046

 htmlspecialchars is the thing
 to use to disable tags, right? (Not relevant for this, but will be in my
 rewrite of SpecialAllmessages).
 
Yes, but you probably want to use Xml:: functions there as well, as they
sanitize stuff for you.


-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
You are on the CC list for the bug.

___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l