Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mstef’s picture

Status: Active » Needs review
FileSize
479 bytes

What do you think about using strip_tags() instead of check_plain() in theme_privatemsg_username()?

Status: Needs review » Needs work

The last submitted patch, privatemsg_prevent-encoded-username-autocomplete-1694558.patch, failed testing.

mstef’s picture

Status: Needs work » Needs review
FileSize
1.08 KB

Update the test to match

Berdir’s picture

Well, strip_tags() would on the other side break a username that has in it, which is not possible by default but still.

Not sure, how does core handle this? There is a user autocomplete on the node author textfield, I think.

mstef’s picture

It uses:

function user_autocomplete($string = '') {
  $matches = array();
  if ($string) {
    $result = db_select('users')->fields('users', array('name'))->condition('name', db_like($string) . '%', 'LIKE')->range(0, 10)->execute();
    foreach ($result as $user) {
      $matches[$user->name] = check_plain($user->name);
    }
  }

  drupal_json_output($matches);
}

Also has check_plain(), but I just tested and the autocomplete doesn't return it to the textfield encoded, and there's no problems submitted.

Hmm..?

Berdir’s picture

Ah yes, makes sense. Only the label that is displayed is run through check_plain(), the key is then what is actually inserted into the textfield, and that doesn't need to be escaped. That's the way then :)

mstef’s picture

Yea but it's not displayed on the textfield as escaped..that's what is confusing me.

And what's confusing me is that privatemsg_autocomplete() is also only escaping the label. But not working..

GBurg’s picture

I have the same issue. Only want to note that probably it is a problem in the js?

stefgosselin’s picture

The fix seems to do the trick but the bundled private message realname module has the same issue.

Attached patch replicates the strategy in privatemsg_realname.module.

Status: Needs review » Needs work

The last submitted patch, privatemsg_prevent-encoded-username-autocomplete-1694558-9.patch, failed testing.

stefgosselin’s picture

stefgosselin’s picture

Berdir’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
stefgosselin’s picture

I think testbot is choking on the patch because it applies a fix in a submodule (privatemsg_realname). To anyone stumbling on this thread needing the fix for privatemsg_realname, patch 12 fixed up the issue for me.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
1.76 KB

Patch re-rolled and made to apply cleanly.

ptmkenny’s picture

Status: Needs review » Reviewed & tested by the community

(deleted)

ptmkenny’s picture

Status: Reviewed & tested by the community » Needs review

Re-setting status; sorry, mistakenly posted on wrong issue (had multiple tabs open- sorry)

ptmkenny’s picture

Status: Needs review » Reviewed & tested by the community

I tested this patch with both realname usernames and standard usernames with apostrophes, including multiple apostrophes, and it worked. Note that on 7.x-2.x, for this patch to work, another patch (https://drupal.org/node/1956038) must be applied.

Berdir’s picture

Version: 7.x-1.x-dev » 6.x-2.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed. Do we need this for 6..x as well, not sure right now.

oadaeh’s picture

Issue summary: View changes
Status: Patch (to be ported) » Closed (won't fix)

This issue is being closed because it is against a branch for a version of Drupal that is no longer supported.
If you feel that this issue is still valid, feel free to re-open and update it (and any possible patch) to work with the 7.x-1.x branch (bug fixes only) or the 7.x-2.x branch.
Thank you.