Merge pull request #1510 from alphagov/add-input-length-validations

Add input length validations

Author: stephencdaly

app/models/question/address.rb

@@ -11,7 +11,7 @@ class Address < Question::QuestionBase
 
     validate :uk_address_valid?, unless: :is_international_address?
     validate :postcode, :invalid_postcode?, unless: :is_international_address?
-    validate :international_adress_valid?, if: :is_international_address?
+    validate :international_address_valid?, if: :is_international_address?
 
     def postcode=(str)
       super str.present? ? UKPostcode.parse(str).to_s : str
@@ -41,16 +41,22 @@ def uk_address_valid?
       return if skipping_question?
 
       errors.add(:address1, :blank) if address1.blank?
+      errors.add(:address1, :too_long) if address1.present? && address1.length > 499
+      errors.add(:address2, :too_long) if address2.present? && address2.length > 499
       errors.add(:town_or_city, :blank) if town_or_city.blank?
+      errors.add(:town_or_city, :too_long) if town_or_city.present? && town_or_city.length > 499
+      errors.add(:county, :too_long) if county.present? && county.length > 499
       errors.add(:postcode, :blank) if postcode.blank?
       errors
     end
 
-    def international_adress_valid?
+    def international_address_valid?
       return if skipping_question?
 
       errors.add(:street_address, :blank) if street_address.blank?
+      errors.add(:street_address, :too_long) if street_address.present? && street_address.length > 4999
       errors.add(:country, :blank) if country.blank?
+      errors.add(:country, :too_long) if country.present? && country.length > 499
       errors
     end
   end

app/models/question/email.rb

@@ -1,13 +1,7 @@
 module Question
   class Email < Question::QuestionBase
-    # see https://design-system.service.gov.uk/patterns/email-addresses/
-    # This model contains very lite checking - only that an @ exists. Pay and
-    # notify both have good examples of better email validation checks and ways
-    # to help users enter the right value
-
-    EMAIL_REGEX = /.*@.*/
     attribute :email
     validates :email, presence: true, unless: :is_optional?
-    validates :email, format: { with: EMAIL_REGEX, message: :invalid_email }, allow_blank: true
+    validates :email, email_address: { message: :invalid_email }, allow_blank: true
   end
 end

app/models/question/name.rb

@@ -6,12 +6,9 @@ class Name < Question::QuestionBase
     attribute :middle_names
     attribute :last_name
 
+    validates :title, length: { maximum: 99 }, allow_blank: true
     validate :full_name_valid?, if: :is_full_name?
-    validate :first_and_last_name_valid?, unless: :is_full_name?
-
-    def validate_title
-      needs_title? && !is_optional?
-    end
+    validate :first_middle_and_last_name_valid?, unless: :is_full_name?
 
     def needs_title?
       answer_settings.present? && answer_settings&.title_needed == "true"
@@ -34,14 +31,18 @@ def full_name_valid?
       return if skipping_question?
 
       errors.add(:full_name, :blank) if full_name.blank?
+      errors.add(:full_name, :too_long) if full_name.present? && full_name.length > 499
       errors
     end
 
-    def first_and_last_name_valid?
+    def first_middle_and_last_name_valid?
       return if skipping_question?
 
       errors.add(:first_name, :blank) if first_name.blank?
+      errors.add(:first_name, :too_long) if first_name.present? && first_name.length > 499
+      errors.add(:middle_names, :too_long) if include_middle_name? && middle_names.present? && middle_names.length > 499
       errors.add(:last_name, :blank) if last_name.blank?
+      errors.add(:last_name, :too_long) if last_name.present? && last_name.length > 499
       errors
     end
 

app/models/question/number.rb

@@ -3,5 +3,6 @@ class Number < QuestionBase
     attribute :number
     validates :number, presence: true, unless: :is_optional?
     validates :number, numericality: { greater_than_or_equal_to: 0 }, allow_blank: true
+    validates :number, length: { maximum: 499 }, allow_blank: true
   end
 end

config/locales/cy.yml

@@ -95,15 +95,23 @@ cy:
           attributes:
             address1:
               blank: Rhowch linell gyntaf y cyfeiriad
+              too_long: The first line of the address must be shorter than 500 characters
+            address2:
+              too_long: The second line of the address must be shorter than 500 characters
             country:
               blank: Rhowch y wlad
+              too_long: The country must be shorter than 500 characters
+            county:
+              too_long: The county must be shorter than 500 characters
             postcode:
               blank: Rhowch y cod post
               invalid_postcode: Rhowch god post gwirioneddol
             street_address:
               blank: Rhowch y cyfeiriad
+              too_long: The address must be shorter than 5,000 characters
             town_or_city:
               blank: Rhowch y dref neu’r ddinas
+              too_long: The town or city must be shorter than 500 characters
         question/date:
           attributes:
             date:
@@ -130,10 +138,17 @@ cy:
           attributes:
             first_name:
               blank: Rhowch enw cyntaf
+              too_long: The first name must be shorter than 500 characters
             full_name:
               blank: Rhowch enw
+              too_long: The name must be shorter than 500 characters
             last_name:
               blank: Rhowch enw olaf
+              too_long: The last name must be shorter than 500 characters
+            middle_names:
+              too_long: The middle name must be shorter than 500 characters
+            title:
+              too_long: The title must be shorter than 100 characters
         question/national_insurance_number:
           attributes:
             national_insurance_number:
@@ -145,6 +160,7 @@ cy:
               blank: Rhowch rif
               greater_than_or_equal_to: Rhaid i’r ateb fod yn fwy na 10 neu’n hafal i 0
               not_a_number: Rhaid i’r ateb fod yn rhif
+              too_long: The number must be shorter than 500 digits
         question/organisation_name:
           attributes:
             text:

config/locales/en.yml

@@ -95,15 +95,23 @@ en:
           attributes:
             address1:
               blank: Enter the first line of the address
+              too_long: The first line of the address must be shorter than 500 characters
+            address2:
+              too_long: The second line of the address must be shorter than 500 characters
             country:
               blank: Enter the country
+              too_long: The country must be shorter than 500 characters
+            county:
+              too_long: The county must be shorter than 500 characters
             postcode:
               blank: Enter the postcode
               invalid_postcode: Enter a full UK postcode
             street_address:
               blank: Enter the address
+              too_long: The address must be shorter than 5,000 characters
             town_or_city:
               blank: Enter the town or city
+              too_long: The town or city must be shorter than 500 characters
         question/date:
           attributes:
             date:
@@ -130,10 +138,17 @@ en:
           attributes:
             first_name:
               blank: Enter a first name
+              too_long: The first name must be shorter than 500 characters
             full_name:
               blank: Enter a name
+              too_long: The name must be shorter than 500 characters
             last_name:
               blank: Enter a last name
+              too_long: The last name must be shorter than 500 characters
+            middle_names:
+              too_long: The middle name must be shorter than 500 characters
+            title:
+              too_long: The title must be shorter than 100 characters
         question/national_insurance_number:
           attributes:
             national_insurance_number:
@@ -145,6 +160,7 @@ en:
               blank: Enter a number
               greater_than_or_equal_to: The answer must be greater than or equal to 0
               not_a_number: The answer must be a number
+              too_long: The number must be shorter than 500 digits
         question/organisation_name:
           attributes:
             text:

spec/models/question/address_spec.rb

@@ -139,6 +139,44 @@
         end
       end
     end
+
+    describe "length validations" do
+      before do
+        question.address1 = "a" * 499
+        question.address2 = "a" * 499
+        question.town_or_city = "a" * 499
+        question.county = "a" * 499
+        question.postcode = "LS11AF"
+      end
+
+      it "is valid when all fields have the maximum allowed length" do
+        expect(question).to be_valid
+      end
+
+      it "is invalid when the first line is too long" do
+        question.address1 = "a" * 500
+        expect(question).not_to be_valid
+        expect(question.errors[:address1]).to include(I18n.t("activemodel.errors.models.question/address.attributes.address1.too_long"))
+      end
+
+      it "is invalid when the second line is too long" do
+        question.address2 = "a" * 500
+        expect(question).not_to be_valid
+        expect(question.errors[:address2]).to include(I18n.t("activemodel.errors.models.question/address.attributes.address2.too_long"))
+      end
+
+      it "is invalid when the town or city is too long" do
+        question.town_or_city = "a" * 500
+        expect(question).not_to be_valid
+        expect(question.errors[:town_or_city]).to include(I18n.t("activemodel.errors.models.question/address.attributes.town_or_city.too_long"))
+      end
+
+      it "is invalid when the county is too long" do
+        question.county = "a" * 500
+        expect(question).not_to be_valid
+        expect(question.errors[:county]).to include(I18n.t("activemodel.errors.models.question/address.attributes.county.too_long"))
+      end
+    end
   end
 
   context "when the address is an international address" do
@@ -238,6 +276,29 @@
         end
       end
     end
+
+    describe "length validations" do
+      before do
+        question.street_address = "a" * 4999
+        question.country = "a" * 499
+      end
+
+      it "is valid when all fields have the maximum allowed length" do
+        expect(question).to be_valid
+      end
+
+      it "is invalid when the street address is too long" do
+        question.street_address = "a" * 5000
+        expect(question).not_to be_valid
+        expect(question.errors[:street_address]).to include(I18n.t("activemodel.errors.models.question/address.attributes.street_address.too_long"))
+      end
+
+      it "is invalid when the country is too long" do
+        question.country = "a" * 5000
+        expect(question).not_to be_valid
+        expect(question.errors[:country]).to include(I18n.t("activemodel.errors.models.question/address.attributes.country.too_long"))
+      end
+    end
   end
 
   describe "#is_international_address?" do

spec/models/question/email_spec.rb

@@ -10,52 +10,60 @@
 
   it_behaves_like "a question model"
 
-  context "when given an empty string or nil" do
-    it "returns invalid with blank email" do
-      expect(question).not_to be_valid
-      expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.blank"))
-    end
+  shared_examples "format validations" do
+    context "when given a valid email address" do
+      let(:email) { Faker::Internet.email }
 
-    it "returns invalid with empty string" do
-      question.email = ""
-      expect(question).not_to be_valid
-      expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.blank"))
-      expect(question.errors[:email]).not_to include(I18n.t("activemodel.errors.models.question/email.attributes.email.invalid_email"))
-    end
+      before do
+        question.email = email
+      end
 
-    it "shows as a blank string" do
-      expect(question.show_answer).to eq ""
+      it "is valid" do
+        expect(question).to be_valid
+      end
+
+      it "is included in show_answer" do
+        expect(question.show_answer).to eq email
+      end
+
+      it "returns the email address in show_answer_in_csv" do
+        expect(question.show_answer_in_csv).to eq(Hash[question_text, email])
+      end
     end
 
-    it "returns a hash with an blank value for show_answer_in_csv" do
-      expect(question.show_answer_in_csv).to eq(Hash[question_text, ""])
+    context "when given an invalid email address" do
+      it "is invalid" do
+        question.email = "no-tld@domain"
+        expect(question).not_to be_valid
+        expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.invalid_email"))
+      end
     end
   end
 
-  context "when given a string with an @ symbol in" do
-    before do
-      question.email = " @ "
-    end
+  context "when question is mandatory" do
+    context "when given an empty string or nil" do
+      it "returns invalid with blank email" do
+        expect(question).not_to be_valid
+        expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.blank"))
+      end
 
-    it "validates" do
-      expect(question).to be_valid
-    end
+      it "returns invalid with empty string" do
+        question.email = ""
+        expect(question).not_to be_valid
+        expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.blank"))
+        expect(question.errors[:email]).not_to include(I18n.t("activemodel.errors.models.question/email.attributes.email.invalid_email"))
+      end
 
-    it "is included in show_answer" do
-      expect(question.show_answer).to eq " @ "
-    end
+      it "shows as a blank string" do
+        expect(question.show_answer).to eq ""
+      end
 
-    it "returns the email address in show_answer_in_csv" do
-      expect(question.show_answer_in_csv).to eq(Hash[question_text, " @ "])
+      it "returns a hash with an blank value for show_answer_in_csv" do
+        expect(question.show_answer_in_csv).to eq(Hash[question_text, ""])
+      end
     end
-  end
 
-  context "when given a string without an @ symbol in" do
-    it "does not validate an address without an @" do
-      question.email = "not an email address"
-      expect(question).not_to be_valid
-      expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.invalid_email"))
-    end
+    include_examples "format validations"
   end
 
   context "when question is optional" do
@@ -81,30 +89,6 @@
       expect(question.show_answer_in_csv).to eq(Hash[question_text, ""])
     end
 
-    context "when given a string with an @ symbol in" do
-      before do
-        question.email = " @ "
-      end
-
-      it "validates" do
-        expect(question).to be_valid
-      end
-
-      it "is included in show_answer" do
-        expect(question.show_answer).to eq " @ "
-      end
-
-      it "returns the email address in show_answer_in_csv" do
-        expect(question.show_answer_in_csv).to eq(Hash[question_text, " @ "])
-      end
-    end
-
-    context "when given a string without an @ symbol in" do
-      it "does not validate an address without an @" do
-        question.email = "not an email address"
-        expect(question).not_to be_valid
-        expect(question.errors[:email]).to include(I18n.t("activemodel.errors.models.question/email.attributes.email.invalid_email"))
-      end
-    end
+    include_examples "format validations"
   end
 end