Skip to content

feat(#1726): add literal colon support for gin, echo, fiber#1879

Merged
mromaszewicz merged 7 commits into
oapi-codegen:mainfrom
cosban:literal-colon-support
Jul 16, 2026
Merged

feat(#1726): add literal colon support for gin, echo, fiber#1879
mromaszewicz merged 7 commits into
oapi-codegen:mainfrom
cosban:literal-colon-support

Conversation

@cosban

@cosban cosban commented Jan 8, 2025

Copy link
Copy Markdown
Contributor

Closes #1726

This MR supersedes #1815

@cosban
cosban requested a review from a team as a code owner January 8, 2025 18:39
# Conflicts:
#	internal/test/parameters/path_edge_cases/path_edge_cases.gen.go
Builds on the original fix by escaping literal colons in the Fiber URI
converter as well, and corrects the escaping to match the register
templates' current behaviour.

The register templates render the converted path through toGoString
(strconv.Quote), so the converter must emit a single backslash (`\:`); the
quoting turns it into `\\:` in the generated source, which is the value the
router sees at runtime. The prior double-backslash escape predated the
templates' switch to toGoString and would emit `\\:` to the router, which
Echo rejects (Not Found) and Gin panics on. A shared escapeLiteralPathColons
helper now feeds SwaggerUriToEchoUri, SwaggerUriToGinUri, and
SwaggerUriToFiberUri.

The Fiber v3 handler template interpolated the path raw inside quotes; a
backslash would produce invalid Go, so it now uses toGoString like every
other server template. Iris and the `{}`-style routers (Chi, Gorilla,
net/http) are unaffected: Iris already routes literal colons correctly and
escaping breaks it, and the others do not treat `:` specially.

Adds a paths/ regression fixture that registers two colon paths sharing a
prefix (/pets:validate, /pets:generate) on Echo, Gin, Fiber v2, and Fiber v3
and asserts each dispatches to its own handler. Moves the existing
path_edge_cases fixture under paths/ and regenerates it with the escaped
route.
@mromaszewicz

Copy link
Copy Markdown
Member

@greptileai

@mromaszewicz mromaszewicz added the bug Something isn't working label Jul 16, 2026
@mromaszewicz mromaszewicz changed the title feat(#1726): add literal colon support for gin and echo feat(#1726): add literal colon support for gin, echo, fiber Jul 16, 2026
@mromaszewicz

Copy link
Copy Markdown
Member

I've basically rewritten this PR as an additional commit. Thanks for submitting this ages ago, and sorry for such a long delay.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes issue #1726 by teaching the code generator to escape literal : characters in OpenAPI paths for the three routers that use : as a path-parameter delimiter — Echo, Gin, and Fiber v2/v3. Without this, two paths sharing a prefix but ending in different literal-colon segments (e.g. /pets:validate and /pets:generate) would collide on the same :param slot and only the first registered route would ever match.

  • A new escapeLiteralPathColons helper is added to utils.go and called at the start of SwaggerUriToEchoUri, SwaggerUriToFiberUri, and SwaggerUriToGinUri, replacing every literal : with \\: before path-parameter substitution introduces its own :param delimiters. The fiber-v3 template is also fixed to pass the path through toGoString (like the other backends already did), so the backslash is correctly double-escaped in the emitted Go source.
  • New per-backend generated fixtures and an integration test in internal/test/paths/ confirm that each affected router dispatches the two colon-suffixed routes to their respective handlers independently. The existing path_edge_cases directory is relocated from parameters/ into the new paths/ top-level category.

Confidence Score: 4/5

The core fix is well-scoped and correct — existing paths without literal colons are unaffected, and paths with colons were already broken before this change, so there is no regression risk.

The escapeLiteralPathColons logic is order-safe and well-tested. The fiber-v3 template correction addresses a pre-existing gap. All three affected router converters are updated consistently, and Iris is correctly left alone since it uses {} not : for path parameters. The only concern is the test directory reorganization, which creates an unofficial top-level category.

The internal/test/paths/ directory and its relocation of parameters/path_edge_cases/ deserve a second look to confirm the team is comfortable with the new category name.

Important Files Changed

Filename Overview
pkg/codegen/utils.go Adds escapeLiteralPathColons helper and calls it in SwaggerUriToEchoUri, SwaggerUriToFiberUri, and SwaggerUriToGinUri before path-param substitution — logic is correct and order-safe. Iris uses {} not : for params, so its converter correctly needs no change.
pkg/codegen/utils_test.go Adds literal-colon test assertions for Echo, Gin, and Fiber URI converters, covering both standalone colons and colons following a path parameter.
pkg/codegen/templates/fiber-v3/fiber-handler.tmpl Switches from embedding the raw path string directly in a Go string literal to passing it through toGoString (strconv.Quote), fixing a pre-existing gap where backslash escapes from swaggerUriToFiberUri would not have been properly quoted in the generated source.
internal/test/paths/colon_paths_test.go Adds per-backend integration tests (Echo, Gin, Fiber v2, Fiber v3) verifying that /pets:validate and /pets:generate dispatch to distinct handlers. Directory placement creates a new top-level paths/ category not in the established structure.
internal/test/paths/path_edge_cases/path_edge_cases.gen.go Package renamed from parameterspathedgecases to pathedgecases (directory moved from parameters/). Generated router registration for /pets:validate now correctly emits \\: escape in Go source.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
internal/test/paths/doc.go:1-10
**New `paths/` category not in established test structure**

The repository organises `internal/test/` into a fixed set of top-level categories (e.g. `parameters/`, `servers/`, `schemas/`, …); `paths/` does not appear in that list. The new colon-routing tests are about server route registration, so `internal/test/servers/` is the most natural home. A sibling directory there (e.g. `servers/colon_paths/`) with per-backend config files would fit the established structure without requiring a new top-level category.

The same concern applies to the relocation of `parameters/path_edge_cases/` into this new directory. Moving an already-established leaf out of `parameters/` into an unofficial category quietly breaks the taxonomy that the rules are meant to preserve.

Reviews (1): Last reviewed commit: "Complete literal-colon path support for ..." | Re-trigger Greptile

Comment thread internal/test/paths/doc.go
@mromaszewicz
mromaszewicz enabled auto-merge (squash) July 16, 2026 18:42
@mromaszewicz
mromaszewicz merged commit ce2e068 into oapi-codegen:main Jul 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generated client is not working when there are more than one API's with the colon in the API path

2 participants