Skip to content

Commit

Permalink
fix: raise 404 on paths with missing path segments (#648)
Browse files Browse the repository at this point in the history
PACT-13
  • Loading branch information
vwong committed Dec 6, 2023
1 parent 9a63732 commit 930b45c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/pact_broker/app.rb
Expand Up @@ -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
Expand All @@ -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"
Expand Down
22 changes: 19 additions & 3 deletions lib/rack/pact_broker/invalid_uri_protection.rb
Expand Up @@ -12,19 +12,21 @@ module PactBroker
class InvalidUriProtection
include ::PactBroker::Messages

CONSECUTIVE_SLASH = /\/{2,}/

def initialize app
@app = app
end

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

Expand All @@ -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
Expand All @@ -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
12 changes: 11 additions & 1 deletion 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
Expand All @@ -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" }
Expand All @@ -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" }

Expand Down

0 comments on commit 930b45c

Please sign in to comment.