-
-
Notifications
You must be signed in to change notification settings - Fork 213
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 issues with Proxy mode and Binary Request Bodies #334
Conversation
Codecov Report
@@ Coverage Diff @@
## master #334 +/- ##
==========================================
+ Coverage 80.16% 80.25% +0.09%
==========================================
Files 111 111
Lines 4442 4442
==========================================
+ Hits 3561 3565 +4
+ Misses 881 877 -4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Can you please take a look at my comments?
I agree with your comments, I will do it as suggested. |
@StefH Please review my changes. I added some test coverage, but I have to admit I don't fully understand the multipart thing in BodyParser. I'm not used to a pull request workflow, is there anything else I should do? Merge sth? |
Hello @andi0b,
|
If you use different, a lot of code changes will be displayed. |
Yeah, actually I use Rider with auto detect of code style (as there is no .editorconfig), and it's using 4 spaces, no idea what happened there... maybe it tried to outsmart all of us. But there is also something weird with this change: c0a43ed#diff-78e158879161b0265ca8030bed233919 |
# Conflicts: # src/WireMock.Net/Util/BodyParser.cs
already solved it, see next commit I don't really know what to do about the failing test So the first part never matched because the content isn't json: // Assign
string content = "hello";
var server = FluentMockServer.Start();
server
.Given(Request.Create().WithBody(b => true))
.AtPriority(0)
.RespondWith(Response.Create().WithStatusCode(400)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
But there is also something weird with this change: c0a43ed#diff-78e158879161b0265ca8030bed233919 --> Yes, I think that the previous fix in that file (done by someone else) also used a different space/tabs setting. |
We used Wiremocks.NET in Proxy mode to sometimes test against the real API and sometimes just mock it. This stopped working when we started POSTing JPEGs. The body was altered because WireMocks.NET started to parse the binary content as UTF8 and converted it back. This causes issues with binary contents, you can see it because the length changes.
This pull request contains: