From 930b45cd103f36e1cd9d44ba3b5551391a79055a Mon Sep 17 00:00:00 2001
From: Voon Siong Wong <voon.wong@smartbear.com>
Date: Wed, 6 Dec 2023 13:45:38 +1100
Subject: [PATCH] fix: raise 404 on paths with missing path segments (#648)

PACT-13
---
 lib/pact_broker/app.rb                        |  2 +-
 .../pact_broker/invalid_uri_protection.rb     | 22 ++++++++++++++++---
 .../invalid_uri_protection_spec.rb            | 12 +++++++++-
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/lib/pact_broker/app.rb b/lib/pact_broker/app.rb
index d44c893b1..c67d9948e 100644
--- a/lib/pact_broker/app.rb
+++ b/lib/pact_broker/app.rb
@@ -181,6 +181,7 @@ def configure_middleware
       @app_builder.use PactBroker::Api::Middleware::HttpDebugLogs if configuration.http_debug_logging_enabled
       configure_basic_auth
       configure_rack_protection
+      @app_builder.use Rack::PactBroker::ApplicationContext, application_context
       @app_builder.use Rack::PactBroker::InvalidUriProtection
       @app_builder.use Rack::PactBroker::ResetThreadData
       @app_builder.use Rack::PactBroker::AddPactBrokerVersionHeader
@@ -191,7 +192,6 @@ def configure_middleware
       @app_builder.use Rack::PactBroker::ConvertFileExtensionToAcceptHeader
       # Rack::PactBroker::SetBaseUrl needs to be before the Rack::PactBroker::HalBrowserRedirect
       @app_builder.use Rack::PactBroker::SetBaseUrl, configuration.base_urls
-      @app_builder.use Rack::PactBroker::ApplicationContext, application_context
 
       if configuration.use_hal_browser
         logger.info "Mounting HAL browser"
diff --git a/lib/rack/pact_broker/invalid_uri_protection.rb b/lib/rack/pact_broker/invalid_uri_protection.rb
index 536e4fb98..850c6aa7e 100644
--- a/lib/rack/pact_broker/invalid_uri_protection.rb
+++ b/lib/rack/pact_broker/invalid_uri_protection.rb
@@ -12,6 +12,8 @@ module PactBroker
     class InvalidUriProtection
       include ::PactBroker::Messages
 
+      CONSECUTIVE_SLASH = /\/{2,}/
+
       def initialize app
         @app = app
       end
@@ -19,12 +21,12 @@ def initialize app
       def call env
         if (uri = valid_uri?(env))
           if (error_message = validate(uri))
-            [422, {"Content-Type" => "text/plain"}, [error_message]]
+            [422, headers, [body(env, error_message, "Unprocessable", "invalid-request-parameter-value", 422)]]
           else
             app.call(env)
           end
         else
-          [404, {}, []]
+          [404, headers, [body(env, "Empty path component found", "Not Found", "not-found", 404)]]
         end
       end
 
@@ -34,7 +36,9 @@ def call env
 
       def valid_uri? env
         begin
-          parse(::Rack::Request.new(env).url)
+          uri = parse(::Rack::Request.new(env).url)
+          return nil if CONSECUTIVE_SLASH.match(uri.path)
+          uri
         rescue URI::InvalidURIError, ArgumentError
           nil
         end
@@ -52,6 +56,18 @@ def validate(uri)
           message("errors.tab_in_url_path")
         end
       end
+
+      def headers
+        {"Content-Type" => "application/problem+json"}
+      end
+
+      def body(env, detail, title, type, status)
+        env["pactbroker.application_context"]
+          .decorator_configuration
+          .class_for(:custom_error_problem_json_decorator)
+          .new(detail: detail, title: title, type: type, status: status)
+          .to_json(user_options: { base_url: env["pactbroker.base_url"] })
+      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 73e8e3004..9e2617ade 100644
--- a/spec/lib/rack/pact_broker/invalid_uri_protection_spec.rb
+++ b/spec/lib/rack/pact_broker/invalid_uri_protection_spec.rb
@@ -1,4 +1,6 @@
 require "rack/pact_broker/invalid_uri_protection"
+require "pact_broker/application_context"
+require "pact_broker/api/decorators/custom_error_problem_json_decorator"
 
 module Rack
   module PactBroker
@@ -7,7 +9,7 @@ module PactBroker
       let(:app) { InvalidUriProtection.new(target_app) }
       let(:path) { "/foo" }
 
-      subject { get(path) }
+      subject { get(path, {}, {"pactbroker.application_context" => ::PactBroker::ApplicationContext.default_application_context} ) }
 
       context "with a URI that the Ruby default URI library cannot parse" do
         let(:path) { "/badpath" }
@@ -27,6 +29,14 @@ module PactBroker
           expect(subject.status).to eq 200
         end
 
+        context "when the path contains missing path segments" do
+          let(:path) { "/foo//bar" }
+
+          it "returns a 404" do
+            expect(subject.status).to eq 404
+          end
+        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) { "/foo%0A/bar" }