-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

Bryan Tong Minh wrote:

...

Revert r39793 "* (bug 13879) Special:EmailUser shows a form in case no user was specified" for the moment

• Recipient name seems to be output raw into HTML form; this is insecure

There was an htmlspecialchars around it:

...

•           $recipient = $this->target instanceof User ?
  

•                   htmlspecialchars( $this->target->getName() ) :
  

•                   '';
  

Ugh. That's a bad practice, with two big things wrong with it:

1) The escaping is separated from the output (which happens many lines down), making it harder to find and more prone to future error as code is updated over the years

and

2) The escaped variable doesn't follow our convention for output-escaped variables, making it harder to confirm it's correct and easier to run into future troubles if the variable gets reused for something else in the middle of the function because future maintainers don't realize it's output-escaped.

If it must be used, it should be named $encRecipient to follow the convention and aid future code maintenance.

Better still would be to construct the form components with the HTML generator functions in the Xml class, which will ensure proper escaping.

...

• We've lost the link to the target's user page in the primary use case (followed 'email this user' link)

You suggest that we only show the input box in case no target or an invalid target is specified and else the current behaviour with a link to the receipent?

That might be nice; otherwise we've got no link back.

- -- brion