From 80195176e99893e9f045476f3f71f22ac179af2b Mon Sep 17 00:00:00 2001 From: Rocky Duan Date: Fri, 24 Aug 2012 01:45:55 -0700 Subject: [PATCH] use escape html everywhere and only send discussion data relavent --- .../django_comment_client/base/views.py | 26 +++++++++---------- .../django_comment_client/forum/views.py | 12 ++++----- lms/djangoapps/django_comment_client/utils.py | 14 ++++++++++ .../coffee/src/discussion/discussion.coffee | 2 +- .../discussion/_content_renderer.html | 2 +- .../discussion/_discussion_module.html | 2 +- lms/templates/discussion/_forum.html | 2 +- lms/templates/discussion/_inline.html | 2 +- lms/templates/discussion/_paginator.html | 4 +-- .../discussion/_recent_active_posts.html | 2 +- lms/templates/discussion/_search_bar.html | 4 +-- lms/templates/discussion/_similar_posts.html | 2 +- lms/templates/discussion/_single_thread.html | 2 +- lms/templates/discussion/_sort.html | 2 +- lms/templates/discussion/_trending_tags.html | 2 +- .../discussion/_user_active_threads.html | 2 +- lms/templates/discussion/_user_profile.html | 6 ++--- lms/templates/discussion/index.html | 2 +- lms/templates/discussion/user_profile.html | 2 +- 19 files changed, 53 insertions(+), 39 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index d1e428ef9c..68250a035e 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -55,7 +55,7 @@ def ajax_content_response(request, course_id, content, template_name): annotated_content_info = utils.get_annotated_content_info(course_id, content, request.user, user_info) return JsonResponse({ 'html': html, - 'content': content, + 'content': utils.safe_content(content), 'annotated_content_info': annotated_content_info, }) @@ -78,7 +78,7 @@ def create_thread(request, course_id, commentable_id): if request.is_ajax(): return ajax_content_response(request, course_id, thread.to_dict(), 'discussion/ajax_create_thread.html') else: - return JsonResponse(thread.to_dict()) + return JsonResponse(utils.safe_content(thread.to_dict())) @require_POST @login_required @@ -90,7 +90,7 @@ def update_thread(request, course_id, thread_id): if request.is_ajax(): return ajax_content_response(request, course_id, thread.to_dict(), 'discussion/ajax_update_thread.html') else: - return JsonResponse(thread.to_dict()) + return JsonResponse(utils.safe_content(thread.to_dict())) def _create_comment(request, course_id, thread_id=None, parent_id=None): post = request.POST @@ -109,7 +109,7 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None): if request.is_ajax(): return ajax_content_response(request, course_id, comment.to_dict(), 'discussion/ajax_create_comment.html') else: - return JsonResponse(comment.to_dict()) + return JsonResponse(utils.safe_content(comment.to_dict())) @require_POST @login_required @@ -126,7 +126,7 @@ def create_comment(request, course_id, thread_id): def delete_thread(request, course_id, thread_id): thread = cc.Thread.find(thread_id) thread.delete() - return JsonResponse(thread.to_dict()) + return JsonResponse(utils.safe_content(thread.to_dict())) @require_POST @login_required @@ -138,7 +138,7 @@ def update_comment(request, course_id, comment_id): if request.is_ajax(): return ajax_content_response(request, course_id, comment.to_dict(), 'discussion/ajax_update_comment.html') else: - return JsonResponse(comment.to_dict()), + return JsonResponse(utils.safe_content(comment.to_dict())) @require_POST @login_required @@ -147,7 +147,7 @@ def endorse_comment(request, course_id, comment_id): comment = cc.Comment.find(comment_id) comment.endorsed = request.POST.get('endorsed', 'false').lower() == 'true' comment.save() - return JsonResponse(comment.to_dict()) + return JsonResponse(utils.safe_content(comment.to_dict())) @require_POST @login_required @@ -158,7 +158,7 @@ def openclose_thread(request, course_id, thread_id): thread.save() thread = thread.to_dict() return JsonResponse({ - 'content': thread, + 'content': utils.safe_content(thread), 'ability': utils.get_ability(course_id, thread, request.user), }) @@ -177,7 +177,7 @@ def create_sub_comment(request, course_id, comment_id): def delete_comment(request, course_id, comment_id): comment = cc.Comment.find(comment_id) comment.delete() - return JsonResponse(comment.to_dict()) + return JsonResponse(utils.safe_content(comment.to_dict())) @require_POST @login_required @@ -186,7 +186,7 @@ def vote_for_comment(request, course_id, comment_id, value): user = cc.User.from_django_user(request.user) comment = cc.Comment.find(comment_id) user.vote(comment, value) - return JsonResponse(comment.to_dict()) + return JsonResponse(utils.safe_content(comment.to_dict())) @require_POST @login_required @@ -195,7 +195,7 @@ def undo_vote_for_comment(request, course_id, comment_id): user = cc.User.from_django_user(request.user) comment = cc.Comment.find(comment_id) user.unvote(comment) - return JsonResponse(comment.to_dict()) + return JsonResponse(utils.safe_content(comment.to_dict())) @require_POST @login_required @@ -204,7 +204,7 @@ def vote_for_thread(request, course_id, thread_id, value): user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) user.vote(thread, value) - return JsonResponse(thread.to_dict()) + return JsonResponse(utils.safe_content(thread.to_dict())) @require_POST @login_required @@ -213,7 +213,7 @@ def undo_vote_for_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) user.unvote(thread) - return JsonResponse(thread.to_dict()) + return JsonResponse(utils.safe_content(thread.to_dict())) @require_POST diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index eda574cb6e..62055f0ab7 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -83,7 +83,7 @@ def render_discussion(request, course_id, threads, *args, **kwargs): 'base_url': base_url, 'query_params': strip_blank(strip_none(extract(query_params, ['page', 'sort_key', 'sort_order', 'tags', 'text']))), 'annotated_content_info': json.dumps(annotated_content_info), - 'discussion_data': json.dumps({ (discussion_id or user_id): threads }) + 'discussion_data': json.dumps({ (discussion_id or user_id): map(utils.safe_content, threads) }) } context = dict(context.items() + query_params.items()) return render_to_string(template, context) @@ -128,7 +128,7 @@ def inline_discussion(request, course_id, discussion_id): return utils.JsonResponse({ 'html': html, - 'discussionData': threads, + 'discussion_data': map(utils.safe_content, threads), }) def render_search_bar(request, course_id, discussion_id=None, text=''): @@ -149,7 +149,7 @@ def forum_form_discussion(request, course_id): if request.is_ajax(): return utils.JsonResponse({ 'html': content, - 'discussionData': threads, + 'discussion_data': map(utils.safe_content, threads), }) else: recent_active_threads = cc.search_recent_active_threads( @@ -186,7 +186,7 @@ def render_single_thread(request, discussion_id, course_id, thread_id): 'annotated_content_info': json.dumps(annotated_content_info), 'course_id': course_id, 'request': request, - 'discussion_data': json.dumps({ discussion_id: [thread] }), + 'discussion_data': json.dumps({ discussion_id: [utils.safe_content(thread)] }), } return render_to_string('discussion/_single_thread.html', context) @@ -202,7 +202,7 @@ def single_thread(request, course_id, discussion_id, thread_id): return utils.JsonResponse({ 'html': html, - 'content': thread.to_dict(), + 'content': utils.safe_content(thread.to_dict()), 'annotated_content_info': annotated_content_info, }) @@ -252,7 +252,7 @@ def user_profile(request, course_id, user_id): if request.is_ajax(): return utils.JsonResponse({ 'html': content, - 'discussionData': threads, + 'discussion_data': map(utils.safe_content, threads), }) else: context = { diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 4908ee91b5..45e0fc196c 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -219,3 +219,17 @@ def extend_content(content): 'permalink': permalink(content), } return merge_dict(content, content_info) + +def safe_content(content): + fields = [ + 'id', 'body', 'course_id', 'anonymous', 'endorsed', + 'parent_id', 'thread_id', 'votes', 'closed', + 'created_at', 'updated_at', 'depth', 'type', + 'commentable_id', 'comments_count', 'at_position_list', + 'children', 'highlighted_title', 'highlighted_body', + ] + + if content.get('anonymous') is False: + fields += ['username', 'user_id'] + + return strip_none(extract(content, fields)) diff --git a/lms/static/coffee/src/discussion/discussion.coffee b/lms/static/coffee/src/discussion/discussion.coffee index 8f9639d344..d30fdc70a8 100644 --- a/lms/static/coffee/src/discussion/discussion.coffee +++ b/lms/static/coffee/src/discussion/discussion.coffee @@ -48,7 +48,7 @@ if Backbone? $parent = @$el.parent() @$el.replaceWith(response.html) $discussion = $parent.find("section.discussion") - @model.reset(response.discussionData, { silent: false }) + @model.reset(response.discussion_data, { silent: false }) view = new DiscussionView el: $discussion[0], model: @model DiscussionUtil.bulkUpdateContentInfo(window.$$annotated_content_info) $("html, body").animate({ scrollTop: 0 }, 0) diff --git a/lms/templates/discussion/_content_renderer.html b/lms/templates/discussion/_content_renderer.html index 54371bf4dd..ac2b0b4897 100644 --- a/lms/templates/discussion/_content_renderer.html +++ b/lms/templates/discussion/_content_renderer.html @@ -5,7 +5,7 @@ <%def name="render_content_with_comments(content, *args, **kwargs)"> -
+
${render_content(content, *args, **kwargs)} ${render_comments(content.get('children', []), *args, **kwargs)}
diff --git a/lms/templates/discussion/_discussion_module.html b/lms/templates/discussion/_discussion_module.html index 3d92f242a1..b9e69cc0ad 100644 --- a/lms/templates/discussion/_discussion_module.html +++ b/lms/templates/discussion/_discussion_module.html @@ -1,3 +1,3 @@
- Show Discussion + Show Discussion
diff --git a/lms/templates/discussion/_forum.html b/lms/templates/discussion/_forum.html index 0a812fd1bb..b43efae666 100644 --- a/lms/templates/discussion/_forum.html +++ b/lms/templates/discussion/_forum.html @@ -1,6 +1,6 @@ <%namespace name="renderer" file="_content_renderer.html"/> -
+
diff --git a/lms/templates/discussion/_inline.html b/lms/templates/discussion/_inline.html index aa90e4e6ba..abef7f39e8 100644 --- a/lms/templates/discussion/_inline.html +++ b/lms/templates/discussion/_inline.html @@ -1,6 +1,6 @@ <%namespace name="renderer" file="_content_renderer.html"/> -
+
diff --git a/lms/templates/discussion/_paginator.html b/lms/templates/discussion/_paginator.html index dd9bd2d43d..bb94b64289 100644 --- a/lms/templates/discussion/_paginator.html +++ b/lms/templates/discussion/_paginator.html @@ -9,7 +9,7 @@ %> <%def name="link_to_page(_page, text)"> - ${text} + ${text} <%def name="div_page(_page)"> @@ -36,7 +36,7 @@ % endfor -
+
% if page > 1: ${link_to_page(page - 1, "< Previous page")} diff --git a/lms/templates/discussion/_recent_active_posts.html b/lms/templates/discussion/_recent_active_posts.html index baf505838d..b787df2fcf 100644 --- a/lms/templates/discussion/_recent_active_posts.html +++ b/lms/templates/discussion/_recent_active_posts.html @@ -8,7 +8,7 @@
    % for thread in recent_active_threads: -
  1. ${thread['title']} ${thread['votes']['point']}
    2. +
  2. ${thread['title'] | h} ${thread['votes']['point'] | h}
    3. % endfor
      diff --git a/lms/templates/discussion/_search_bar.html b/lms/templates/discussion/_search_bar.html index e93b46efb2..1f46a8e3c8 100644 --- a/lms/templates/discussion/_search_bar.html +++ b/lms/templates/discussion/_search_bar.html @@ -10,9 +10,9 @@ def base_url_for_search():
      % if query_params.get('tags', None): - + % else: - + % endif
      Search posts
      diff --git a/lms/templates/discussion/_similar_posts.html b/lms/templates/discussion/_similar_posts.html index f68964acec..ef6eedee2e 100644 --- a/lms/templates/discussion/_similar_posts.html +++ b/lms/templates/discussion/_similar_posts.html @@ -3,7 +3,7 @@ Hide
      % for thread in threads: - ${thread['title']} + ${thread['title'] | h} % endfor
      % endif diff --git a/lms/templates/discussion/_single_thread.html b/lms/templates/discussion/_single_thread.html index 395eb72e46..bfbf7b069e 100644 --- a/lms/templates/discussion/_single_thread.html +++ b/lms/templates/discussion/_single_thread.html @@ -1,6 +1,6 @@ <%namespace name="renderer" file="_content_renderer.html"/> -
      +
      Discussion
      ${renderer.render_content_with_comments(thread)} diff --git a/lms/templates/discussion/_sort.html b/lms/templates/discussion/_sort.html index 934973a966..7ba10eab7b 100644 --- a/lms/templates/discussion/_sort.html +++ b/lms/templates/discussion/_sort.html @@ -26,7 +26,7 @@ else: return base_url + '?' + urlencode(merge(query_params, {'page': 1, 'sort_key': key, 'sort_order': order})) %> - ${title} + ${title}
      diff --git a/lms/templates/discussion/_trending_tags.html b/lms/templates/discussion/_trending_tags.html index fea18c02dc..509516c2d5 100644 --- a/lms/templates/discussion/_trending_tags.html +++ b/lms/templates/discussion/_trending_tags.html @@ -7,7 +7,7 @@
        % for tag, count in trending_tags: -
      1. ${tag}×${count}
        2. +
      2. ${tag | h}×${count | h}
        3. % endfor
          diff --git a/lms/templates/discussion/_user_active_threads.html b/lms/templates/discussion/_user_active_threads.html index ad72ccdd5e..1844009466 100644 --- a/lms/templates/discussion/_user_active_threads.html +++ b/lms/templates/discussion/_user_active_threads.html @@ -1,6 +1,6 @@ <%namespace name="renderer" file="_content_renderer.html"/> -
          +
          diff --git a/lms/templates/discussion/_user_profile.html b/lms/templates/discussion/_user_profile.html index a092a97951..8660d8035f 100644 --- a/lms/templates/discussion/_user_profile.html +++ b/lms/templates/discussion/_user_profile.html @@ -7,12 +7,12 @@ <% role_names = sorted(map(attrgetter('name'), django_user.roles.all())) %> -
          ${django_user.username}
          +
          ${django_user.username | h}
          ${", ".join(role_names)}
          -
          ${profiled_user['threads_count']} ${pluralize('discussion', profiled_user['threads_count'])} started
          -
          ${profiled_user['comments_count']} ${pluralize('comment', profiled_user['comments_count'])}
          +
          ${profiled_user['threads_count'] | h} ${pluralize('discussion', profiled_user['threads_count']) | h} started
          +
          ${profiled_user['comments_count'] | h} ${pluralize('comment', profiled_user['comments_count']) | h}
          % if check_permissions_by_view(user, course.id, content=None, name='update_moderator_status'): % if "Moderator" in role_names: Revoke Moderator provileges diff --git a/lms/templates/discussion/index.html b/lms/templates/discussion/index.html index 43291b6f9b..1160a14d90 100644 --- a/lms/templates/discussion/index.html +++ b/lms/templates/discussion/index.html @@ -1,7 +1,7 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion -<%block name="title">Discussion – ${course.number} +<%block name="title">Discussion – ${course.number | h} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/discussion/user_profile.html b/lms/templates/discussion/user_profile.html index a0a64deeb7..4c067db710 100644 --- a/lms/templates/discussion/user_profile.html +++ b/lms/templates/discussion/user_profile.html @@ -3,7 +3,7 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion -<%block name="title">Discussion – ${course.number} +<%block name="title">Discussion – ${course.number | h} <%block name="headextra"> <%static:css group='course'/>