diff --git a/app/models/quizzes/quiz_question/answer_parsers/essay.rb b/app/models/quizzes/quiz_question/answer_parsers/essay.rb index 9a1fdeba56d..aa10791f3c5 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/essay.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/essay.rb @@ -24,7 +24,7 @@ module Quizzes::QuizQuestion::AnswerParsers answer = Quizzes::QuizQuestion::RawFields.new({comments: comment, comments_html: comments_html}) question[:comments] = answer.fetch_with_enforced_length(:comments, max_size: 5.kilobyte) - question[:comments_html] = answer.fetch_with_enforced_length(:comments_html, max_size: 5.kilobyte) + question[:comments_html] = answer.sanitize(answer.fetch_with_enforced_length(:comments_html, max_size: 5.kilobyte)) question.answers = @answers question diff --git a/app/models/quizzes/quiz_question/answer_parsers/fill_in_multiple_blanks.rb b/app/models/quizzes/quiz_question/answer_parsers/fill_in_multiple_blanks.rb index 7b361281729..3dbcb7b3147 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/fill_in_multiple_blanks.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/fill_in_multiple_blanks.rb @@ -26,7 +26,7 @@ module Quizzes::QuizQuestion::AnswerParsers id: fields.fetch_any(:id, nil), text: fields.fetch_with_enforced_length([:answer_text, :text]), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), weight: fields.fetch_any([:answer_weight, :weight]).to_f, blank_id: fields.fetch_with_enforced_length(:blank_id) }) diff --git a/app/models/quizzes/quiz_question/answer_parsers/matching.rb b/app/models/quizzes/quiz_question/answer_parsers/matching.rb index 60106dde36d..01ce79c5175 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/matching.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/matching.rb @@ -30,7 +30,7 @@ module Quizzes::QuizQuestion::AnswerParsers left: fields.fetch_with_enforced_length(:answer_match_left), right: fields.fetch_with_enforced_length(:answer_match_right), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]) + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])) } a[:left_html] = a[:html] = fields.sanitize(fields.fetch_any(:answer_match_left_html)) if answer[:answer_match_left_html].present? diff --git a/app/models/quizzes/quiz_question/answer_parsers/missing_word.rb b/app/models/quizzes/quiz_question/answer_parsers/missing_word.rb index 2cf05294921..03640e119b6 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/missing_word.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/missing_word.rb @@ -24,7 +24,7 @@ module Quizzes::QuizQuestion::AnswerParsers a = { text: fields.fetch_with_enforced_length([:answer_text, :text]), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), weight: fields.fetch_any([:answer_weight, :weight]).to_f } diff --git a/app/models/quizzes/quiz_question/answer_parsers/multiple_answers.rb b/app/models/quizzes/quiz_question/answer_parsers/multiple_answers.rb index 338291fe403..6320ed936c2 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/multiple_answers.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/multiple_answers.rb @@ -26,7 +26,7 @@ module Quizzes::QuizQuestion::AnswerParsers id: fields.fetch_any(:id, nil), text: fields.fetch_with_enforced_length([:answer_text, :text]), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), weight: fields.fetch_any([:answer_weight, :weight]).to_f } diff --git a/app/models/quizzes/quiz_question/answer_parsers/multiple_choice.rb b/app/models/quizzes/quiz_question/answer_parsers/multiple_choice.rb index 86f6e2bc274..97967e5fd47 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/multiple_choice.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/multiple_choice.rb @@ -26,7 +26,7 @@ module Quizzes::QuizQuestion::AnswerParsers id = id.to_i if id text = fields.fetch_with_enforced_length([:answer_text, :text]) comments = fields.fetch_with_enforced_length([:answer_comment, :comments]) - comments_html = fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]) + comments_html = fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])) weight = fields.fetch_any([:answer_weight, :weight]).to_f html = fields.sanitize(fields.fetch_any([:answer_html, :html])) diff --git a/app/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns.rb b/app/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns.rb index a98f53b382e..8a0729a4389 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns.rb @@ -28,7 +28,7 @@ module Quizzes::QuizQuestion::AnswerParsers id: fields.fetch_any([:id, :answer_id], nil).try(:to_i), text: fields.fetch_with_enforced_length([:answer_text, :text]), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), weight: fields.fetch_any(:answer_weight, 0).to_f, blank_id: fields.fetch_with_enforced_length(:blank_id) } diff --git a/app/models/quizzes/quiz_question/answer_parsers/numerical.rb b/app/models/quizzes/quiz_question/answer_parsers/numerical.rb index ddcd0edaf3d..fd06143822c 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/numerical.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/numerical.rb @@ -25,7 +25,7 @@ module Quizzes::QuizQuestion::AnswerParsers id: fields.fetch_any(:id, nil), text: fields.fetch_with_enforced_length([:answer_text, :text]), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), weight: 100 } diff --git a/app/models/quizzes/quiz_question/answer_parsers/short_answer.rb b/app/models/quizzes/quiz_question/answer_parsers/short_answer.rb index 13f640ba729..3548c29ff5b 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/short_answer.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/short_answer.rb @@ -25,7 +25,7 @@ module Quizzes::QuizQuestion::AnswerParsers id: fields.fetch_any([:id, :answer_id], nil), text: fields.fetch_with_enforced_length([:answer_text, :text]), comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), weight: 100 } diff --git a/app/models/quizzes/quiz_question/answer_parsers/true_false.rb b/app/models/quizzes/quiz_question/answer_parsers/true_false.rb index e7615da77cb..bf3b83f04f8 100644 --- a/app/models/quizzes/quiz_question/answer_parsers/true_false.rb +++ b/app/models/quizzes/quiz_question/answer_parsers/true_false.rb @@ -23,7 +23,7 @@ module Quizzes::QuizQuestion::AnswerParsers fields = Quizzes::QuizQuestion::RawFields.new(answer) a = { comments: fields.fetch_with_enforced_length([:answer_comment, :comments]), - comments_html: fields.fetch_with_enforced_length([:answer_comment_html, :comments_html]), + comments_html: fields.sanitize(fields.fetch_with_enforced_length([:answer_comment_html, :comments_html])), text: fields.fetch_any([:answer_text, :text]), weight: fields.fetch_any([:answer_weight, :weight]).to_i } diff --git a/app/models/quizzes/quiz_question/question_data.rb b/app/models/quizzes/quiz_question/question_data.rb index 35d1e0a4643..72427b69c74 100644 --- a/app/models/quizzes/quiz_question/question_data.rb +++ b/app/models/quizzes/quiz_question/question_data.rb @@ -69,9 +69,9 @@ class Quizzes::QuizQuestion::QuestionData question[:incorrect_comments] = fields.fetch_with_enforced_length(:incorrect_comments, max_size: 5.kilobyte) question[:neutral_comments] = fields.fetch_with_enforced_length(:neutral_comments, max_size: 5.kilobyte) - question[:correct_comments_html] = fields.fetch_with_enforced_length(:correct_comments_html, max_size: 5.kilobyte) - question[:incorrect_comments_html] = fields.fetch_with_enforced_length(:incorrect_comments_html, max_size: 5.kilobyte) - question[:neutral_comments_html] = fields.fetch_with_enforced_length(:neutral_comments_html, max_size: 5.kilobyte) + question[:correct_comments_html] = fields.sanitize(fields.fetch_with_enforced_length(:correct_comments_html, max_size: 5.kilobyte)) + question[:incorrect_comments_html] = fields.sanitize(fields.fetch_with_enforced_length(:incorrect_comments_html, max_size: 5.kilobyte)) + question[:neutral_comments_html] = fields.sanitize(fields.fetch_with_enforced_length(:neutral_comments_html, max_size: 5.kilobyte)) question[:question_type] = fields.fetch_any(:question_type, "text_only_question") question[:question_name] = fields.fetch_any(:question_name, I18n.t(:default_question_name, "Question")) @@ -79,7 +79,7 @@ class Quizzes::QuizQuestion::QuestionData question[:name] = question[:question_name] question[:question_text] = fields.sanitize(fields.fetch_with_enforced_length(:question_text, default: I18n.t(:default_question_text, "Question text"))) question[:answers] = fields.fetch_any(:answers, []) - question[:text_after_answers] = fields.fetch_any(:text_after_answers) + question[:text_after_answers] = fields.sanitize(fields.fetch_any(:text_after_answers)) if question.is_type?(:calculated) question[:formulas] = fields.fetch_any(:formulas, []) diff --git a/spec/models/quizzes/quiz_question/answer_parsers/answer_parser_spec_helper.rb b/spec/models/quizzes/quiz_question/answer_parsers/answer_parser_spec_helper.rb index 812b485087f..41147471278 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/answer_parser_spec_helper.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/answer_parser_spec_helper.rb @@ -47,6 +47,11 @@ shared_examples_for "All answer parsers" do ids = @answer_data.answers.map { |a| a[:id] } ids.each { |id| expect(id).to be_kind_of(Integer) } end + + it "sanitizes answer comments" do + expect(@answer_data.first[:comments_html]).to include('' } ] end @@ -39,6 +40,8 @@ describe Quizzes::QuizQuestion::AnswerParsers::Essay do question = Quizzes::QuizQuestion::QuestionData.new({}) essay.parse(question) expect(question[:comments]).to eq raw_answers[0][:answer_comments] + expect(question[:comments_html]).to include '', answer_weight: 100, blank_id: "answer1" }, diff --git a/spec/models/quizzes/quiz_question/answer_parsers/matching_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/matching_spec.rb index 76be137f90e..4b8e6cc1c70 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/matching_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/matching_spec.rb @@ -29,24 +29,22 @@ describe Quizzes::QuizQuestion::AnswerParsers::Matching do answer_match_left: "Answer 1", answer_match_right: "Match to Answer 1", answer_comment: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { answer_text: "Answer 2", answer_match_left: "Answer 2", answer_match_right: "Match to Answer 2", answer_comment: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Answer 3", answer_match_left: "Answer 3", answer_match_right: "Match to Answer 3", answer_comment: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end @@ -70,24 +68,21 @@ describe Quizzes::QuizQuestion::AnswerParsers::Matching do answer_match_left: "Salt Lake City", answer_match_right: "Utah", answer_comment: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_weight: 0 }, { answer_text: "San Diego", answer_match_left: "San Diego", answer_match_right: "California", answer_comment: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Los Angeles", answer_match_left: "Los Angeles", answer_match_right: "California", answer_comment: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/missing_word_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/missing_word_spec.rb index ce2b009703c..df238ad77c0 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/missing_word_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/missing_word_spec.rb @@ -28,20 +28,18 @@ describe Quizzes::QuizQuestion::AnswerParsers::MissingWord do { answer_text: "Answer 1", answer_comments: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { answer_text: "Answer 2", answer_comments: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Answer 3", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/multiple_answers_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/multiple_answers_spec.rb index f877d7bc189..b290965e0bb 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/multiple_answers_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/multiple_answers_spec.rb @@ -27,20 +27,18 @@ describe Quizzes::QuizQuestion::AnswerParsers::MultipleAnswers do { answer_text: "Answer 1", answer_comments: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { answer_text: "Answer 2", answer_comments: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Answer 3", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/multiple_choice_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/multiple_choice_spec.rb index d594be652db..bc0fc28c5fc 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/multiple_choice_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/multiple_choice_spec.rb @@ -28,22 +28,20 @@ describe Quizzes::QuizQuestion::AnswerParsers::MultipleChoice do answer_text: "Answer 1", answer_html: "", answer_comments: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { answer_text: "Answer 2", answer_html: "", answer_comments: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Answer 3", answer_html: "", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns_spec.rb index b7ca96b416b..0268a5a472b 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/multiple_dropdowns_spec.rb @@ -27,8 +27,8 @@ describe Quizzes::QuizQuestion::AnswerParsers::MultipleDropdowns do { answer_text: "Answer 1", answer_comments: "This is answer 1", + answer_comment_html: '', answer_weight: 0, - text_after_answers: "Text after Answer 1", id: '1000' }, @@ -36,14 +36,12 @@ describe Quizzes::QuizQuestion::AnswerParsers::MultipleDropdowns do answer_text: "Answer 2", answer_comments: "This is answer 2", answer_weight: 100, - text_after_answers: "Text after Answer 2", id: 1001 }, { answer_text: "Answer 3", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/numerical_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/numerical_spec.rb index 22f994d336d..82e8a4da555 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/numerical_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/numerical_spec.rb @@ -27,20 +27,18 @@ describe Quizzes::QuizQuestion::AnswerParsers::Numerical do { answer_text: "Answer 1", answer_comments: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { answer_text: "Answer 2", answer_comments: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Answer 3", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/short_answer_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/short_answer_spec.rb index 7cd91f59a85..7c90d75a711 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/short_answer_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/short_answer_spec.rb @@ -27,20 +27,18 @@ describe Quizzes::QuizQuestion::AnswerParsers::ShortAnswer do { answer_text: "Answer 1", answer_comments: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { answer_text: "Answer 2", answer_comments: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { answer_text: "Answer 3", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question/answer_parsers/true_false_spec.rb b/spec/models/quizzes/quiz_question/answer_parsers/true_false_spec.rb index a59536fc9ad..46f21d06d46 100644 --- a/spec/models/quizzes/quiz_question/answer_parsers/true_false_spec.rb +++ b/spec/models/quizzes/quiz_question/answer_parsers/true_false_spec.rb @@ -25,22 +25,20 @@ describe Quizzes::QuizQuestion::AnswerParsers::TrueFalse do let(:raw_answers) do [ { - answer_text: "Answer 1", + answer_text: "True", answer_comments: "This is answer 1", - answer_weight: 0, - text_after_answers: "Text after Answer 1" + answer_comment_html: '', + answer_weight: 0 }, { - answer_text: "Answer 2", + answer_text: "False", answer_comments: "This is answer 2", - answer_weight: 100, - text_after_answers: "Text after Answer 2" + answer_weight: 100 }, { - answer_text: "Answer 3", + answer_text: "File not found", answer_comments: "This is answer 3", - answer_weight: 0, - text_after_answers: "Text after Answer 3" + answer_weight: 0 } ] end diff --git a/spec/models/quizzes/quiz_question_spec.rb b/spec/models/quizzes/quiz_question_spec.rb index 4bd5346cf42..daee9379679 100644 --- a/spec/models/quizzes/quiz_question_spec.rb +++ b/spec/models/quizzes/quiz_question_spec.rb @@ -86,6 +86,45 @@ describe Quizzes::QuizQuestion do expect(question_regrade).to be expect(question_regrade.regrade_option).to eq 'full_credit' end + + it "sanitizes all the html" do + question_data = { + "id"=>nil, + "regrade_option"=>"", + "points_possible"=>1.0, + "correct_comments_html"=>"", + "incorrect_comments_html"=>"", + "neutral_comments_html"=>"", + "question_type"=>"multiple_choice_question", + "question_name"=>"Question", + "name"=>"Question", + "question_text"=>"", + "answers"=> + [ + { + "id"=>8206, + "html"=>"", + "comments_html"=>"", + "weight"=>100.0 + }, + { + "id"=>6973, + "html"=>"", + "comments_html"=>"", + "weight"=>0.0 + } + ] + } + qq = @quiz.quiz_questions.create(:question_data => question_data) + expect(qq.question_data['correct_comments_html']).not_to include('onerror') + expect(qq.question_data['incorrect_comments_html']).not_to include('onerror') + expect(qq.question_data['neutral_comments_html']).not_to include('onerror') + expect(qq.question_data['question_text']).not_to include('onerror') + expect(qq.question_data['answers'][0]['html']).not_to include('onerror') + expect(qq.question_data['answers'][0]['comments_html']).not_to include('onerror') + expect(qq.question_data['answers'][1]['html']).not_to include('onerror') + expect(qq.question_data['answers'][1]['comments_html']).not_to include('onerror') + end end describe ".update_all_positions" do