fatmanverse opened a new pull request, #12921:
URL: https://github.com/apache/seatunnel/pull/12921
Fixes #12916
### Purpose of this pull request
`SubmitJobServlet#doPost` read its options with
`HttpServletRequest#getParameterMap()` before reading the job config from the
body. For an `application/x-www-form-urlencoded` request, which is what `curl
-d` / `--data-binary` send by default, Jetty parses the body as form fields
there and consumes the input stream, so the following `getReader()` failed with
`IllegalStateException: STREAMED`. HOCON, SQL and JSON bodies were all affected
unless the client set another content type.
The options of `/submit-job` (`format`, `jobId`, `jobName`,
`isStartWithSavePoint`, `restoreMode`, ...) are query parameters, so the
servlet now reads them from the query string only, through a new
`BaseServlet#getQueryParameterMap`. Values are URL-decoded as before. The body
is no longer parsed as form fields, so a config can never be mistaken for
parameters.
Other servlets are unchanged: `/submit-job/upload` reads its options with
`getParameterMap()` from a multipart request, and the other body-reading
endpoints except `/loggers` do not read request parameters. `/loggers` reads
`level` with `getParameter` before the body on purpose, so that a form field
works; it is left as it is.
Not covered here: with no charset in the `Content-Type`,
`BaseServlet#requestBody` decodes the body as ISO-8859-1 (the servlet default),
so non-ASCII text in a body sent as `text/plain` or form-encoded is garbled
while `application/json` is read as UTF-8. That happens on dev already for
`text/plain`, affects every body-reading endpoint, and is reported separately
in #12918.
### Does this PR introduce _any_ user-facing change?
Yes, as a bug fix: `/submit-job` accepts a body sent with any content type,
including curl's default. Sending options as form fields in the body never
worked, because the body is the job config.
### How was this patch tested?
- New `BaseServletQueryParameterTest` for the query string parser: no query
string, URL-decoding of names and values, keys without value and empty pairs,
repeated names (last wins, as before), invalid `%` escapes (rejected with
`IllegalArgumentException`, which `ExceptionHandlingFilter` turns into 400),
and that neither `getParameterMap()` nor `getParameter()` is called.
- New `RestApiSubmitJobContentTypeTest` against a real Jetty server: HOCON
and JSON bodies with `application/x-www-form-urlencoded`, JSON with
`application/json`, and URL-decoding of `jobName`. Three of the four cases fail
with `STREAMED` without the fix. The test passes together with
`RestApiSubmitJobConfigShadeDecryptTest`,
`RestApiSubmitJobStartWithSavePointTest`, `BaseServletTest`,
`OptionRulesApiTest` and `RestApiHttpBasicTest` (33 tests together with the
parser test) on JDK 17.
- On a Zeta cluster with plain `curl` and no `Content-Type`: HOCON, SQL and
JSON bodies failed with `STREAMED` before and are submitted now; `text/plain`
and `application/json` keep working; `?jobName=sales%26ops` gives job name
`sales&ops`.
### Check list
* [ ] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
* [ ] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [ ] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
--
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]
