diff --git a/app/models/assessment_question.rb b/app/models/assessment_question.rb index 5526e16b44f..103eb727957 100644 --- a/app/models/assessment_question.rb +++ b/app/models/assessment_question.rb @@ -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 diff --git a/app/models/quiz.rb b/app/models/quiz.rb index fa4f82de0fd..0ee08e67fb1 100644 --- a/app/models/quiz.rb +++ b/app/models/quiz.rb @@ -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| "" } + options = variable_answers.map{|a| "" } select = "" re = Regexp.new("\\[#{variable}\\]") text = text.sub(re, select) diff --git a/app/models/quiz_submission.rb b/app/models/quiz_submission.rb index ed4e02848a1..43c695b4eef 100644 --- a/app/models/quiz_submission.rb +++ b/app/models/quiz_submission.rb @@ -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) diff --git a/app/views/quizzes/_display_question.html.erb b/app/views/quizzes/_display_question.html.erb index ea259dd5e51..7b9023e783f 100644 --- a/app/views/quizzes/_display_question.html.erb +++ b/app/views/quizzes/_display_question.html.erb @@ -212,7 +212,7 @@
Your Answer: <% if question_type.entry_type == 'textarea' %> -
<%= hash_get(user_answer, :text) %>
+
<%= hash_get(user_answer, :text).html_safe %>
<% else %> <%= h(hash_get(user_answer, :text) || "You left this blank") %> <% 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) } %> diff --git a/app/views/quizzes/_quiz_preview.html.erb b/app/views/quizzes/_quiz_preview.html.erb index a608d843327..aa333e0633c 100644 --- a/app/views/quizzes/_quiz_preview.html.erb +++ b/app/views/quizzes/_quiz_preview.html.erb @@ -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("
"); } @@ -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("
"); } @@ -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("
"); } 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("
"); } diff --git a/public/javascripts/quizzes.js b/public/javascripts/quizzes.js index c60f64aa030..9106cf0f4b9 100644 --- a/public/javascripts/quizzes.js +++ b/public/javascripts/quizzes.js @@ -361,7 +361,7 @@ var quiz = {}; var code = ""; for(var cdx in split) { if(split[cdx]) { - code = code + "
  • " + split[cdx] + "
  • "; + code = code + "
  • " + $.htmlEscape(split[cdx]) + "
  • "; } } if(code) { @@ -632,20 +632,12 @@ var quiz = {}; $question.show(); return $question; } - var $helper_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]) {