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

[Bug]: Request/Response Interceptors Broken with .NET8 Upgrade / 6.6.1 #2906

Closed
pinkfloydx33 opened this issue May 16, 2024 · 8 comments · Fixed by #2907
Closed

[Bug]: Request/Response Interceptors Broken with .NET8 Upgrade / 6.6.1 #2906

pinkfloydx33 opened this issue May 16, 2024 · 8 comments · Fixed by #2907
Assignees
Labels
Milestone

Comments

@pinkfloydx33
Copy link

pinkfloydx33 commented May 16, 2024

Describe the bug

After the upgrade to 6.6.1, Request and Response interceptors are output to the document with the wrong property names (incorrect casing). They are expected to be written with initial uppercase letters, RequestInterceptorFunction and ResponseInterceptorFunction but are now being written as requestInterceptorFunction and responseInterceptorFunction. This causes them to longer function as expected (or rather, at all).

We rely on this functionality and missed this during the upgrade process as we were more focused on the document generation. We will need to roll back unless there's a workaround I'm not aware of

Expected behavior

The serialized functions should be output using the correct property names (or the expected property name in the index.html should be adjusted to reflect the new names) and should be invoked in response to requests/responses.

Actual behavior

Functions are serialized with incorrect names and no longer execute.

Steps to reproduce

Set a request or response interceptor:

services.Configure<SwaggerUIOptions>(s => s.UseRequestInterceptor("function(req) { return req; }"));

Previously this would produce the following:

var interceptors = JSON.parse('{"RequestInterceptorFunction":"function (req) { return req; }","ResponseInterceptorFunction":null}');
if (interceptors.RequestInterceptorFunction)
    configObject.requestInterceptor = parseFunction(interceptors.RequestInterceptorFunction);
if (interceptors.ResponseInterceptorFunction)
    configObject.responseInterceptor = parseFunction(interceptors.ResponseInterceptorFunction);

With latest version (6.6.1) it produces this instead:

var interceptors = JSON.parse('{"requestInterceptorFunction":"function (req) { return req; }"}');
if (interceptors.RequestInterceptorFunction)
    configObject.requestInterceptor = parseFunction(interceptors.RequestInterceptorFunction);
if (interceptors.ResponseInterceptorFunction)
    configObject.responseInterceptor = parseFunction(interceptors.ResponseInterceptorFunction);

Exception(s) (if any)

No response

Swashbuckle.AspNetCore version

6.6.1

.NET Version

net8.0

Anything else?

No response

@pinkfloydx33
Copy link
Author

I was able to workaround this by settings SwaggerUIOptions.JsonSerializerOptions with a DefaultJsonTypeInfoResolver with modifiers that fixed the casing on the two property names.

services.Configure<SwaggerUIOptions>(s =>
{
    s.JsonSerializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web)
    {
        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
        TypeInfoResolver = new DefaultJsonTypeInfoResolver
        {
            Modifiers =
            {
                static s =>
                {
                    if (s.Type != typeof(InterceptorFunctions)) 
                        return;
                    // SwaggerUI Index expected capital letters for the property names on the 
                    // interceptor functions (RequestInterceptorFunction and ResponseInterceptorFunction)
                    foreach (var prop in s.Properties)
                        prop.Name = char.ToUpper(prop.Name[0], CultureInfo.InvariantCulture) + prop.Name[1..];
                }
            }
        }
    };
});

Note that I found I needed to specify the DefaultIgnoreCondition, otherwise OAuth flows didn't work properly for some reason (that I couldn't figure out).

@pinkfloydx33
Copy link
Author

One fix would be to change the casing on lines 100 and 102 in the index.html file.

var interceptors = JSON.parse('%(Interceptors)');
if (interceptors.RequestInterceptorFunction)
configObject.requestInterceptor = parseFunction(interceptors.RequestInterceptorFunction);
if (interceptors.ResponseInterceptorFunction)
configObject.responseInterceptor = parseFunction(interceptors.ResponseInterceptorFunction);

Alternatively, you could add [JsonPropertyName] to the properties using an explicit uppercase character:

public class InterceptorFunctions
{
/// <summary>
/// MUST be a valid Javascript function.
/// Function to intercept remote definition, "Try it out", and OAuth 2.0 requests.
/// Accepts one argument requestInterceptor(request) and must return the modified request, or a Promise that resolves to the modified request.
/// Ex: "function (req) { req.headers['MyCustomHeader'] = 'CustomValue'; return req; }"
/// </summary>
public string RequestInterceptorFunction { get; set; }
/// <summary>
/// MUST be a valid Javascript function.
/// Function to intercept remote definition, "Try it out", and OAuth 2.0 responses.
/// Accepts one argument responseInterceptor(response) and must return the modified response, or a Promise that resolves to the modified response.
/// Ex: "function (res) { console.log(res); return res; }"
/// </summary>
public string ResponseInterceptorFunction { get; set; }

@martincostello
Copy link
Collaborator

JsonPropertyName attributes are how I would go about the fix if I were to do it. If you'd like to submit a PR we'd be grateful, otherwise I'll look into it tomorrow or over the weekend.

@pinkfloydx33
Copy link
Author

Personally I disagree with that. All of the other properties are lowercase including the ones on configObject that the deserialized functions get assigned to. The only place they exist with an uppercase character are in the expected shape of the temporary object used for deserialization. It just seems weird IMO.

Unfortunately I don't have the time to offer a PR right now

@martincostello
Copy link
Collaborator

I typically use JsonPropertyName to codify any property names and make them refactor-proof in my own projects to be explicit, rather than rely on conventions.

I'll go with that approach when I fix this.

@martincostello
Copy link
Collaborator

I might also rename it for consistency, but I've seen so many bugs caused by refactoring breaking deserialisation that I always use attributes for new code.

@martincostello martincostello self-assigned this May 16, 2024
@pinkfloydx33
Copy link
Author

If you don't rename it and just use attributes, then I won't have to remember to remove the workaround from my project since it will just keep working... So there's that upside =)

martincostello added a commit to martincostello/Swashbuckle.AspNetCore that referenced this issue May 16, 2024
Fix property casing of Swagger UI interceptor function properties.
Resolves domaindrivendev#2906.
@martincostello martincostello added this to the v6.6.2 milestone May 16, 2024
martincostello added a commit that referenced this issue May 21, 2024
Fix property casing of Swagger UI interceptor function properties.
Resolves #2906.
@martincostello
Copy link
Collaborator

Thanks for reporting this issue - the fix is available in Swashbuckle.AspNetCore 6.6.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants