gong commented on code in PR #17646:
URL: https://github.com/apache/iceberg/pull/17646#discussion_r3793646066
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -134,6 +140,81 @@ public void testPostSuccess() throws Exception {
testHttpMethodOnSuccess(HttpMethod.POST);
}
+ @Test
+ public void testPostUnicodeBody() throws Exception {
Review Comment:
Thanks for the review — you're right.
The reason the tests still pass after reverting HTTPClient.java is that the
encoding doesn't come from our code in the current dependency: new
StringEntity(String) picks its charset from httpcore5's hard-coded
ContentType.DEFAULT_TEXT, which was changed to UTF-8 in httpcore5 5.3.0.
Iceberg has been on ≥ 5.3.1 since Jan 2024, so the explicit
StandardCharsets.UTF_8 is currently a no-op (defensive only).
@DefaultLocale won't help either — it only sets Locale, while the charset is
a hard-coded constant, not derived from locale or Charset.defaultCharset().
Given the current dependency can't reproduce the bug, my suggestion is to
keep the explicit UTF-8 plus the round-trip tests as a cheap, self-documenting
behavioral contract (guarding against a future httpcore5 regression), since
making them "revert-failing" under today's dependency isn't possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
