-
Notifications
You must be signed in to change notification settings - Fork 579
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
Issue #7102 - WebClient should have a mode that is resilient to bad media/content types #9040
Conversation
…t to bad media/content types Signed-off-by: Tomáš Kraus <[email protected]>
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.
I have a 2 questions. :-)
- Why have you decided to drop ParserMode enum, which was used in the 4.x implementation?
- I think I have not seen it also in the 4.x version, but should not these relaxed formats of the content types be also configurable? I mean if user wants to add their own.
client.get() | ||
.path("/invalidContentType") | ||
.request() | ||
.await(); |
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.
Please set timeout to the await method.
client.get() | ||
.path("/invalidTextContentType") | ||
.request() | ||
.await(); |
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.
add timeout please.
WebClientResponse response = client.get() | ||
.path("/invalidTextContentType") | ||
.request() | ||
.await(); |
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.
add timeout please.
assertThat(maybeContentType.get().toString(), is(RELAXED_CONTENT_TYPE_TEXT)); | ||
} | ||
|
||
private static final String INVALID_CONTENT_TYPE_VALUE = "invalid header value"; |
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.
constants should be at the top of the class.
private static final String INVALID_CONTENT_TYPE_VALUE = "invalid header value"; | ||
|
||
// HTTP service with invalid Content-Type | ||
private static void invalidContentType(ServerRequest request, ServerResponse response) { |
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.
Move also these static methods under the beforeAll method please. I could not find them when trying to figure out what the endpoint does.
@Test | ||
void testInvalidTextContentTypeStrict() { | ||
try { | ||
client.get() |
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.
I think this formatting should be padded.
MediaType in 4.x is rewritten. It's parse method is not public so changing prototype did not cause any issues.
4.x version has hard-coded relaxed format as well. I made it the way jeff requested. This issue is just backport, not any additional enhancement to be done in 3.x only. |
Signed-off-by: Tomáš Kraus <[email protected]>
fixes issue #7102
backport of issue #6774 fix