Skip to content

fix(cube-cli): default to https:// for scheme-less Cube Cloud URLs - #11335

Merged
paveltiunov merged 1 commit into
masterfrom
claude/fix-it-5akbs9
Jul 23, 2026
Merged

fix(cube-cli): default to https:// for scheme-less Cube Cloud URLs#11335
paveltiunov merged 1 commit into
masterfrom
claude/fix-it-5akbs9

Conversation

@paveltiunov

Copy link
Copy Markdown
Member

Check List

  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Description of Changes Made (if issue reference is not provided)

Entering a Cube Cloud URL without a scheme (e.g. cloud.cubecloud.dev) at the cube login prompt — or via --api-url / CUBE_API_URL / a saved context — failed with reqwest's error: builder error: relative URL without a base.

  • Added a normalize_url helper in src/util.rs that trims whitespace and trailing slashes and prepends https:// when no scheme is present (explicit schemes like http://localhost:4000 are left untouched).
  • Applied it to the URL from the cube login prompt / --url flag, and in Ctx::api() so --api-url, CUBE_API_URL, and contexts saved with a scheme-less URL are normalized too.
  • Added unit tests covering scheme-less, trailing-slash, explicit-scheme, and empty inputs.

cargo test, cargo clippy, and cargo fmt --check pass in rust/cube-cli.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JLCPy9xaxFgSdH7fPMMLwX


Generated by Claude Code

Entering a URL like cloud.cubecloud.dev at the `cube login` prompt (or
via --api-url/CUBE_API_URL or a saved context) failed with reqwest's
"builder error: relative URL without a base". Normalize user-supplied
URLs by prepending https:// when no scheme is present.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JLCPy9xaxFgSdH7fPMMLwX
@github-actions github-actions Bot added the rust Pull requests that update Rust code label Jul 23, 2026
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @paveltiunov's task in 39s —— View job


Code Review — PR #11335

Verdict: LGTM — small, focused fix. Nothing blocking; a couple of minor observations below.

Details
  • Read util.rs, main.rs, login.rs changes
  • Check edge cases and test coverage
  • Post review

What the PR does well

  • normalize_url is applied at both entry points (login prompt/flag and Ctx::api()), so --api-url, CUBE_API_URL, prompt input, and previously-saved scheme-less contexts are all covered.
  • Unit tests cover the interesting cases: scheme-less, trailing slash + whitespace, explicit scheme untouched, empty string.
  • Explicit http://localhost:4000 is preserved — the right call for local dev.

Minor observations (non-blocking)

  1. contains("://") is a lenient scheme check. It'll pass through any scheme:// shape, including nonsense like ftp:// or foo://bar. That's fine — those will fail loudly downstream and the goal here is only to catch the obvious "user typed a hostname" case. Just noting the tradeoff. If you ever want stricter behavior, url::Url::parse (already in the reqwest dep tree) is available.

  2. trim_end_matches('/') strips all trailing slashes, so https://foo.com///https://foo.com (good) and https://foo.com/api/https://foo.com/api (also fine — Client presumably joins with /api/v1/...). Worth a quick mental check that no existing config out there stores a URL with a meaningful trailing slash on a path segment, but I don't see anywhere that would matter given the API paths used.

  3. Empty input returns "" and then falls through to Client::new("", ...), which will error with a less-friendly message than the original "not logged in" bail. Not a regression (empty input was equally broken before), but a bail! on empty in Ctx::api() after normalization could give a cleaner error. Purely a polish nit.

  4. Saved contexts: since login.rs:44 saves the normalized URL into ContextConfig, new logins persist with the scheme. Older configs get healed at read time in Ctx::api(). Nice — no migration needed.

Security / performance

Nothing to flag. Pure string manipulation, no allocations of concern, no auth logic changed.

• [`claude/fix-it-5akbs9`](https://github.com/cube-js/cube/tree/claude/fix-it-5akbs9)

@paveltiunov
paveltiunov merged commit 46aedb9 into master Jul 23, 2026
19 checks passed
@paveltiunov
paveltiunov deleted the claude/fix-it-5akbs9 branch July 23, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants