search

twaddington / django-gravatar

Essential Gravatar support for Django. Features helper methods, templatetags and a full test suite!
MIT License
149 stars 33 forks source link

template tag gravatar_url and gravatar shouldn't escape & in the url #16

Closed yultide closed 10 years ago

yultide commented 10 years ago

{% gravatar_url request.user.email 250 %} return this

'https://secure.gravatar.com/avatar/936c6f79aabd47f7b205fbe6c498b836.jpg?s=250&r=g&d=https%3A%2F%2Fjumpch.at%2Fstatic%2Fimg%2Fdefault-avatar.png'

The & should have not be escaped. It should look like the following.

'https://secure.gravatar.com/avatar/936c6f79aabd47f7b205fbe6c498b836.jpg?s=250&r=g&d=https%3A%2F%2Fjumpch.at%2Fstatic%2Fimg%2Fdefault-avatar.png'

twaddington commented 10 years ago

@yultide that might be a regression introduced in 1.1.4. Can you try 1.1.3 and see if the problem is present? I'll look into this.

yultide commented 10 years ago

1.1.3 is still broken.

Here's the issue. django_gravatar/templatetags/gravatar.py

def gravatar_url(user_or_email, size=GRAVATAR_DEFAULT_SIZE): """ Builds a gravatar url from an user or email """ if hasattr(user_or_email, 'email'): email = user_or_email.email else: email = user_or_email

  try:
      return escape(get_gravatar_url(email=email, size=size)) <-- remove escape
  except:
      return ''

def gravatar(user_or_email, size=GRAVATAR_DEFAULT_SIZE, alt_text='', css_class='gravatar'): """ Builds an gravatar tag from an user or email """ if hasattr(user_or_email, 'email'): email = user_or_email.email else: email = user_or_email

  try:
      url = escape(get_gravatar_url(email=email, size=size)) <-- remove escape
  except:
      return ''

  return '<img class="{css_class}" src="{src}" width="{width}" height="{height}" alt="{alt}" />'.format(\
    css_class=css_class, src=url, width=size, height=size, alt=alt_text)
twaddington commented 10 years ago

@yultide ah, I see what you're saying. Are you not using the gravatar_url in an HTML template? I can't think of a reason why you wouldn't escape ampersands in a template. If you don't escape them your page won't pass HTML validation.

twaddington commented 10 years ago

I'm going to close this issue. Feel free to reopen if you have a legitimate need for an unescaped version of that tag output.

Thanks for the feedback.

yultide commented 10 years ago

yes. i'm using gravatar_url in a template. i use it like this. {% gravatar_url request.user.email 250 %}. the code is escaping the url twice. get_gravatar_url escapes the arguments once. the second time, the url is escaped turning & to &amp; That's not a valid url.

you understand the difference between the two urls i posted originally right?

twaddington commented 10 years ago

I understand the difference. The output of the template tags is not being escaped twice as far as I can see. You understand that an ampersand should be escaped when outputted as HTML right?

It's possible Django is autoescaping the output a second time. You might play around with the autoescape tag and see if disabling autoescaping fixes the problem you're seeing.

As far as I can tell this tag is working as intended.

yultide commented 10 years ago

It's not autoescaping from django. I think template tags are returned as is. Try this and the & is still escaped to &amp;

{% autoescape off %} {% gravatar_url request.user.email 250 %} {% endautoescape %}

If I change the code to not escape from the return value of the template tag, it works out fine. I'm ok with closing the bug since I ended up using a fork.

twaddington commented 10 years ago

@yultide I think you'll run into validation issues if you remove the escape tag.

twaddington commented 10 years ago

@yultide here's an example. If you run this through the HTML validator you'll get an error:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">

    <title>Test</title>
  </head>
  <body>
    <img src="http://example.com?one=1&two=2" alt="test" />
  </body>
</html>

If you run the following through the validator it'll pass:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">

    <title>Test</title>
  </head>
  <body>
    <img src="http://example.com?one=1&amp;two=2" alt="test" />
  </body>
</html>

See the difference?

http://validator.w3.org/#validate_by_input

yultide commented 10 years ago

Don't worry about it. I ended up using another gravatar package.

twaddington commented 10 years ago

@yultide fair enough. I'm not sure why you think this is a problem. For what it's worth I verified everything is working locally.

screen shot 2014-05-21 at 7 36 37 pm