Repository navigation
Fix #2951: drop the regex route constraint from the default Swagger route - #4130
martincostello merged 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4130 +/- ##
==========================================
+ Coverage 95.24% 95.32% +0.08%
==========================================
Files 111 111
Lines 4142 4149 +7
Branches 848 850 +2
==========================================
+ Hits 3945 3955 +10
+ Misses 197 194 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…fault Swagger route
SwaggerMiddleware resolves a TemplateBinder for SwaggerOptions.RouteTemplate in
its constructor, and the default template constrained {extension} with an inline
regex. An application built with CreateSlimBuilder or AddRoutingCore does not
register that constraint, so the middleware threw at startup.
The default template no longer uses a constraint, and the middleware applies the
same json, yaml and yml allow-list in code when the extension parameter carries
no constraint of its own. Custom templates that bring their own constraints are
unaffected.
|
Both suggestions are applied: the test is renamed to On the |
18fdfb2 to
1af6de0
Compare
|
Thanks for your contribution @RaphaelFakhri - the changes from this pull request have been published as part of version 10.3.0 📦, which is now available from NuGet.org 🚀 |
Fixes #2951. This takes up the offer above: "We'd be happy to take a PR that changes the route template ... that doesn't need a Regex."
It still reproduces on master.
SwaggerOptions.DefaultRouteTemplateis/swagger/{documentName}/swagger.{extension:regex(^(json|ya?ml)$)}, andSwaggerMiddleware's constructor eagerly resolves aTemplateBinderfor it. An application built withCreateSlimBuilderorAddRoutingCoredoes not register the regex constraint, so a standalone app against master throws at startup with the exception in the report, fromSwaggerMiddleware..ctor.The default template drops the constraint, and the middleware applies the equivalent allow-list in code, but only when the
extensionparameter has no inline constraint of its own. A custom template that brings its own constraints keeps going through theTemplateBinderpath unchanged. The allow-list isjson,yaml,yml, compared withOrdinalIgnoreCase, matching the case-insensitivity of the regex it replaces.MapSwagger's default parameter value changes with it, soPublicAPI.Unshipped.txtcarries the*REMOVED*and replacement pair the analyzers require.This also deletes the repo's own workaround:
test/WebSites/WebApi.Aot/Program.cswas registeringSetParameterPolicy<RegexInlineRouteConstraint>to get around exactly this, so the in-repo AoT slim-builder site now exercises the fix rather than papering over it.SwaggerMiddlewareRouteTemplateTestscovers building the pipeline underAddRoutingCore()with no regex constraint registered, plus eight route cases:.json,.yaml,.yml,.JSONand.YAMLserve 200, and.txt,.htmland.jsonxfall through to a 404. All nine fail against master's source with the sameRouteCreationExceptionfrom the report. With the change,Swashbuckle.AspNetCore.SwaggerGen.Testis 769 passed, 0 failed on net8.0. I also confirmed the standalone slim-builder app end to end: it crashed before, and after the change it starts, serves a valid document atswagger.json, servesswagger.yamlastext/yaml, and 404sswagger.txt.Two things I did not change, deliberately.
.YAMLreturns 200 with JSON content, because the old regex was case-insensitive while the json/yaml switch downstream is case-sensitive; that is master's existing behaviour and this PR is not the place to change it. And a user with a custom, unconstrained{extension}template, saydocs/{documentName}/spec.{extension}, previously got JSON for any extension and now sees anything outside the allow-list fall through to the next middleware. That is the same tightening #2418 intended for the default template, but it is a behaviour change and worth your call.