Allow multi-segment runtime REST/GraphQL paths (e.g. /api/v2)#3684
Allow multi-segment runtime REST/GraphQL paths (e.g. /api/v2)#3684Copilot wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes runtime path validation so multi-segment REST/GraphQL paths (e.g. /api/v2) are accepted while still rejecting empty segments and reserved characters, aligning CLI behavior (dab configure, dab init) with runtime routing capabilities.
Changes:
- Added segment-aware runtime path validation (
DoesUriPathContainReservedChars) and wired it intoTryValidateUriComponent. - Updated CLI validation (
IsURIComponentValid) to use the same segment-aware check for parity. - Updated/added tests to cover multi-segment success cases and empty-segment/trailing-slash failure cases.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Core/Configurations/RuntimeConfigValidatorUtil.cs | Implements segment-aware runtime path validation and updates shared runtime validation to allow / as a segment separator. |
| src/Cli/Utils.cs | Aligns CLI URI component validation with the new segment-aware runtime validator. |
| src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs | Adds positive and negative unit test cases for multi-segment runtime paths and empty-segment/trailing-slash scenarios. |
| src/Cli.Tests/EndToEndTests.cs | Updates E2E CLI tests to validate multi-segment paths succeed and empty segments fail. |
| src/Cli.Tests/ConfigureOptionsTests.cs | Adds configure-option test coverage for multi-segment REST paths. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
RubenCerna2079
left a comment
There was a problem hiding this comment.
LGTM! Approved assuming comments will be addressed.
| [DataRow("/updatedPath", true, DisplayName = "Success in updated GraphQL Path to /updatedPath.")] | ||
| [DataRow("/updated-Path", true, DisplayName = "Success in updated GraphQL Path to /updated-Path.")] | ||
| [DataRow("/updated_Path", true, DisplayName = "Success in updated GraphQL Path to /updated_Path.")] | ||
| [DataRow("/api/v2", true, DisplayName = "Success in updated GraphQL Path to multi-segment path /api/v2.")] |
There was a problem hiding this comment.
nit: Think it would be better to use a different name than api since that is the default we use for REST paths. I think it is better to use graphql
| [DataRow("/updated.Path", false, DisplayName = "Failure due to reserved char '.'.")] | ||
| [DataRow("/updated@Path", false, DisplayName = "Failure due reserved chars '@'.")] | ||
| [DataRow("/updated/Path", false, DisplayName = "Failure due reserved chars '/'.")] | ||
| [DataRow("/api//v2", false, DisplayName = "Failure due to empty path segment.")] |
There was a problem hiding this comment.
Same as previous comment
| DisplayName = "REST path containing only a forward slash.")] | ||
| [DataRow("/api/v2", null, ApiType.REST, false, | ||
| DisplayName = "REST path containing multiple segments.")] | ||
| [DataRow("/api/v2", null, ApiType.GraphQL, false, |
There was a problem hiding this comment.
Same as previous comment
Why make this change?
dab configure --runtime.rest.path "/api/v2"(and the GraphQL equivalent) failed withcontains one or more reserved characters. The path validator stripped the leading/and then rejected the whole remainder using a regex where/is itself reserved, so any multi-segment path was rejected even though routing supports them.What is this change?
RuntimeConfigValidatorUtil.DoesUriPathContainReservedChars, which splits the path (after the leading/) into/-separated segments and validates each one./is now permitted as a separator; empty segments (leading/consecutive/trailing slashes) and reserved characters are still rejected. The root path/stays valid.TryValidateUriComponentnow uses the helper, so REST, GraphQL, MCP, and base-route all accept multi-segment paths while preserving existing error messages for genuinely invalid input.IsURIComponentValid(used bydab init) now uses the same segment-aware check./api/v2cases and empty-segment/trailing-slash failure cases across configure, init, and config-validation tests.How was this tested?
Sample Request(s)