Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rose bush: view: default to escaping output HTML #1809

Merged
merged 4 commits into from Feb 10, 2016

Conversation

oliver-sanders
Copy link
Member

Also added feature to make url(s) clickable when in text mode.
closes #1684
@matthewrmshin Please review
@benfitzpatrick Please review

@oliver-sanders oliver-sanders added this to the soon milestone Jan 22, 2016
@matthewrmshin matthewrmshin modified the milestones: next-release, soon Jan 22, 2016
@matthewrmshin matthewrmshin self-assigned this Jan 22, 2016
@matthewrmshin matthewrmshin changed the title Merged benfitzpatrick/1693 rose bush: view: default to escaping output HTML Jan 22, 2016
# convert url(s) to hyperlinks if in text mode
if mode == None:
anchor = lambda url: '<a href="%s">%s</a>' % (url, url)
urlregex = '(http|https|ftp)://[^\s]+'
Copy link
Member Author

Choose a reason for hiding this comment

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

URL detection quite crude, expects urls to be space or new line terminated.

Copy link
Member

Choose a reason for hiding this comment

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

May be worth doing:

\b(?:https?|ftp)://\S+\b

Copy link
Member

Choose a reason for hiding this comment

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

You may want to use re.compile to pre-compile the regular expression to speed things up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, that regex is far better.

lines[i] = '%s'*3 % (
line[:begining],
'<a href="{0}">{0}</a>'.format(match.group(0)),
line[end:])
Copy link
Member

Choose a reason for hiding this comment

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

I like the intention of this change, but I think this is leaking template logic into this module. Jinja2 appears to have a urlize filter that should allow us to do this sort of stuff within the template.

@matthewrmshin
Copy link
Member

@benfitzpatrick please sanity check.

@benfitzpatrick
Copy link
Contributor

Great

benfitzpatrick added a commit that referenced this pull request Feb 10, 2016
rose bush: view: default to escaping output HTML
@benfitzpatrick benfitzpatrick merged commit c82e89b into metomi:master Feb 10, 2016
@oliver-sanders oliver-sanders deleted the 1693.default-escape-html branch March 15, 2016 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rose bush: handle accidental HTML tags in output
3 participants