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

Fix JSON parsing of text/plain content type #1172

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

ruxo
Copy link
Contributor

@ruxo ruxo commented Sep 15, 2024

Hello,

I am submitting a pull request to address the behavior of request body parsing, as discussed in issue #1168. My fix attempts to correct this behavior.

However, I'm not entirely sure if this fix is appropriate, as I don't fully understand why the current code relies on the body's detected type instead of the body's content type in both the request and response flows. I believe that the correction should prioritize the content type first, using the body's detected type as a fallback. But I'm not aware of all the history behind this implementation.

Could you please provide guidance or suggest any improvements?

Best regards,

Rux

Copy link
Collaborator

@StefH StefH left a comment

Choose a reason for hiding this comment

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

At least 1 unit fails. Please check your code.

And if possible, make also a unit test which covers your new scenario.

@@ -33,12 +33,17 @@ internal static HttpRequestMessage Create(IRequestMessage requestMessage, string
MediaTypeHeaderValue.TryParse(value, out contentType);
}

httpRequestMessage.Content = requestMessage.BodyData?.DetectedBodyType switch
var bodyData = requestMessage.BodyData;
var bodyType = bodyData?.DetectedBodyTypeFromContentType?.ToNullable()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably better and more readable to add a method BodyType GetBodyType() to the BodyData class.

BodyType GetBodyType()
{
    if (DetectedBodyTypeFromContentType != null)
    {
        return DetectedBodyTypeFromContentType;
    }

    // and so on.....
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problem is, here, we get IBodyData from the requestMessage. But I can write a class extension for that interface :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Just FYI, we can't simply check null as the value of DetectedBodyTypeFromContentType is not null, but BodyType.None. That's why I had to write an extension method for assuming null for this value.

@StefH StefH added the bug label Sep 20, 2024
@StefH StefH merged commit dd80fd7 into WireMock-Net:master Sep 20, 2024
8 checks passed
@StefH StefH changed the title Attempt to fix JSON parsing of text/plain content type Fix JSON parsing of text/plain content type Sep 25, 2024
@ruxo ruxo deleted the FixJsonDetection branch October 4, 2024 12:27
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 this pull request may close these issues.

2 participants