From db9f7f4d6834f922021c7b80e5df570e40c827bd Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 3 Jun 2020 17:19:28 +1000 Subject: [PATCH] fix: return a 422 if the URL path contains a new line or tab --- .../pact_broker/invalid_uri_protection.rb | 27 +++++++++++++++---- .../invalid_uri_protection_spec.rb | 24 ++++++++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/lib/rack/pact_broker/invalid_uri_protection.rb b/lib/rack/pact_broker/invalid_uri_protection.rb index 02e44849c..c9fff3454 100644 --- a/lib/rack/pact_broker/invalid_uri_protection.rb +++ b/lib/rack/pact_broker/invalid_uri_protection.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'uri' # This class is for https://github.com/pact-foundation/pact_broker/issues/101 @@ -6,31 +8,46 @@ module Rack module PactBroker class InvalidUriProtection - def initialize app @app = app end def call env - if valid_uri? env - @app.call(env) + if (uri = valid_uri?(env)) + if (error_message = validate(uri)) + [422, {'Content-Type' => 'text/plain'}, [error_message]] + else + app.call(env) + end else [404, {}, []] end end + private + + attr_reader :app + def valid_uri? env begin parse(::Rack::Request.new(env).url) - true rescue URI::InvalidURIError, ArgumentError - false + nil end end def parse uri URI.parse(uri) end + + def validate(uri) + decoded_path = URI.decode(uri.path) + if decoded_path.include?("\n") + 'URL path cannot contain a new line character.' + elsif decoded_path.include?("\t") + 'URL path cannot contain a tab character.' + end + end end end end diff --git a/spec/lib/rack/pact_broker/invalid_uri_protection_spec.rb b/spec/lib/rack/pact_broker/invalid_uri_protection_spec.rb index 0dfdedb61..5296dc5cf 100644 --- a/spec/lib/rack/pact_broker/invalid_uri_protection_spec.rb +++ b/spec/lib/rack/pact_broker/invalid_uri_protection_spec.rb @@ -3,12 +3,14 @@ module Rack module PactBroker describe InvalidUriProtection do + let(:target_app) { ->(env){ [200, {}, []] } } + let(:app) { InvalidUriProtection.new(target_app) } + let(:path) { URI.encode("/foo") } - let(:app) { InvalidUriProtection.new(->(env){ [200,{},[]] }) } - - subject { get "/badpath"; last_response } + subject { get(path) } context "with a URI that the Ruby default URI library cannot parse" do + let(:path) { "/badpath" } before do # Can't use or stub URI.parse because rack test uses it to execute the actual test @@ -24,6 +26,22 @@ module PactBroker it "passes the request to the underlying app" do expect(subject.status).to eq 200 end + + context "when the URI contains a new line because someone forgot to strip the result of `git rev-parse HEAD`, and I have totally never done this before myself" do + let(:path) { URI.encode("/foo\n/bar") } + + it "returns a 422" do + expect(subject.status).to eq 422 + end + end + + context "when the URI contains a tab because sooner or later someone is eventually going to do this" do + let(:path) { URI.encode("/foo\t/bar") } + + it "returns a 422" do + expect(subject.status).to eq 422 + end + end end end end