hansva commented on PR #8569:
URL: https://github.com/apache/hop/pull/8569#issuecomment-5814538189
## Review: Do not send a charset on the REST Client Content-Type (#8507)
The fix works for JSON, the case in the issue, but it removes the charset
parameter from the header for every content type. The body is still encoded
with that charset, so for non-JSON types the header no longer says how the
bytes are encoded.
### Should fix
1. **The charset is stripped for every type, not just JSON**
(`Rest.java:1669`)
When the row doesn't set a Content-Type, `mimeTypeOnly()` sends the bare
mime type whatever the application type is.
- Form posts use `ContentType.APPLICATION_FORM_URLENCODED`, which is
ISO-8859-1. The body is still encoded as ISO-8859-1, so `café` goes out as the
byte `0xE9` with just `Content-Type: application/x-www-form-urlencoded`. A
server that assumes UTF-8 garbles it.
- The default application type is TEXT PLAIN (`RestMeta.java:317`). It
maps to `ContentType.TEXT_PLAIN`, which is `text/plain; charset=UTF-8` in
httpcore5 5.4. Its UTF-8 bytes now go out as bare `text/plain`, which RFC 6657
says defaults to US-ASCII.
- For form posts this matches neither the behaviour before this change (a
correct label) nor Hop 2.17 (UTF-8 bytes, no label).
2. **A test now locks in that behaviour**
`testFormUrlEncodedPostOmitsCharsetAndKeepsLatin1Bytes`
(`RestCallRestTest.java:283`) asserts that Latin-1 bytes are sent with no
charset label. It would have to change with any fix.
3. **A Content-Type the row sets that can't be parsed now fails silently**
(`Rest.java:1830`)
An unsupported charset such as `charset=utf-99` used to fail the request.
Now it's ignored: the body is encoded with the application type's charset,
`charset=utf-99` still goes on the wire, and no error is reported.
### Suggested fix
Fix JSON only (`Rest.java:2307`). JSON is the only type here whose charset
parameter means nothing, since JSON is defined as UTF-8 (RFC 8259).
- In `init()`, map the JSON application type to
`ContentType.create("application/json")` instead of
`ContentType.APPLICATION_JSON`. Text and form types keep their constants, and
their correct labels.
- Encode a String body explicitly with `resolveCharset(type)` and send it as
a `ByteArrayEntity`. Don't use `StringEntity`: for a type with no charset it
falls back to ISO-8859-1. `resolveCharset` already falls back to UTF-8, so JSON
bodies stay UTF-8.
- Parse a row-supplied Content-Type strictly again, as before this change.
That replaces `contentTypeForBody()` and `mimeTypeOnly()`, fixes 1 and 3,
and the test in 2 changes to expect form and text types to keep their charset
label. The new JSON tests stay as they are.
--
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]
