Skip to content

fix: StripRequestPath returns "/" when base path fully consumes request path#265

Merged
daveshanley merged 1 commit intopb33f:mainfrom
jspdown:fix/strip-request-path-empty-result
Apr 26, 2026
Merged

fix: StripRequestPath returns "/" when base path fully consumes request path#265
daveshanley merged 1 commit intopb33f:mainfrom
jspdown:fix/strip-request-path-empty-result

Conversation

@jspdown
Copy link
Copy Markdown
Contributor

@jspdown jspdown commented Apr 22, 2026

Summary

paths.StripRequestPath returns "" (instead of "/") when the base path derived from servers[0].url exactly consumes the request path. The subsequent segment-based comparison inside FindPath's slow path then splits "" into an empty slice, while the spec's / route splits into [""] (one empty segment), so comparePaths short-circuits on the length mismatch and the request is reported as "Path not found".

Example that hits the bug:

servers:
 - url: https://things.com/v1beta/weather
paths:
 /:
   post:
     operationId: rootOp

A POST https://things.com/v1beta/weather should match the / operation but today returns a validation error.

Fix

Drop the len(stripped) > 0 && guard so an emptied stripped path is re-prefixed to "/", which matches a spec declaring paths: { "/": ... }.

…st path

When a server URL contains a path that is identical to the incoming request
path (e.g. servers[0].url = "https://things.com/v1beta/openai/chat/completions"
and request path "/v1beta/openai/chat/completions"), stripBaseFromPath leaves
an empty string. The guard prevented the leading "/" from being restored, so
downstream segment-based comparison in FindPath's slow path could not match
the spec's "/" route.

Remove the len(stripped) > 0 guard so an emptied stripped path always becomes
"/", which correctly maps to a spec declaring paths: { "/": ... }.

Adds a table-driven TestStripRequestPath covering the bug case plus
regression cases, and TestNewValidator_FindPath_BasePathFullyConsumesRequestPath
exercising the slow path end-to-end.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.89%. Comparing base (1d13412) to head (b4c9610).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
- Coverage   97.97%   97.89%   -0.09%     
==========================================
  Files          64       64              
  Lines        6627     8119    +1492     
==========================================
+ Hits         6493     7948    +1455     
- Misses        133      170      +37     
  Partials        1        1              
Flag Coverage Δ
unittests 97.89% <100.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jspdown
Copy link
Copy Markdown
Contributor Author

jspdown commented Apr 23, 2026

I've run the coverage check locally and nothing wrong is reported. This looks like a bug on the CI.

Copy link
Copy Markdown
Member

@daveshanley daveshanley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@daveshanley daveshanley merged commit 00ad42a into pb33f:main Apr 26, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants