Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAPI doesn't support stream parameters #59770

Closed
1 task done
rmannibucau opened this issue Jan 8, 2025 · 6 comments
Closed
1 task done

OpenAPI doesn't support stream parameters #59770

rmannibucau opened this issue Jan 8, 2025 · 6 comments
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.

Comments

@rmannibucau
Copy link

rmannibucau commented Jan 8, 2025

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Microsoft.AspNetCore.OpenApi.OpenApiSchemaStore prepopulate the Stream schema (note that it doesn't use the latest openapi representation but that would be an enhancement) but the key uses parameterinfo (set to null by default) so the getoradd in the store fails cause when using [FromBody] Stream body there is a parameter info.

Note that setting Stream as metadata to avoid to have a parameter fails in Microsoft.AspNetCore.OpenApi.OpenApiSchemaService.GetOrCreateSchemaAsync cause there is an api description so it still tries to lookup a JsonTypeInfo.

TIP: using this code can workaround the issue in an ugly manner:

[JsonSourceGenerationOptions]
[JsonSerializable(typeof(Stream))] // make GetOrCreateSchemaAsync happy (ApplyParameterInfo)
public partial class OpenApiWorkaroundForStream : JsonSerializerContext;

builder.Services.ConfigureHttpJsonOptions(o => o
            .SerializerOptions
            .TypeInfoResolverChain
            .Add(OpenApiWorkaroundForStream.Default));

        app
            .MapPost(....)
            .WithMetadata(new AcceptsMetadata(["application/octect-stream"], typeof(Stream))); // no param so hardcoded schema is used

Expected Behavior

The Stream schema is looked up as expected

Steps To Reproduce

use an minimal api endpoint with request body in parameters

Exceptions (if any)

No response

.NET Version

9

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jan 8, 2025
@martincostello martincostello added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jan 8, 2025
@mikekistler
Copy link
Contributor

@rmannibucau Can you explain a bit more about the expected behavior? I tried recreating this issue but I'm not seeing a problem. I have this endpoint, with a body parameter of type Stream:

app.MapPost("/bug", ([FromBody]Stream body) =>
{
    using var reader = new StreamReader(body);
    var json = reader.ReadToEndAsync().Result;
    Console.WriteLine(json);
    return TypedResults.Ok();
})
.WithName("bug");

and the framework generates the following in the OpenAPI doc:

  "paths": {
    "/bug": {
      "post": {
        "tags": [
          "issue-59770"
        ],
        "operationId": "bug",
        "requestBody": {
          "content": {
            "application/octet-stream": {
              "schema": {
                "$ref": "#/components/schemas/Stream"
              }
            }
          },
          "required": true
        },
        "responses": {
          "200": {
            "description": "OK"
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Stream": {
        "type": "string",
        "format": "binary"
      }
    }
  },

which looks right to me. Is there actually a problem here?

@mikekistler mikekistler added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Jan 17, 2025
@rmannibucau
Copy link
Author

Hi @mikekistler , I assume you didn't enable AOT for json:

/home/rmannibucau/.nuget/packages/microsoft.extensions.apidescription.server/9.0.0/build/Microsoft.Extensions.ApiDescription.Server.targets(68,5): error : System.AggregateException: One or more errors occurred. (JsonTypeInfo metadata for type 'System.Void' was not provided by TypeInfoResolver of type 'System.Text.Json.Serialization.Metadata.JsonTypeInfoResolverWithAddedModifiers'. If using source generation, ensure that all root types passed to the serializer have been annotated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.) 

I didn't test with reflection enabled for JSON but without it fails like that whereas with the proposed workaround it works.

@dotnet-policy-service dotnet-policy-service bot added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jan 17, 2025
@mikekistler
Copy link
Contributor

I've been able to recreate this problem. With AoT enabled:

    <PublishAot>true</PublishAot>

and generating the OpenAPI at build time I get this exception from dotnet build:

>dotnet build
Restore complete (0.3s)
  issue-59770 failed with 25 error(s) (1.9s) → bin/Debug/net9.0/issue-59770.dll
    /Users/mikekistler/.nuget/packages/microsoft.extensions.apidescription.server/9.0.1/build/Microsoft.Extensions.ApiDescription.Server.targets(68,5): error : System.AggregateException: One or more errors occurred. (JsonTypeInfo metadata for type 'System.IO.Stream' was not provided by TypeInfoResolver of type 'System.Text.Json.Serialization.Metadata.JsonTypeInfoResolverWithAddedModifiers'. If using source generation, ensure that all root types passed to the serializer have been annotated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.)
    /Users/mikekistler/.nuget/packages/microsoft.extensions.apidescription.server/9.0.1/build/Microsoft.Extensions.ApiDescription.Server.targets(68,5): error :  ---> System.NotSupportedException: JsonTypeInfo metadata for type 'System.IO.Stream' was not provided by TypeInfoResolver of type 'System.Text.Json.Serialization.Metadata.JsonTypeInfoResolverWithAddedModifiers'. If using source generation, ensure that all root types passed to the serializer have been annotated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.
    /Users/mikekistler/.nuget/packages/microsoft.extensions.apidescription.server/9.0.1/build/Microsoft.Extensions.ApiDescription.Server.targets(68,5): error :    at System.Text.Json.ThrowHelper.ThrowNotSupportedException_NoMetadataForType(Type type, IJsonTypeInfoResolver resolver)
...

This appears to be a dup of #56021. That issue alludes to a workaround but it took me a while to figure out. And there is some documentation on this but it too did not provide the full story.

https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/source-generation

From the docs it seems that you need to add a class to your app the derives from JsonSerializerContext and then add attributes on that class to specify the types that should be serializable. For Stream this would look something like this:

[JsonSerializable(typeof(Stream))]
internal partial class SourceGenerationContext : JsonSerializerContext {}

Then since the Stream is a parameter you need to define global JsonSerializerOptions for your app with the TypeInfoResolver from the context. You can do that with the following bit of code.

builder.Services.ConfigureHttpJsonOptions(options =>
{
    options.SerializerOptions.TypeInfoResolver = SourceGenerationContext.Default;
});

With these changes the app builds cleanly and produces the expected OpenAPI.

I hope this explanation is helpful. Since this is the same problem as reported in #56021 I'm going to close this as a dup.

@rmannibucau
Copy link
Author

@mikekistler can you reopen it please? As explained aot setup was properly done and stream must not be be added to serilization context since it will not work so this stays a bug in openapi code due to the key in the dictionary in the code i pointed out. Fix is trivial by ignoring parameter from the key for some known types.

@mikekistler
Copy link
Contributor

@rmannibucau The issue was closed as a dup of #56021, which remains open and is flagged as a bug and for consideration for .NET 10. I can reopen this if you think this bug is somehow different from the one in ##56021. If you think that is the case can you explain the difference? In my testing the exception thrown is the same.

But I'm curious why you say "stream must not be added to serialization context". In my testing this did not affect the handling of the stream parameter. My test method looks like this:

app.MapPost("/stream-param", ([FromBody]Stream stream) =>
{
    using var reader = new StreamReader(stream);
    var body = reader.ReadToEndAsync().Result;
    return TypedResults.Ok(body);
})
.WithName("stream-param");

And it works as expected -- here's the response:

HTTP/1.1 200 OK
Connection: close
Content-Type: application/json; charset=utf-8
Date: Mon, 20 Jan 2025 12:38:35 GMT
Server: Kestrel
Transfer-Encoding: chunked

"This is the stream body"

Did you have different results?

@rmannibucau
Copy link
Author

@mikekistler now use your custom serialization context to serialize a stream -> doesn't do at all what you expect so you fixed one side and broke another one when json context is used as a registry so you do not want it. Workaround to do yet another context split from the app one kind of workish but is really unexpected.

Also note that openapi code handles stream properly but has a deeper bug to fallback on json for any parameter (whereas openapi also supports other media types) so minimum for me is to ensure the hardcoded types can be used for explicit parameters (stream is there) and ideally add an attribute to flag a parameter as not json friendly or something like that.

I'm fine if both issues converge - think they can - but they are different in the sense adding bool to a json context is fine since json handles bool but stream has unexpected side effects in apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Projects
None yet
Development

No branches or pull requests

3 participants