pjfanning opened a new pull request, #1871:
URL: https://github.com/apache/pekko-connectors/pull/1871
**Motivation:**
The HTTPS enforcement added in #1810 has three problems.
**1. The error message never interpolates the scheme.**
```scala
val msg =
"Credentials are configured but the request URI scheme is '%s' (not
'https'). " +
"Sending BasicAuth credentials over plain HTTP is insecure. " +
"Configure a HTTPS base URL to use with credentials.".format(scheme)
```
`.format` binds to the last string literal only — the one containing no `%s`
— so the message is emitted with a literal `'%s'` and the operator never learns
which scheme was used.
**2. The check throws synchronously.**
`ElasticsearchSimpleFlowStage.onPush` calls `executeRequest` outside any
try/catch, so the exception escapes the stage rather than going through the
connector's failure handling. `ElasticsearchSourceStage.sendScrollScanRequest`
happens to wrap its call in `try { … } catch { failureHandler.invoke }`, so the
two stages behaved differently for the same condition.
**3. There is no way to opt out.**
The documented `#connection-settings` snippet pairs a `http://` base URL
with `withCredentials`, as do the Elasticsearch and Opensearch parameterized
tests. A previously working local setup now fails outright with no override
available.
Additionally, the credentials branch did not pass `connectionContext`, so a
connection context supplied via `withSSLContext` was silently dropped whenever
credentials were configured.
**Modification:**
- Parenthesise the concatenation so `format` applies to the whole message.
- Return a failed `Future` rather than throwing, so both stages route the
error through their own failure handling.
- Add `withAllowInsecureCredentialsTransport` (default `false`) as an
opt-out that downgrades the rejection to a logged warning, for local
development and testing.
- Pass `connectionContext` in the credentials branch as well.
The scheme decision is extracted into `checkCredentialsTransport` so it can
be tested without opening a socket. The documented snippets now use a
`https://` base URL with credentials, which is the configuration users should
be copying.
**Result:**
The message names the actual scheme. Failures arrive as a failed `Future`
and are reported by the connector rather than escaping the stage. Users who
need plain HTTP with credentials for local testing can opt in explicitly. A
user-supplied connection context is honoured when credentials are set.
**Tests:**
Extended `ElasticsearchApiSpec`:
- the rejection is asserted on the failed `Future` rather than as a
synchronous throw
- a new test asserts the scheme is interpolated and no `%s` remains
- four tests cover `checkCredentialsTransport`: reject `http`, accept
`https`, accept `HTTPS` case-insensitively, and warn when opted in
14 tests pass. Ran `elasticsearch/mimaReportBinaryIssues` (clean) and the
module's scalafmt checks.
**References:**
Follow-up to #1810.
--
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]
