Skip to content

[AMORO #4232] Improve AmsThriftUrl parsing and validation.#4233

Open
slfan1989 wants to merge 1 commit into
apache:masterfrom
slfan1989:amoro-4232
Open

[AMORO #4232] Improve AmsThriftUrl parsing and validation.#4233
slfan1989 wants to merge 1 commit into
apache:masterfrom
slfan1989:amoro-4232

Conversation

@slfan1989
Copy link
Copy Markdown
Contributor

Why are the changes needed?

Close #4232.

AmsThriftUrl is used to parse AMS thrift and ZooKeeper URLs. The previous thrift URL parsing lowercased the whole URL before parsing, which could unexpectedly change case-sensitive parts such as catalog names or query values.

This patch improves URL parsing robustness by preserving the original URL case and adding clearer validation for invalid thrift URLs.

Brief change log

  • Avoid lowercasing the whole thrift URL during parsing.
  • Keep URL scheme matching case-insensitive.
  • Preserve host, catalog name, and query value case.
  • Add explicit validation for thrift URL:
    • unsupported scheme
    • missing host
    • missing port
  • Extract shared socketTimeout query parsing logic.
  • Make socketTimeout parameter name case-insensitive.
  • Add TestAmsThriftUrl unit tests.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement]: Improve AmsThriftUrl parsing and validation

1 participant