Skip to content

Commit

Permalink
Merge pull request #683 from coyote-team/jkva/682_relative_source_uri…
Browse files Browse the repository at this point in the history
…s_are_invalid

Ensure source_uri is a valid absolute http(s) uri
  • Loading branch information
jkva authored Apr 14, 2023
2 parents 7ed4a6f + c71036c commit 51bf0f2
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
13 changes: 13 additions & 0 deletions app/models/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class Resource < ApplicationRecord
validates :source_uri, uniqueness: {case_sensitive: false, scope: :organization_id}, if: :check_source_uri?
validates :name, presence: true

validate :source_uri_is_valid_uri

enum resource_type: Coyote::Resource::TYPES

audited
Expand Down Expand Up @@ -322,6 +324,17 @@ def viewable?

private

def source_uri_is_valid_uri
return unless source_uri.present?
uri = URI.parse(source_uri)

errors.add(:source_uri, "is not a valid URI") if
(uri.scheme.nil? && uri.host.nil?) ||
uri.host.present? && !/[^.\\]+/.match?(uri.host) ||
uri.path.empty? ||
uri.scheme.present? && ['http', 'https'].exclude?(uri.scheme)
end

def check_canonical_id?
Array(skip_unique_validations).include?(:canonical_id) && canonical_id_changed?
end
Expand Down
22 changes: 21 additions & 1 deletion spec/models/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,34 @@
expect(resource).to be_viewable
end

describe "without the presence of a source URI" do
describe "without the presence of a non-blank source URI" do
let(:resource) { build(:resource, source_uri: "") }

specify do
expect(resource).not_to be_viewable
end
end

describe "without the presence of a valid URI pattern source URI" do
def with_uri(uri)
build(:resource, source_uri: uri)
end

specify do
expect(with_uri("../foo.jpg")).not_to be_valid
expect(with_uri("//foo.jpg")).not_to be_valid
expect(with_uri("//../foo.jpg")).not_to be_valid
expect(with_uri("//../")).not_to be_valid
expect(with_uri("javascript://foo.jpg")).not_to be_valid
expect(with_uri("https://foo.jpg")).not_to be_valid

expect(with_uri("http://example.org/foo.jpg")).to be_valid
expect(with_uri("http://example.org/images/misc/../foo.jpg")).to be_valid
expect(with_uri("https://example.org/foo.jpg")).to be_valid
expect(with_uri("//example.org/foo.jpg")).to be_valid
end
end

describe "#related_resources" do
# this test requires the database as rspec stubs don't completely replicate has_many behaviors
let!(:resource_link) do
Expand Down

0 comments on commit 51bf0f2

Please sign in to comment.