Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

use TwitterAccount's get_url instead of custom html_url #2001

Merged
merged 1 commit into from
Feb 8, 2014

Conversation

seanlinsley
Copy link
Contributor

This resolves #1997 by using what we have instead of relying on a derivative value from the database (that happened to be blown away by #1989)

This resolves #1997 by using what we have instead of relying on a
derivative value from the database (that happened to be blown away by
#1989)
zbynekwinkler added a commit that referenced this pull request Feb 8, 2014
use TwitterAccount's get_url instead of custom html_url
@zbynekwinkler zbynekwinkler merged commit f584892 into master Feb 8, 2014
@zbynekwinkler zbynekwinkler deleted the 1997-fix-twitter-links branch February 8, 2014 22:30
@@ -12,7 +12,7 @@
No Twitter account connected.
{% endif %}
{% else %}
<a rel="me" href="{{ twitter_account.user_info.get('html_url', '')|e }}"
<a rel="me" href="{{ twitter_account.get_url() }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We lost the escaping here. Was that intentional? Do we know that get_url() returns a safe value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the escaping because get_url is this:

def get_url(self):
    return "https://twitter.com/" + self.user_info['screen_name']

and I'm pretty sure we can expect Twitter to provide an HTML-safe username, since they only allow alphanumeric characters (+ underscore).

My other motivation was that there are plenty of places where we don't currently escape this data. If this approach is vulnerable, then we were already vulnerable.

@chadwhitacre
Copy link
Contributor

Good catch, @seanlinsley. Sorry about that! :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connected twitter account incorrectly links to gittip profile
3 participants