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

.Net: Handle media types with parameters #10000

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SergeyMenshykh
Copy link
Member

Motivation, Context and Description

This PR updates the RestApiOperationRunner class to be resilient to media types specified with parameters. It's a continuation of the first PR - #9959, where the OpenApiDocumentParser was updated to allow media types with parameters.

@SergeyMenshykh SergeyMenshykh requested a review from a team as a code owner December 17, 2024 13:59
@markwallace-microsoft markwallace-microsoft added the .NET Issue or Pull requests regarding .NET code label Dec 17, 2024
@SergeyMenshykh SergeyMenshykh added openapi Issues related to the OpenAPI function importer and removed .NET Issue or Pull requests regarding .NET code labels Dec 17, 2024
@markwallace-microsoft markwallace-microsoft added the .NET Issue or Pull requests regarding .NET code label Dec 17, 2024
@@ -352,7 +352,10 @@ private async Task<RestApiOperationResponse> ReadContentAndCreateOperationRespon
mediaType = mediaTypeFallback;
}

if (!this._payloadFactoryByMediaType.TryGetValue(mediaType!, out var payloadFactory))
// Remove media type parameters, such as x-api-version, from the "text/plain; x-api-version=2.0" media type string.
mediaType = mediaType!.Split(';').First();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might consider do a trimming as well?

Scenarios like " text/plain ; x-yz"

Is it possible to get the media type after the comma? (x-api=2.1; text/plain) ? If so pattern matching might worth.

Suggested change
mediaType = mediaType!.Split(';').First();
mediaType = mediaType!.Split(';').First();

// Remove media type parameters, such as x-api-version, from the "text/plain; x-api-version=2.0" media type string.
mediaType = mediaType!.Split(';').First();

if (!this._payloadFactoryByMediaType.TryGetValue(mediaType, out var payloadFactory))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any risk of case differences when comparing? I see on the mozilla site it says that:

MIME types are case-insensitive but are traditionally written in lowercase. The parameter values can be case-sensitive.

https://developer.mozilla.org/en-US/docs/Web/HTTP/MIME_types#structure_of_a_mime_type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Issue or Pull requests regarding .NET code openapi Issues related to the OpenAPI function importer
Projects
Status: Sprint: In Review
Development

Successfully merging this pull request may close these issues.

4 participants