Skip to content

(WIP) Fix TNL-3429. #11588

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.

Sign up for GitHub

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

Closed
agaylard wants to merge 5 commits into from
Closed

(WIP) Fix TNL-3429. #11588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f135d63
Fix TNL-3429.
Feb 18, 2016
7c81088
Fix pylint wrinkles.
Feb 19, 2016
a0c3bb5
Add missing |h filter.
Feb 19, 2016
70ff67c
Remove selective escape_html logic.
Feb 19, 2016
5f39190
Remove empty line.
Feb 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/views/item.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F

xblock_info = {
"id": unicode(xblock.location),
"display_name": xblock.display_name_with_default_escaped,
"display_name": xblock.display_name_with_default,
"category": xblock.category,
"edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None,
"published": published,
Expand Down Expand Up @@ -1158,4 +1158,4 @@ def _xblock_type_and_display_name(xblock):
"""
return _('{section_or_subsection} "{display_name}"').format(
section_or_subsection=xblock_type_display_name(xblock),
display_name=xblock.display_name_with_default_escaped)
display_name=xblock.display_name_with_default)
8 changes: 4 additions & 4 deletions cms/templates/container.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
)
from openedx.core.djangolib.markup import HTML, ugettext as _
%>
<%block name="title">${xblock.display_name_with_default_escaped} ${xblock_type_display_name(xblock) | h}</%block>
<%block name="title">${xblock.display_name_with_default | h} ${xblock_type_display_name(xblock) | h}</%block>

@robrap robrap Feb 19, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@agaylard - As @mushtaqak pointed out, we are moving toward making all templates "safe by default" by adding a page directive like you can see in index.html in this PR. Here's a comment about some things to look for if you make these changes.

As I noted earlier, more instructions will be coming and this will be a requirement in the future, but it isn't now. It's up to you if you want us to help chip away at this. However, it is important for you (and everyone) to understand that there is nothing special about this particular field, and we will want all fields to be escaped properly.

Thanks.

@mushtaqak mushtaqak Feb 23, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@robrap Yeah sure. Whatever you need me to for this, I will gladly help out :)

@robrap robrap Feb 23, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @mushtaqak. I actually took care of this in a different PR.

FYI: the instructions for this will soon be available in Read the Docs, and they will have changed slightly for internationalization with _() and HTML().

<%block name="bodyclass">is-signedin course container view-container</%block>

<%namespace name='static' file='static_content.html'/>
Expand Down Expand Up @@ -57,15 +57,15 @@
ancestor_url = xblock_studio_url(ancestor)
%>
% if ancestor_url:
<a href="${ancestor_url | h}" class="navigation-item navigation-link navigation-parent">${ancestor.display_name_with_default_escaped | h}</a>
<a href="${ancestor_url | h}" class="navigation-item navigation-link navigation-parent">${ancestor.display_name_with_default | h}</a>
% else:
<span class="navigation-item navigation-parent">${ancestor.display_name_with_default_escaped | h}</span>
<span class="navigation-item navigation-parent">${ancestor.display_name_with_default | h}</span>
% endif
% endfor
</small>
<div class="wrapper-xblock-field incontext-editor is-editable"
data-field="display_name" data-field-display-name="${_("Display Name")}">
<h1 class="page-header-title xblock-field-value incontext-editor-value"><span class="title-value">${xblock.display_name_with_default_escaped | h}</span></h1>
<h1 class="page-header-title xblock-field-value incontext-editor-value"><span class="title-value">${xblock.display_name_with_default | h}</span></h1>
</div>
</div>

Expand Down
4 changes: 2 additions & 2 deletions cms/templates/js/course-outline.underscore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ if (is_proctored_exam) {
<% } %>
<% if (xblockInfo.isVertical()) { %>
<span class="unit-title item-title">
<a href="<%= xblockInfo.get('studio_url') %>"><%= xblockInfo.get('display_name') %></a>
<a href="<%= xblockInfo.get('studio_url') %>"><%- xblockInfo.get('display_name') %></a>

@robrap robrap Feb 19, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@agaylard For Underscore templates, there is no equivalent syntax for making the page safe by default. Instead, it should be understood that using <%- should be the rule, and using <%= should be the rare exception, so we'd be switching almost all of these, rather than a few particular ones.

Again, this isn't a requirement for this PR, but it certainly would be helpful if you want to wrap up that work. Of course, as you know, the Mako and underscore templates are related so if you escape here you need to make sure the data is not escaped when sent from Mako to JavaScript.

</span>
<% } else { %>
<span class="wrapper-<%= xblockType %>-title wrapper-xblock-field incontext-editor is-editable" data-field="display_name" data-field-display-name="<%= gettext("Display Name") %>">
<span class="<%= xblockType %>-title item-title xblock-field-value incontext-editor-value"><%= xblockInfo.get('display_name') %></span>
<span class="<%= xblockType %>-title item-title xblock-field-value incontext-editor-value"><%- xblockInfo.get('display_name') %></span>
</span>
<% } %>
</h3>
Expand Down
2 changes: 1 addition & 1 deletion cms/templates/js/unit-outline.underscore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<div class="<%= xblockType %>-header">
<h3 class="<%= xblockType %>-header-details">
<span class="<%= xblockType %>-title item-title">
<a href="<%= xblockInfo.get('studio_url') %>"><%= xblockInfo.get('display_name') %></a>
<a href="<%= xblockInfo.get('studio_url') %>"><%- xblockInfo.get('display_name') %></a>
</span>
</h3>
</div>
Expand Down