From e49bcebb35b04a940701eb858b4f0997cd998f22 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Mon, 31 Dec 2018 11:48:30 +0200
Subject: [PATCH] FIX: Validate poll arguments. (#6740)
* FIX: Validate number poll.
* FEATURE: Poll's min can be 0.
* FIX: Fix URL to user profile.
---
plugins/poll/app/models/poll.rb | 2 +-
.../javascripts/widgets/discourse-poll.js.es6 | 6 +-
plugins/poll/config/locales/server.en.yml | 2 +
plugins/poll/lib/polls_validator.rb | 66 +++++++++
plugins/poll/spec/lib/polls_validator_spec.rb | 127 +++++++++++++++---
.../javascripts/acceptance/polls-test.js.es6 | 2 +
6 files changed, 180 insertions(+), 25 deletions(-)
diff --git a/plugins/poll/app/models/poll.rb b/plugins/poll/app/models/poll.rb
index 192e599ef8a..fd5422a620b 100644
--- a/plugins/poll/app/models/poll.rb
+++ b/plugins/poll/app/models/poll.rb
@@ -29,7 +29,7 @@ class Poll < ActiveRecord::Base
everyone: 1,
}
- validates :min, numericality: { allow_nil: true, only_integer: true, greater_than: 0 }
+ validates :min, numericality: { allow_nil: true, only_integer: true, greater_than_or_equal_to: 0 }
validates :max, numericality: { allow_nil: true, only_integer: true, greater_than: 0 }
validates :step, numericality: { allow_nil: true, only_integer: true, greater_than: 0 }
diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6
index 88195905657..0ac240dee62 100644
--- a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6
+++ b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6
@@ -8,6 +8,7 @@ import evenRound from "discourse/plugins/poll/lib/even-round";
import { avatarFor } from "discourse/widgets/post";
import round from "discourse/lib/round";
import { relativeAge } from "discourse/lib/formatter";
+import { userPath } from "discourse/lib/url";
function optionHtml(option) {
return new RawHtml({ html: `<span>${option.html}</span>` });
@@ -143,6 +144,7 @@ createWidget("discourse-poll-voters", {
return h("li", [
avatarFor("tiny", {
username: user.username,
+ url: userPath(user.username),
template: user.avatar_template
}),
" "
@@ -560,8 +562,8 @@ export default createWidget("discourse-poll", {
min() {
let min = parseInt(this.attrs.poll.get("min"), 10);
- if (isNaN(min) || min < 1) {
- min = 1;
+ if (isNaN(min) || min < 0) {
+ min = 0;
}
return min;
},
diff --git a/plugins/poll/config/locales/server.en.yml b/plugins/poll/config/locales/server.en.yml
index 6d5274d6413..8e4abe64a84 100644
--- a/plugins/poll/config/locales/server.en.yml
+++ b/plugins/poll/config/locales/server.en.yml
@@ -22,6 +22,8 @@ en:
poll_minimum_trust_level_to_create: "Define the minimum trust level needed to create polls."
poll:
+ invalid_argument: "Invalid value '%{value}' for argument '%{argument}'."
+
multiple_polls_without_name: "There are multiple polls without a name. Use the '<code>name</code>' attribute to uniquely identify your polls."
multiple_polls_with_same_name: "There are multiple polls with the same name: <strong>%{name}</strong>. Use the '<code>name</code>' attribute to uniquely identify your polls."
diff --git a/plugins/poll/lib/polls_validator.rb b/plugins/poll/lib/polls_validator.rb
index 790d8e2c010..fca71f89fc4 100644
--- a/plugins/poll/lib/polls_validator.rb
+++ b/plugins/poll/lib/polls_validator.rb
@@ -1,5 +1,8 @@
module DiscoursePoll
class PollsValidator
+
+ MAX_VALUE = 2_147_483_647
+
def initialize(post)
@post = post
end
@@ -8,6 +11,8 @@ module DiscoursePoll
polls = {}
DiscoursePoll::Poll::extract(@post.raw, @post.topic_id, @post.user_id).each do |poll|
+ return false unless valid_arguments?(poll)
+ return false unless valid_numbers?(poll)
return false unless unique_poll_name?(polls, poll)
return false unless unique_options?(poll)
return false unless at_least_two_options?(poll)
@@ -21,6 +26,27 @@ module DiscoursePoll
private
+ def valid_arguments?(poll)
+ valid = true
+
+ if poll["type"].present? && !::Poll.types.has_key?(poll["type"])
+ @post.errors.add(:base, I18n.t("poll.invalid_argument", argument: "type", value: poll["type"]))
+ valid = false
+ end
+
+ if poll["status"].present? && !::Poll.statuses.has_key?(poll["status"])
+ @post.errors.add(:base, I18n.t("poll.invalid_argument", argument: "status", value: poll["status"]))
+ valid = false
+ end
+
+ if poll["results"].present? && !::Poll.results.has_key?(poll["results"])
+ @post.errors.add(:base, I18n.t("poll.invalid_argument", argument: "results", value: poll["results"]))
+ valid = false
+ end
+
+ valid
+ end
+
def unique_poll_name?(polls, poll)
if polls.has_key?(poll["name"])
if poll["name"] == ::DiscoursePoll::DEFAULT_POLL_NAME
@@ -96,5 +122,45 @@ module DiscoursePoll
true
end
+
+ def valid_numbers?(poll)
+ return true if poll["type"] != "number"
+
+ valid = true
+
+ min = poll["min"].to_i
+ max = (poll["max"].presence || MAX_VALUE).to_i
+ step = (poll["step"].presence || 1).to_i
+
+ if min < 0
+ @post.errors.add(:base, "Min #{I18n.t("errors.messages.greater_than", count: 0)}")
+ valid = false
+ elsif min > MAX_VALUE
+ @post.errors.add(:base, "Min #{I18n.t("errors.messages.less_than", count: MAX_VALUE)}")
+ valid = false
+ end
+
+ if max < min
+ @post.errors.add(:base, "Max #{I18n.t("errors.messages.greater_than", count: "min")}")
+ valid = false
+ elsif max > MAX_VALUE
+ @post.errors.add(:base, "Max #{I18n.t("errors.messages.less_than", count: MAX_VALUE)}")
+ valid = false
+ end
+
+ if step <= 0
+ @post.errors.add(:base, "Step #{I18n.t("errors.messages.greater_than", count: 0)}")
+ valid = false
+ elsif ((max - min + 1) / step) < 2
+ if poll["name"] == ::DiscoursePoll::DEFAULT_POLL_NAME
+ @post.errors.add(:base, I18n.t("poll.default_poll_must_have_at_least_2_options"))
+ else
+ @post.errors.add(:base, I18n.t("poll.named_poll_must_have_at_least_2_options", name: poll["name"]))
+ end
+ valid = false
+ end
+
+ valid
+ end
end
end
diff --git a/plugins/poll/spec/lib/polls_validator_spec.rb b/plugins/poll/spec/lib/polls_validator_spec.rb
index 56cd7d17b8d..ee2d4b37e70 100644
--- a/plugins/poll/spec/lib/polls_validator_spec.rb
+++ b/plugins/poll/spec/lib/polls_validator_spec.rb
@@ -5,6 +5,53 @@ describe ::DiscoursePoll::PollsValidator do
subject { described_class.new(post) }
describe "#validate_polls" do
+ it "ensures that polls have valid arguments" do
+ raw = <<~RAW
+ [poll type=not_good1 status=not_good2 results=not_good3]
+ * 1
+ * 2
+ [/poll]
+ RAW
+
+ post.raw = raw
+ expect(post.valid?).to eq(false)
+
+ expect(post.errors[:base]).to include(I18n.t("poll.invalid_argument", argument: "type", value: "not_good1"))
+ expect(post.errors[:base]).to include(I18n.t("poll.invalid_argument", argument: "status", value: "not_good2"))
+ expect(post.errors[:base]).to include(I18n.t("poll.invalid_argument", argument: "results", value: "not_good3"))
+ end
+
+ it "ensures that all possible values are valid" do
+ raw = <<~RAW
+ [poll type=regular result=always]
+ * 1
+ * 2
+ [/poll]
+ RAW
+
+ post.raw = raw
+ expect(post.valid?).to eq(true)
+
+ raw = <<~RAW
+ [poll type=multiple result=on_vote min=1 max=2]
+ * 1
+ * 2
+ * 3
+ [/poll]
+ RAW
+
+ post.raw = raw
+ expect(post.valid?).to eq(true)
+
+ raw = <<~RAW
+ [poll type=number result=on_close min=3 max=7]
+ [/poll]
+ RAW
+
+ post.raw = raw
+ expect(post.valid?).to eq(true)
+ end
+
it "ensure that polls have unique names" do
raw = <<~RAW
[poll]
@@ -18,7 +65,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.multiple_polls_without_name")
@@ -36,7 +84,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.multiple_polls_with_same_name", name: "test")
@@ -51,7 +100,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_must_have_different_options")
@@ -64,7 +114,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.named_poll_must_have_different_options", name: "test")
@@ -78,7 +129,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_must_have_at_least_2_options")
@@ -90,7 +142,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.named_poll_must_have_at_least_2_options", name: "test")
@@ -108,7 +161,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(I18n.t(
"poll.default_poll_must_have_less_options",
@@ -123,7 +177,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(I18n.t(
"poll.named_poll_must_have_less_options",
@@ -141,7 +196,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
@@ -155,7 +211,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.named_poll_with_multiple_choices_has_invalid_parameters", name: "test")
@@ -170,7 +227,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
@@ -185,14 +243,15 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
)
end
- it "ensure that min > 0" do
+ it "ensure that min >= 0" do
raw = <<~RAW
[poll type=multiple min=-1]
* 1
@@ -200,14 +259,15 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
)
end
- it "ensure that min != 0" do
+ it "ensure that min cannot be 0" do
raw = <<~RAW
[poll type=multiple min=0]
* 1
@@ -215,11 +275,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
-
- expect(post.errors[:base]).to include(
- I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
- )
+ post.raw = raw
+ expect(post.valid?).to eq(false)
end
it "ensure that min != number of options" do
@@ -230,7 +287,8 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
@@ -245,12 +303,37 @@ describe ::DiscoursePoll::PollsValidator do
[/poll]
RAW
- expect(post.update_attributes(raw: raw)).to eq(false)
+ post.raw = raw
+ expect(post.valid?).to eq(false)
expect(post.errors[:base]).to include(
I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")
)
end
end
+
+ it "number type polls are validated" do
+ raw = <<~RAW
+ [poll type=number min=-5 max=-10 step=-1]
+ [/poll]
+ RAW
+
+ post.raw = raw
+ expect(post.valid?).to eq(false)
+ expect(post.errors[:base]).to include("Min #{I18n.t("errors.messages.greater_than", count: 0)}")
+ expect(post.errors[:base]).to include("Max #{I18n.t("errors.messages.greater_than", count: "min")}")
+ expect(post.errors[:base]).to include("Step #{I18n.t("errors.messages.greater_than", count: 0)}")
+
+ raw = <<~RAW
+ [poll type=number min=9999999999 max=9999999999 step=1]
+ [/poll]
+ RAW
+
+ post.raw = raw
+ expect(post.valid?).to eq(false)
+ expect(post.errors[:base]).to include("Min #{I18n.t("errors.messages.less_than", count: 2_147_483_647)}")
+ expect(post.errors[:base]).to include("Max #{I18n.t("errors.messages.less_than", count: 2_147_483_647)}")
+ expect(post.errors[:base]).to include(I18n.t("poll.default_poll_must_have_at_least_2_options"))
+ end
end
end
diff --git a/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 b/plugins/poll/test/javascripts/acceptance/polls-test.js.es6
index 1f4dbf4f5b1..357af88094b 100644
--- a/plugins/poll/test/javascripts/acceptance/polls-test.js.es6
+++ b/plugins/poll/test/javascripts/acceptance/polls-test.js.es6
@@ -2023,6 +2023,8 @@ test("Public number poll", async assert => {
"it should display the right number of voters"
);
+ assert.ok(find(".poll-voters:first li:first a").attr("href"), "user URL exists");
+
await click(".poll-voters-toggle-expand:first a");
assert.equal(