The function url() don't escape HTML so you need to add check_url() or drupal_urlencode() to do this. This code will generate links with "&" instead of "&".

    if (variable_get('user_register', 1)) {
      return t('<a href="/%login">login</a> or <a href="/%register">register</a> to post comments', array('%login' =>url('user/login', $destination, 'comment-form'), '%register' => url('user/register', $destination, 'comment-form')));
    }
    else {
      return t('<a href="/%login">login</a> to post comments', array('%login' =>url('user/login', $destination, 'comment-form')));
    }

The handbook page "How to handle text in a secure fashion" http://drupal.org/node/28984 has good info on this. It suggest check_url() for user contributed links and drupal_urlencode() for other links.

My patch applies drupal_urlencode() since it's internal links only.

CommentFileSizeAuthor
#4 comment.module-fj-01.revised.patch.txt776 bytesacp
comment.module-fj-01.patch1004 bytesfrjo

Comments

frjo’s picture

frjo commented
Component: base system » comment.module
acp’s picture

acp commented

Just found this error while validating some pages. Could you please apply the patch ?
I know it's not much, but would be great to have full compliance with w3c standards :).

Steven’s picture

Steven commented
Status: Needs review » Needs work

Patch is bogus, you are misunderstanding the guidelines. Check_url() is necessary.

acp’s picture

acp commented
StatusFileSize
new comment.module-fj-01.revised.patch.txt776 bytes

Indeed it seems so, here's the corrected version of the patch...

Gman’s picture

Gman commented

It seems that the drupal_urlencode has gotten into the core, since drupal.com is not working in this respect, and neither is my site.

If you try to login while responding to a comment or forum post ("login or register to comment"), then the URL looks like this:

http://drupal.org/user/login?destination=comment/reply/77562%2523comment...

Which leads to this after you login, which is a failure, no page found:

http://drupal.org/comment/reply/77562%2523comment_form

With the check_url I get:

http://drupal.org/user/login?destination=comment/reply/77880#comment_form

Which leads to

http://drupal.org/comment/reply/77247

Which drops the # anchor altogether. At least it is not a blank page though. Definitely needs to be fixed since this is breaks the login sequence.

mhutch’s picture

mhutch commented

I can repro this too.

(Also subscribing to issue, as it's been discouraging my users from commenting!).

toemaz’s picture

toemaz commented
Version: x.y.z » 4.7.x-dev

I encouter the same bug with 4.7.4.
When submitting this follow up, I had to choose a version (x.y.z. is not a valid version), so I've selected 4.7.x-dev. Hope that this is ok.

toemaz’s picture

toemaz commented
Status: Needs work » Closed (duplicate)

You can find the solution over here:
http://drupal.org/node/78515#comment-133103