-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Fix][Connector-Http] fix Invalid mime type #9363
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
Conversation
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.
Pull Request Overview
This pull request fixes an invalid MIME type issue in the HTTP connector by adjusting how the request body is handled. Key changes include making the addBody method static, removing redundant header addition that set a content encoding, and updating the test to ensure that no manual content encoding is set.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| seatunnel-connectors-v2/connector-http/connector-http-base/src/test/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProviderTest.java | Added unit tests to assert headers remain unmodified and content encoding is not set |
| seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProvider.java | Made addBody static and removed redundant and potentially problematic header modifications |
Comments suppressed due to low confidence (1)
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProvider.java:492
- Consider adding a comment here explaining that the content encoding is intentionally not set to avoid triggering an invalid MIME type, which will help future maintainers understand the rationale behind this change.
StringEntity entity = new StringEntity(JsonUtils.toJsonString(body), ContentType.APPLICATION_JSON);
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.
Tks! @litiliu
Purpose of this pull request
close #9362
Does this PR introduce any user-facing change?
How was this patch tested?
UT added
Check list
New License Guide