Merge pull request #759 from wurmlab/tt/escaping

Validate user inputs ore thoroughly

Author: tadast

lib/sequenceserver/api_errors.rb

@@ -52,6 +52,30 @@ def message
     end
   end
 
+  # InvalidParameterError is a more generic error class that can be
+  # raised when the frontend sends a request with an invalid parameter
+  class InvalidParameterError < ValidationError
+    attr_reader :more_info
+
+    def initialize(more_info)
+      super
+      @more_info = more_info
+    end
+
+    def http_status
+      422
+    end
+
+    def title
+      'Invalid parameter'
+    end
+
+    def message
+      "The action you're trying to perform is not possible because \
+        one of the provided parameters is invalid."
+    end
+  end
+
   # API errors have an http status, title, message, and additional information
   # like stacktrace or information from program output.
   APIError = Class.new(Error)

lib/sequenceserver/blast.rb

@@ -1,3 +1,9 @@
 require_relative 'blast/job'
 require_relative 'blast/report'
 require_relative 'blast/constants'
+
+module SequenceServer
+  module BLAST
+    VALID_SEQUENCE_ID = /\A[a-zA-Z0-9\-_.:*#|\[\]]+\z/
+  end
+end

lib/sequenceserver/database.rb

@@ -37,8 +37,19 @@ def initialize(*args)
     alias path name
 
     def retrieve(accession, coords = nil)
+      fail(
+        InvalidSequenceIdError,
+        "Invalid sequence id: #{accession}"
+      ) unless accession =~ SequenceServer::BLAST::VALID_SEQUENCE_ID
+
       cmd = "blastdbcmd -db #{name} -entry '#{accession}'"
+
       if coords
+        fail(
+          InvalidParameterError,
+          "Invalid range coordinates: #{coords}"
+        ) unless coords =~ /[0-9]+-[0-9]*/
+
         cmd << " -range #{coords}"
       end
       out, = sys(cmd, path: config[:bin])
@@ -52,6 +63,8 @@ def retrieve(accession, coords = nil)
     # Returns true if the database contains the given sequence id.
     # Returns false otherwise.
     def include?(id)
+      fail ArgumentError, "Invalid sequence id: #{id}" unless id =~ SequenceServer::BLAST::VALID_SEQUENCE_ID
+
       cmd = "blastdbcmd -entry '#{id}' -db #{name}"
       sys(cmd, path: config[:bin]) rescue false
     end

lib/sequenceserver/sequence.rb

@@ -232,9 +232,8 @@ def validate
           )
         end
 
-        valid_id_regex = /\A[a-zA-Z0-9-_.:*#|\[\]]+\z/
         invalid_sequence_ids = sequence_ids.reject do |id|
-          id =~ valid_id_regex
+          id =~ SequenceServer::BLAST::VALID_SEQUENCE_ID
         end
 
         unless invalid_sequence_ids.empty?

spec/database_spec.rb

@@ -27,8 +27,53 @@ module SequenceServer
       Database[id].first
     end
 
-    it 'knows if a given accession is in the database or not' do
-      solenopsis_protein_database.include?('SI2.2.0_06267').should be_truthy
+    describe '#include?' do
+      it 'knows if a given accession is present in the database' do
+        expect(solenopsis_protein_database).to include('SI2.2.0_06267')
+      end
+
+      it 'knows if a given accession is absent in the database' do
+        expect(solenopsis_protein_database).not_to include('LOL.2.0_404')
+      end
+
+      it 'validates the id' do
+        expect do
+          solenopsis_protein_database.include?("';hi")
+        end.to raise_error(ArgumentError, "Invalid sequence id: ';hi")
+      end
+    end
+
+    describe '.retrieve' do
+      it "retrieves the sequence for a given accession" do
+        sequence = Database.retrieve("SI2.2.0_06267")
+        expect(sequence).to include('SI2.2.0_06267')
+        expect(sequence).to include('MNTLWLSLWDYPGKL') # start of fasta sequence
+      end
+
+      it "retrieves an open sequence range for a given accession" do
+        sequence = Database.retrieve("SI2.2.0_06267:10-")
+        expect(sequence).to include('SI2.2.0_06267')
+        expect(sequence).not_to include('MNTLWLSLWD') # excludes first 10 chars
+        expect(sequence.lines[1]).to start_with('DYPGKLP') # start at an offset of 10
+      end
+
+      it "retrieves a closed sequence range for a given accession" do
+        sequence = Database.retrieve("SI2.2.0_06267:1-10")
+        expect(sequence).to include('SI2.2.0_06267')
+        expect(sequence.lines.last.size).to eq(10)
+      end
+
+      it "validates the sequence id" do
+        expect do
+          Database.retrieve("';hi")
+        end.to raise_error(SequenceServer::InvalidSequenceIdError)
+      end
+
+      it "validates the range" do
+        expect do
+          Database.retrieve("SI2.2.0_06267:';hi")
+        end.to raise_error(SequenceServer::InvalidParameterError)
+      end
     end
   end
 end