leizhiyuan opened a new pull request, #1421:
URL: https://github.com/apache/rocketmq-clients/pull/1421
### Which Issue(s) This PR Fixes
Fixes #1419
### Brief Description
**Scenario.** The official lite examples (`rust/examples/lite_producer.rs`
and `lite_push_consumer.rs`) call
`client_option.set_access_url("http://localhost:8080";)`. Users copy that, or a
url that a console shows with a scheme, and `start()` fails with a DNS error
for the peer `http://<host>:8080`.
**Root cause.** `Endpoints::from_url` splits at the last `:`, so the host
becomes `http://<host>`. `Session::build_endpoint` then prefixes the scheme
again.
**Change.** This aligns with Java, where `Endpoints(String)` strips both
prefixes for a single address (covered by
`EndpointsTest#testEndpointsWith*AndHttp(s)Prefix`).
- For a single address, `Endpoints::from_url` strips a leading `http://` or
`https://`. `endpoint_url()`, which is also the session map key and the `peer`
in logs and errors, then becomes `host:port`.
- A multi-address url (`a:1,b:2`) is left as is, as in Java.
- The prefix does **not** enable TLS. Java only strips it too. TLS stays
controlled by `ClientOption::set_enable_tls`. The doc comment says so.
- The examples are left unchanged: `http://localhost:8080` works now.
#### Notes for reviewers
- **Port is still required.** Without a port, Java falls back to
`DEFAULT_PORT = 80`. The Rust parser still requires a port (`port in endpoint
url is missing`), including after a stripped prefix. I left that out of scope,
since it is a separate behavior and nobody reported it.
- **No conflicts.** Only `rust/src/model/common.rs` is touched. `git
merge-tree` against the open Rust PRs #1381, #1404, #1407, #1409, #1411, #1414,
#1416 and __B11_PR__ (the blank lite topic fix) is clean. Overview in #1412.
### How Did You Test This Change?
**Unit test.** `parse_endpoint_url_with_http_prefix` fails on `master`. It
checks that:
- `http://localhost:8080` and `https://localhost:8080` parse to
`localhost:8080` with scheme `DomainName`, host `localhost` and port `8080`;
- `http://127.0.0.1:8080` parses to `127.0.0.1:8080` with scheme `IPv4`;
- `http://localhost` still fails with `port in endpoint url is missing`.
The existing `parse_*_endpoint_url` tests are unchanged and pass.
```
$ cargo test
test result: ok. 90 passed; 0 failed; 1 ignored (lib; master: 89)
test result: ok. 3 passed; 0 failed (lite_integration_test)
test result: ok. 2 passed; 0 failed; 3 ignored (doc-tests)
$ cargo fmt --check # clean
$ RUSTDOCFLAGS="--cfg docsrs" cargo doc --no-deps --all-features # no
warnings
$ cargo build --examples # ok
```
`cargo clippy --all-features -- -D warnings` reports only the 2 `for_kv_map`
errors that `master` already has in `rust/src/client.rs` (lines 904 and 912).
`cargo clippy --lib --tests` reports the same findings as `master`.
**End-to-end.** I ran this against a managed RocketMQ 5 server, with the
access url set to `http://<endpoint>:8080` as in the examples:
| | before | after |
|---|---|---|
| producer start + send a lite message | `Failed to create session ...
failed to connect to peer http://<endpoint>:8080`, dns error | `Ok`, message id
returned |
---
Tracked in #1412 (overview of the related Rust issues / PRs and a suggested
merge order).
--
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]
