Skip to content

Commit

Permalink
widgets: Add support for clickable links.
Browse files Browse the repository at this point in the history
The change was made to support links in polls, as mentioned in
issue zulip#12947. We used markdown renderer to render
the link content, and parsed out any unnecessary p tags.
We changed javascript and hbs files so that they properly
render the content. Tested locally whether the links work,
in addition to checking for XSS vulnerbilities.
Everything tested worked, and no vulnerabilities
discovered. Double check that there are no XSS
issues.

Fixes: zulip#12947
  • Loading branch information
vagrant authored and zhark01 committed Dec 8, 2020
1 parent 6f962c1 commit d704472
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
10 changes: 4 additions & 6 deletions static/js/poll_widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class PollData {
for (const [key, obj] of this.key_to_option) {
const voters = Array.from(obj.votes.keys());
const current_user_vote = voters.includes(this.me);

options.push({
option: obj.option,
names: people.safe_full_names(voters),
Expand Down Expand Up @@ -87,7 +86,6 @@ class PollData {
};

this.my_idx += 1;

return event;
},

Expand Down Expand Up @@ -208,7 +206,7 @@ exports.activate = function (opts) {
const author_help = is_my_poll && !has_question;

elem.find(".poll-question-header").toggle(!input_mode);
elem.find(".poll-question-header").text(question);
elem.find(".poll-question-header").html(question);
elem.find(".poll-edit-question").toggle(can_edit);
update_edit_controls();

Expand Down Expand Up @@ -245,9 +243,9 @@ exports.activate = function (opts) {
new_question = old_question;
}

// Optimistically set the question locally.
poll_data.set_question(new_question);
render_question();
// Decided not to do this to prevent XSS attacks, so that escaping could happen
// poll_data.set_question(new_question);
// render_question();

// If there were no actual edits, we can exit now.
if (new_question === old_question) {
Expand Down
2 changes: 1 addition & 1 deletion static/templates/widgets/poll_widget_results.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<button class="poll-vote {{#if current_user_vote}}current-user-vote{{/if}}" data-key="{{ key }}">
{{ count }}
</button>
<span class="poll-option">{{ option }}</span>
<span class="poll-option"> {{{ option }}} </span>
{{#if names}}
<span class="poll-names">({{ names }})</span>
{{/if}}
Expand Down
12 changes: 10 additions & 2 deletions zerver/lib/actions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
import itertools
import json
import logging
import os
import time
Expand Down Expand Up @@ -173,7 +174,7 @@
)
from zerver.lib.utils import generate_api_key, log_statsd_event
from zerver.lib.validator import check_widget_content
from zerver.lib.widget import do_widget_post_save_actions
from zerver.lib.widget import do_widget_post_save_actions, filter_and_render_string
from zerver.models import (
MAX_MESSAGE_LENGTH,
Attachment,
Expand Down Expand Up @@ -1803,14 +1804,21 @@ def do_add_submessage(realm: Realm,
msg_type: str,
content: str,
) -> None:
content_json = json.loads(content)

if "option" in content_json:
content_json["option"] = filter_and_render_string(content_json["option"])
elif "question" in content_json:
content_json["question"] = filter_and_render_string(content_json["question"])
content = json.dumps(content_json)

submessage = SubMessage(
sender_id=sender_id,
message_id=message_id,
msg_type=msg_type,
content=content,
)
submessage.save()

event = dict(
type="submessage",
msg_type=msg_type,
Expand Down
15 changes: 12 additions & 3 deletions zerver/lib/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@
import re
from typing import Any, MutableMapping, Optional, Tuple

from zerver.models import SubMessage
from zerver.lib.markdown import markdown_convert
from zerver.models import SubMessage, get_realm


def filter_and_render_string(input: str) -> str:
# Run through the markdown engine so that links will work
output = markdown_convert(input, message_realm=get_realm('zulip'),)
# Remove p tags from render output, so the options do not create new lines
output = re.sub(r'<\/*p>', '', output)
return output

def get_widget_data(content: str) -> Tuple[Optional[str], Optional[str]]:
valid_widget_types = ['poll', 'todo']
tokens = content.split(' ')
Expand All @@ -28,17 +36,18 @@ def get_extra_data_from_widget_type(content: str,
question = ''
options = []
if lines and lines[0]:
question = lines.pop(0).strip()
question = filter_and_render_string(lines.pop(0).strip())
for line in lines:
# If someone is using the list syntax, we remove it
# before adding an option.
option = re.sub(r'(\s*[-*]?\s*)', '', line.strip(), 1)
if len(option) > 0:
options.append(option)
options.append(filter_and_render_string(option))
extra_data = {
'question': question,
'options': options,
}

return extra_data
return None

Expand Down

0 comments on commit d704472

Please sign in to comment.