Swashbuckle.WebApi icon indicating copy to clipboard operation
Swashbuckle.WebApi copied to clipboard

Stack Overflow when action has recursive url parameter

Open henkosch opened this issue 9 years ago • 7 comments

Swashbuckle.Swagger.Filters.HandleFromUriParams.ExtractAndAddQueryParams goes into an infinite recursive loop when you define an action with a url parameter that has circular references in it. IIS dies instantly with Stack Overflow Exception when Swashbuckle tries to generate the schema.

The problem only occurs when you explicitly set the model to be an url parameter with [FromUri] attribute. It can detect circular references correctly in body parameters.

Here is a small test case:

public class CategoryModel
{
    public int Id { get; set; }
    public CategoryModel Parent { get; set; }
}

public class CategoryController : ApiController
{
    [HttpGet]
    [Route("api/category/level")]
    public int GetLevel([FromUri]CategoryModel model)
    {
        return 1;
    }
}

And the stack trace:

...
Swashbuckle.Core.dll!Swashbuckle.Swagger.Filters.HandleFromUriParams.ExtractAndAddQueryParams(Swashbuckle.Swagger.Schema sourceSchema, string sourceQualifier, bool sourceRequired, Swashbuckle.Swagger.SchemaRegistry schemaRegistry, System.Collections.Generic.IList<Swashbuckle.Swagger.Parameter> operationParams) Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.Filters.HandleFromUriParams.ExtractAndAddQueryParams(Swashbuckle.Swagger.Schema sourceSchema, string sourceQualifier, bool sourceRequired, Swashbuckle.Swagger.SchemaRegistry schemaRegistry, System.Collections.Generic.IList<Swashbuckle.Swagger.Parameter> operationParams) Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.Filters.HandleFromUriParams.ExtractAndAddQueryParams(Swashbuckle.Swagger.Schema sourceSchema, string sourceQualifier, bool sourceRequired, Swashbuckle.Swagger.SchemaRegistry schemaRegistry, System.Collections.Generic.IList<Swashbuckle.Swagger.Parameter> operationParams) Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.Filters.HandleFromUriParams.ExtractAndAddQueryParams(Swashbuckle.Swagger.Schema sourceSchema, string sourceQualifier, bool sourceRequired, Swashbuckle.Swagger.SchemaRegistry schemaRegistry, System.Collections.Generic.IList<Swashbuckle.Swagger.Parameter> operationParams) Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.Filters.HandleFromUriParams.HandleFromUriObjectParams(Swashbuckle.Swagger.Operation operation, Swashbuckle.Swagger.SchemaRegistry schemaRegistry, System.Web.Http.Description.ApiDescription apiDescription)   Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.Filters.HandleFromUriParams.Apply(Swashbuckle.Swagger.Operation operation, Swashbuckle.Swagger.SchemaRegistry schemaRegistry, System.Web.Http.Description.ApiDescription apiDescription)   Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.SwaggerGenerator.CreateOperation(System.Web.Http.Description.ApiDescription apiDescription, Swashbuckle.Swagger.SchemaRegistry schemaRegistry) Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.SwaggerGenerator.CreatePathItem(System.Collections.Generic.IEnumerable<System.Web.Http.Description.ApiDescription> apiDescriptions, Swashbuckle.Swagger.SchemaRegistry schemaRegistry) Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.SwaggerGenerator.GetSwagger.AnonymousMethod__4(System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription> group) Unknown
System.Core.dll!System.Linq.Enumerable.ToDictionary<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>,string,Swashbuckle.Swagger.PathItem>(System.Collections.Generic.IEnumerable<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>> source, System.Func<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>,string> keySelector, System.Func<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>,Swashbuckle.Swagger.PathItem> elementSelector, System.Collections.Generic.IEqualityComparer<string> comparer) Unknown
System.Core.dll!System.Linq.Enumerable.ToDictionary<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>,string,Swashbuckle.Swagger.PathItem>(System.Collections.Generic.IEnumerable<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>> source, System.Func<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>,string> keySelector, System.Func<System.Linq.IGrouping<string,System.Web.Http.Description.ApiDescription>,Swashbuckle.Swagger.PathItem> elementSelector)    Unknown
Swashbuckle.Core.dll!Swashbuckle.Swagger.SwaggerGenerator.GetSwagger(string rootUrl, string apiVersion) Unknown

henkosch avatar Apr 03 '15 12:04 henkosch

Good catch! Although, a self referencing type doesn't seem like a great idea for a FromUri binding because it essentially represents an infinite set of individual query params.

That said though, SB should handle it correctly. Do you have any thoughts on the correct behavior here - it's a little different to the body parameter scenario because in this case the complex type needs to be flattened into a finite set of individual parameter descriptions.

The only thing I can think of is to only support an arbitrary nesting level when flattening out an object graph. This would produce unexpected behavior if the level of nesting in your FromUri object happens to exceed the chosen arbitrary value. And, as with all arbitrary values, what should it be?

Would be interested to get your thoughts?

domaindrivendev avatar Apr 07 '15 20:04 domaindrivendev

I can't really think of a scenario where such url parameters would be userful. In fact I accidentally found this bug thanks to a controller action that was created by our team to temporarly test a custom json model binder. I just banged my head into the desk until I figured out that it was this strange test model that caused it.

In my opinion you should do the same as you did with recursive models in the request body. Just detect the recursion and switch the self referencing parameter to string or remove it completely.

henkosch avatar Apr 08 '15 19:04 henkosch

Because this is a serious edge-case I'm going to punt on it. In the next version of AspNet (vNext), expanding out these params will be fully handled in the framework and so won't need to be done by SB. As a result, this should be resolved in the next major version of SB because it targets vNext.

domaindrivendev avatar May 30 '15 18:05 domaindrivendev

Hi all, Unfortunately I have this problem in my current project :( My project is running on .NET framework 4.5.2, and using the latest Swashbackle 5. But according to the discussion above it seems this problem is not resolved. Have anyone any idea how to approach it or resolve it?

theCuriousOne avatar Sep 06 '16 07:09 theCuriousOne

This happened to me when one of my actions was taking System.Web.Http.OData.Query.ODataQueryOption as a parameter and I was setting readonly = null on all properties. My workaround was to perform the readonly = null logic in an IDocumentFilter instead of an IOperationFilter. This allowed the buggy HandleFromUriParams OperationFilter to be bypassed because it did its work when readonly was set to null.

ivanmartinvalle avatar Nov 15 '17 05:11 ivanmartinvalle

@henkosch I just ran into this issue too although the recursion was due to using Entity Framework models in the parameters and as return types. Therefore, if you want to use any of those models in your parameters, Swagger will break. To me, this seems like a somewhat common use case as EF is used quite frequently.

sean09lee avatar Sep 11 '19 23:09 sean09lee

I have the same issue, but I am quite sure, that the reason is just the circle reference itsself (not FromUri). A 'small' workaround is to map the 'problem' classes with: config.EnableSwagger("docs/{apiVersion}/swagger", c => { c.MapType<ProblemCircleClass>(() => new Schema { type = "ProblemCircleClass", format = "ProblemCircleClass" }); ... }

and repeat this line for every class with a circle in it.

(Note: this obviously won't serialize the whole class, but you will get a json file in return and 'just' have to add all the problem classes by hand)

Saskia321 avatar Mar 31 '21 13:03 Saskia321