ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix escaping issues with quizzes 

Since plaintext fields are stored raw in the database, a lot of the previous
escaping/unescaping behavior in quizzes has changed.

Change-Id: I299fa7979a30b1efa8944c6df7150b82980c47c5
Reviewed-on: https://gerrit.instructure.com/2403
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Zach Wily 2011-02-23 10:19:32 -07:00
parent 39db2a2199
commit 0858d4e337
6 changed files with 60 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
app/models/assessment_question.rb
View File

@ -147,20 +147,15 @@ class AssessmentQuestion < ActiveRecord::Base
self.save
end
def self.sanitize_and_trim(html, type, max=16.kilobytes, escape=false)
if html && html.length > max
raise "The text for #{type} is too long, max length is #{max}"
end
html = CGI::escapeHTML(html || "") if escape
html = Sanitize.clean(html || "", Instructure::SanitizeField::SANITIZE)
def self.sanitize(html)
Sanitize.clean(html || "", Instructure::SanitizeField::SANITIZE)
end
def self.escape_and_trim(html, type, max=16.kilobytes)
def self.check_length(html, type, max=16.kilobytes)
if html && html.length > max
raise "The text for #{type} is too long, max length is #{max}"
end
html = CGI::unescapeHTML(html || "")
html = CGI::escapeHTML(html)
html
end
def self.parse_question(qdata, assessment_question=nil)
@ -168,13 +163,13 @@ class AssessmentQuestion < ActiveRecord::Base
qdata = qdata.with_indifferent_access
previous_data = assessment_question.question_data rescue {}
question[:points_possible] = (qdata[:points_possible] || previous_data[:points_possible] || 0.0).to_f
question[:correct_comments] = sanitize_and_trim(qdata[:correct_comments] || previous_data[:correct_comments] || "", 'correct comments', 5.kilobyte, true)
question[:incorrect_comments] = sanitize_and_trim(qdata[:incorrect_comments] || previous_data[:incorrect_comments] || "", 'incorrect comments', 5.kilobyte, true)
question[:neutral_comments] = sanitize_and_trim(qdata[:neutral_comments], 'neutral comments', 5.kilobyte, true)
question[:correct_comments] = check_length(qdata[:correct_comments] || previous_data[:correct_comments] || "", 'correct comments', 5.kilobyte)
question[:incorrect_comments] = check_length(qdata[:incorrect_comments] || previous_data[:incorrect_comments] || "", 'incorrect comments', 5.kilobyte)
question[:neutral_comments] = check_length(qdata[:neutral_comments], 'neutral comments', 5.kilobyte)
question[:question_type] = qdata[:question_type] || previous_data[:question_type] || "text_only_question"
question[:question_name] = qdata[:question_name] || qdata[:name] || previous_data[:question_name] || "Question"
question[:question_name] = "Question" if question[:question_name].blank?
question[:question_text] = sanitize_and_trim(qdata[:question_text] || previous_data[:question_text] || "Question text", 'question text')
question[:question_text] = sanitize(check_length(qdata[:question_text] || previous_data[:question_text] || "Question text", 'question text'))
min_size = 1.kilobyte
question[:answers] = []
reset_local_ids
@ -184,7 +179,7 @@ class AssessmentQuestion < ActiveRecord::Base
found_correct = false
answers.each do |key, answer|
found_correct = true if answer[:answer_weight].to_i == 100
a = {:text => escape_and_trim(answer[:answer_text], 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(answer[:answer_text], 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :id => unique_local_id(answer[:id].to_i)}
question[:answers] << a
end
question[:answers][0][:weight] = 100 unless found_correct
@ -196,10 +191,10 @@ class AssessmentQuestion < ActiveRecord::Base
false_id = nil
answers.each do |key, answer|
if key != answers[0][0]
false_comments = escape_and_trim(answer[:answer_comments], 'answer comments', min_size)
false_comments = check_length(answer[:answer_comments], 'answer comments', min_size)
false_id = unique_local_id(answer[:id].to_i)
else
true_comments = escape_and_trim(answer[:answer_comments], 'answer comments', min_size)
true_comments = check_length(answer[:answer_comments], 'answer comments', min_size)
true_id = unique_local_id(answer[:id].to_i)
end
if key != answers[0][0] && answer[:answer_weight].to_i == 100
@ -212,22 +207,22 @@ class AssessmentQuestion < ActiveRecord::Base
question[:answers] << f
elsif question[:question_type] == "short_answer_question"
answers.each do |key, answer|
a = {:text => escape_and_trim(scrub(answer[:answer_text]), 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => 100, :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(scrub(answer[:answer_text]), 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => 100, :id => unique_local_id(answer[:id].to_i)}
question[:answers] << a
end
elsif question[:question_type] == "essay_question"
question[:comments] = escape_and_trim((qdata[:answers][0][:answer_comments] rescue ""), 'essay comments', 5.kilobyte)
question[:comments] = check_length((qdata[:answers][0][:answer_comments] rescue ""), 'essay comments', 5.kilobyte)
elsif question[:question_type] == "matching_question"
answers.each do |key, answer|
a = {:text => escape_and_trim(answer[:answer_match_left], 'answer match', min_size), :left => escape_and_trim(answer[:answer_match_left], 'answer match', min_size), :right => escape_and_trim(answer[:answer_match_right], 'answer match', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size)}
a = {:text => check_length(answer[:answer_match_left], 'answer match', min_size), :left => check_length(answer[:answer_match_left], 'answer match', min_size), :right => check_length(answer[:answer_match_right], 'answer match', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size)}
a[:match_id] = unique_local_id(answer[:match_id].to_i)
a[:id] = unique_local_id(answer[:id].to_i)
question[:answers] << a
question[:matches] ||= []
question[:matches] << {:match_id => a[:match_id], :text => escape_and_trim(answer[:answer_match_right], 'answer match', min_size) }
question[:matches] << {:match_id => a[:match_id], :text => check_length(answer[:answer_match_right], 'answer match', min_size) }
end
(qdata[:matching_answer_incorrect_matches] || "").split("\n").each do |other|
m = {:text => escape_and_trim(other[0..255], 'distractor', min_size) }
m = {:text => check_length(other[0..255], 'distractor', min_size) }
m[:match_id] = previous_data[:answers].detect{|a| a[:text] == m[:text] }[:id] rescue nil
m[:match_id] = unique_local_id(m[:match_id])
question[:matches] << m
@ -236,21 +231,21 @@ class AssessmentQuestion < ActiveRecord::Base
found_correct = false
answers.each do |key, answer|
found_correct = true if answer[:answer_weight].to_i == 100
a = {:text => escape_and_trim(answer[:answer_text], 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(answer[:answer_text], 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :id => unique_local_id(answer[:id].to_i)}
question[:answers] << a
end
question[:answers][0][:weight] = 100 unless found_correct
question[:text_after_answers] = sanitize_and_trim(qdata[:text_after_answers] || previous_data[:text_after_answers] || "", 'text after answers', 16.kilobytes)
question[:text_after_answers] = sanitize(check_length(qdata[:text_after_answers] || previous_data[:text_after_answers] || "", 'text after answers', 16.kilobytes))
elsif question[:question_type] == "multiple_dropdowns_question"
variables = {}
answers.each_with_index do |arr, idx|
key, answer = arr
answers[idx][1][:blank_id] = escape_and_trim(answers[idx][1][:blank_id], 'blank id', min_size)
answers[idx][1][:blank_id] = check_length(answers[idx][1][:blank_id], 'blank id', min_size)
end
answers.each do |key, answer|
variables[answer[:blank_id]] ||= false
variables[answer[:blank_id]] = true if answer[:answer_weight].to_i == 100
a = {:text => escape_and_trim(answer[:answer_text], 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :blank_id => answer[:blank_id], :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(answer[:answer_text], 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :blank_id => answer[:blank_id], :id => unique_local_id(answer[:id].to_i)}
question[:answers] << a
end
variables.each do |variable, found_correct|
@ -265,12 +260,12 @@ class AssessmentQuestion < ActiveRecord::Base
end
elsif question[:question_type] == "fill_in_multiple_blanks_question"
answers.each do |key, answer|
a = {:text => escape_and_trim(scrub(answer[:answer_text]), 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :blank_id => escape_and_trim(answer[:blank_id], 'blank id', min_size), :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(scrub(answer[:answer_text]), 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :blank_id => check_length(answer[:blank_id], 'blank id', min_size), :id => unique_local_id(answer[:id].to_i)}
question[:answers] << a
end
elsif question[:question_type] == "numerical_question"
answers.each do |key, answer|
a = {:text => escape_and_trim(answer[:answer_text], 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => 100, :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(answer[:answer_text], 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => 100, :id => unique_local_id(answer[:id].to_i)}
a[:numerical_answer_type] = answer[:numerical_answer_type]
if answer[:numerical_answer_type] == "exact_answer"
a[:exact] = answer[:answer_exact].to_f
@ -286,13 +281,13 @@ class AssessmentQuestion < ActiveRecord::Base
question[:formulas] = []
qdata[:formulas].each do |key, formula|
question[:formulas] << {
:formula => escape_and_trim(formula[0..1024], 'formula', min_size)
:formula => check_length(formula[0..1024], 'formula', min_size)
}
end
question[:variables] = []
qdata[:variables].sort_by{|k, v| k[9..-1].to_i}.each do |key, variable|
question[:variables] << {
:name => escape_and_trim(variable[:name][0..1024], 'variable', min_size),
:name => check_length(variable[:name][0..1024], 'variable', min_size),
:min => variable[:min].to_f,
:max => variable[:max].to_f,
:scale => variable[:scale].to_i
@ -305,7 +300,7 @@ class AssessmentQuestion < ActiveRecord::Base
obj[:answer] = answer[:answer_text].to_f
answer[:variables].sort_by{|k, v| k[9..-1].to_i}.each do |key2, variable|
obj[:variables] << {
:name => escape_and_trim(variable[:name], 'variable', min_size),
:name => check_length(variable[:name], 'variable', min_size),
:value => variable[:value].to_f
}
end
@ -315,7 +310,7 @@ class AssessmentQuestion < ActiveRecord::Base
found_correct = false
answers.each do |key, answer|
found_correct = true if answer[:answer_weight].to_i == 100
a = {:text => escape_and_trim(answer[:answer_text], 'answer text', min_size), :comments => escape_and_trim(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :id => unique_local_id(answer[:id].to_i)}
a = {:text => check_length(answer[:answer_text], 'answer text', min_size), :comments => check_length(answer[:answer_comments], 'answer comments', min_size), :weight => answer[:answer_weight].to_f, :id => unique_local_id(answer[:id].to_i)}
question[:answers] << a
end
question[:answers][0][:weight] = 100 unless found_correct

2
app/models/quiz.rb
View File

@ -358,7 +358,7 @@ class Quiz < ActiveRecord::Base
variables.each do |variable|
variable_id = AssessmentQuestion.variable_id(variable)
variable_answers = q[:answers].select{|a| a[:blank_id] == variable }
options = variable_answers.map{|a| "<option value='#{a[:id]}'>#{a[:text]}</option>" }
options = variable_answers.map{|a| "<option value='#{a[:id]}'>#{CGI::escapeHTML(a[:text])}</option>" }
select = "<select class='question_input' name='question_#{q[:id]}_#{variable_id}'><option value=''>[ Select ]</option>#{options}</select>"
re = Regexp.new("\\[#{variable}\\]")
text = text.sub(re, select)

18
app/models/quiz_submission.rb
View File

@ -26,6 +26,7 @@ class QuizSubmission < ActiveRecord::Base
belongs_to :user
belongs_to :submission, :touch => true
before_save :update_kept_score
before_save :sanitize_responses
after_save :update_assignment_submission
serialize :quiz_data
@ -69,6 +70,23 @@ class QuizSubmission < ActiveRecord::Base
set { can :add_attempts }
end
def sanitize_responses
questions && questions.select {|q| q['question_type'] == 'essay_question' }.each do |q|
question_id = q['id']
if submission_data.is_a?(Array)
if submission = submission_data.find {|s| s[:question_id] == question_id }
submission[:text] = Sanitize.clean(submission[:text] || "", Instructure::SanitizeField::SANITIZE)
end
elsif submission_data.is_a?(Hash)
question_key = "question_#{question_id}"
if submission_data[question_key]
submission_data[question_key] = Sanitize.clean(submission_data[question_key] || "", Instructure::SanitizeField::SANITIZE)
end
end
end
true
end
def temporary_data
raise "Cannot view temporary data for completed quiz" unless !self.completed?
raise "Cannot view temporary data for completed quiz" if self.submission_data && !self.submission_data.is_a?(Hash)

2
app/views/quizzes/_display_question.html.erb
View File

@ -212,7 +212,7 @@
<div style="padding: 5px 20px;">
Your Answer:
<% if question_type.entry_type == 'textarea' %>
<div class="user_content quiz_response_text"><%= hash_get(user_answer, :text) %></div>
<div class="user_content quiz_response_text"><%= hash_get(user_answer, :text).html_safe %></div>
<% else %>
<b><%= h(hash_get(user_answer, :text) || "You left this blank") %></b>
<% matched_answer = hash_get(question, :answers, []).find{|a| hash_get(a, :text) == hash_get(user_answer, :text) || hash_get(a, :id) == hash_get(user_answer, :answer_id) } %>

8
app/views/quizzes/_quiz_preview.html.erb
View File

@ -35,7 +35,7 @@ var evaluateAnswersNow = true;
if(optionId) {
var comment = answer_message
var $comment = $("#answer_comment_template").clone().attr('id', '');
$comment.find(".answer_comment").html(comment);
$comment.find(".answer_comment").text(comment);
$answer.append($comment.show())
.append("<div class='clear'></div>");
}
@ -72,7 +72,7 @@ var evaluateAnswersNow = true;
$question.find(".answer_comment_holder").remove();
if(optionId) {
var $comment = $("#answer_comment_template").clone().attr('id', '');
$comment.find(".answer_comment").html(comments);
$comment.find(".answer_comment").text(comments);
if($question.find(".answers .answer").length == 0) {
$question.find(".answers").append("<div class='answer'></div>");
}
@ -138,7 +138,7 @@ var evaluateAnswersNow = true;
if($(this).attr('checked')) {
var comment = answer.comments || answer_message;
var $comment = $("#answer_comment_template").clone().attr('id', '');
$comment.find(".answer_comment").html(comment);
$comment.find(".answer_comment").text(comment);
$answer.append($comment.show())
.append("<div class='clear'></div>");
} else {
@ -203,7 +203,7 @@ var evaluateAnswersNow = true;
}
if(comments) {
var $comment = $("#answer_comment_template").clone().attr('id', '');
$comment.find(".answer_comment").html(comments);
$comment.find(".answer_comment").text(comments);
$answer.append($comment.show())
.append("<div class='clear'></div>");
}

33
public/javascripts/quizzes.js
View File

@ -361,7 +361,7 @@ var quiz = {};
var code = "";
for(var cdx in split) {
if(split[cdx]) {
code = code + "<li>" + split[cdx] + "</li>";
code = code + "<li>" + $.htmlEscape(split[cdx]) + "</li>";
}
}
if(code) {
@ -632,20 +632,12 @@ var quiz = {};
$question.show();
return $question;
}
var $helper_div = $("<div/>");
function fixAmpersands(val, already_escaped) {
if(val && !already_escaped) {
return $helper_div.text(val).html();
} else {
return val;
}
}
function makeDisplayAnswer(data, escaped) {
data.answer_weight = data.weight || data.answer_weight;
data.answer_comment = fixAmpersands(data.comments || data.answer_comment, escaped);
data.answer_text = fixAmpersands(data.text || data.answer_text, escaped);
data.answer_match_left = fixAmpersands(data.left || data.answer_match_left, escaped);
data.answer_match_right = fixAmpersands(data.right || data.answer_match_right, escaped);
data.answer_comment = data.comments || data.answer_comment;
data.answer_text = data.text || data.answer_text;
data.answer_match_left = data.left || data.answer_match_left;
data.answer_match_right = data.right || data.answer_match_right;
data.answer_exact = data.exact || data.answer_exact;
data.answer_error_margin = data.answer_error_margin || data.margin;
data.answer_range_start = data.start || data.answer_range_start;
@ -662,12 +654,8 @@ var quiz = {};
delete answer['answer_type'];
answer.answer_weight = parseFloat(answer.answer_weight);
if(isNaN(answer.answer_weight)) { answer.answer_weight = 0; }
var unescaped = $helper_div.html(answer.answer_text).text();
$answer.fillFormData({answer_text: unescaped});
$answer.fillTemplateData({
data: answer,
htmlValues: ['answer_text', 'answer_comment', 'answer_match_left', 'answer_match_right']
});
$answer.fillFormData({answer_text: answer.answer_text});
$answer.fillTemplateData({data: answer});
if(!answer.answer_comment || answer.answer_comment == "" || answer.answer_comment == "Answer comments") {
$answer.find(".answer_comment_holder").hide();
}
@ -702,8 +690,8 @@ var quiz = {};
$list.each(function(i) {
var $question = $(this);
var questionData = $question.getTemplateData({
textValues: ['question_name', 'question_points', 'question_type', 'answer_selection_type', 'assessment_question_id', 'correct_comments', 'incorrect_comments', 'neutral_comments', 'text_before_answers', 'text_after_answers', 'matching_answer_incorrect_matches', 'equation_combinations', 'equation_formulas'],
htmlValues: ['question_text']
textValues: ['question_name', 'question_points', 'question_type', 'answer_selection_type', 'assessment_question_id', 'correct_comments', 'incorrect_comments', 'neutral_comments', 'matching_answer_incorrect_matches', 'equation_combinations', 'equation_formulas'],
htmlValues: ['question_text', 'text_before_answers', 'text_after_answers']
});
questionData = $.extend(questionData, $question.find(".original_question_text").getFormData());
questionData.assessment_question_bank_id = $(".question_bank_id").text() || "";
@ -729,8 +717,7 @@ var quiz = {};
$question.find(".answer").each(function() {
var $answer = $(this);
var answerData = $answer.getTemplateData({
textValues: ['answer_exact', 'answer_error_margin', 'answer_range_start', 'answer_range_end', 'answer_weight', 'numerical_answer_type', 'blank_id', 'id', 'match_id'],
htmlValues: ['answer_text', 'answer_match_left', 'answer_match_right', 'answer_comment']
textValues: ['answer_exact', 'answer_error_margin', 'answer_range_start', 'answer_range_end', 'answer_weight', 'numerical_answer_type', 'blank_id', 'id', 'match_id', 'answer_text', 'answer_match_left', 'answer_match_right', 'answer_comment']
});
var answer = $.extend({}, quiz.defaultAnswerData, answerData);
if(only_add_for_blank_ids && answer.blank_id && !blank_ids_hash[answer.blank_id]) {