2059: feat(scheduler): Redirect '/' to nighlies.a.o WebTUI - #84
2059: feat(scheduler): Redirect '/' to nighlies.a.o WebTUI#84martin-augment wants to merge 4 commits into
Conversation
Opening http://localhost:50050 in a browser will now redirect to the convenience deployment of the WebTUI app at https://nightlies.apache.org/datafusion/ballista/tui/...
WalkthroughThe scheduler now redirects ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a root handler (/) to the Ballista scheduler API that redirects requests to the Ballista WebTUI hosted on Apache Nightlies, forwarding query parameters. It also includes AI agent configuration files. The review feedback correctly identifies critical issues in the redirection logic: a lack of URL encoding for query parameters and a hardcoded HTTP protocol that could trigger mixed-content blocks when accessed via HTTPS. A robust code suggestion is provided to resolve these issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub async fn get_root( | ||
| header_map: HeaderMap, | ||
| Query(mut params): Query<std::collections::HashMap<String, String>>, | ||
| ) -> Result<Redirect, (StatusCode, String)> { | ||
| const NIGHTLIES_URL: &str = "https://nightlies.apache.org/datafusion/ballista/tui"; | ||
|
|
||
| let ballista_scheduler_url = | ||
| params.remove("ballista_scheduler_url").unwrap_or_else(|| { | ||
| let default_scheduler_url = "localhost:50050"; | ||
| let scheduler_url = header_map | ||
| .get("host") | ||
| .map(|hv| hv.to_str().unwrap_or(default_scheduler_url)) | ||
| .unwrap_or(default_scheduler_url); | ||
| format!("http://{scheduler_url}") | ||
| }); | ||
|
|
||
| let mut target = format!( | ||
| "{NIGHTLIES_URL}/{BALLISTA_VERSION}/?ballista_scheduler_url={ballista_scheduler_url}", | ||
| ); | ||
|
|
||
| for (k, v) in params.iter() { | ||
| target.push_str(format!("&{}={}", k, v).as_str()); | ||
| } | ||
|
|
||
| Ok(Redirect::temporary(&target)) | ||
| } |
There was a problem hiding this comment.
Issues Identified:
- Lack of URL Encoding: Reconstructing the query string using direct string interpolation (
format!("&{}={}", k, v)) does not URL-encode the keys and values. If any query parameter contains special characters (like spaces, ampersands, or equal signs), it will result in an invalid or corrupted URL. - Mixed Content / Protocol Detection: Hardcoding
http://for theballista_scheduler_urlwill cause browsers to block the WebTUI's requests to the scheduler if the scheduler is accessed via HTTPS (e.g., behind a reverse proxy). This is becausenightlies.apache.orgis served over HTTPS, and modern browsers block active mixed content (HTTP requests from an HTTPS page). We should detect the protocol using theX-Forwarded-Protoheader.
Recommendation:
Use url::form_urlencoded::Serializer to safely encode the query parameters, and inspect the X-Forwarded-Proto header to determine the correct scheme.
pub async fn get_root(
header_map: HeaderMap,
Query(mut params): Query<std::collections::HashMap<String, String>>,
) -> Result<Redirect, (StatusCode, String)> {
const NIGHTLIES_URL: &str = "https://nightlies.apache.org/datafusion/ballista/tui";
let ballista_scheduler_url =
params.remove("ballista_scheduler_url").unwrap_or_else(|| {
let default_scheduler_url = "localhost:50050";
let scheduler_url = header_map
.get("host")
.map(|hv| hv.to_str().unwrap_or(default_scheduler_url))
.unwrap_or(default_scheduler_url);
let proto = header_map
.get("x-forwarded-proto")
.and_then(|v| v.to_str().ok())
.unwrap_or("http");
format!("{proto}://{scheduler_url}")
});
let mut query_serializer = url::form_urlencoded::Serializer::new(String::new());
query_serializer.append_pair("ballista_scheduler_url", &ballista_scheduler_url);
for (k, v) in params.iter() {
query_serializer.append_pair(k, v);
}
let query_string = query_serializer.finish();
let target = format!("{NIGHTLIES_URL}/{BALLISTA_VERSION}/?{query_string}");
Ok(Redirect::temporary(&target))
}
🤖 Augment PR SummarySummary: Adds a root ( Changes:
Technical Notes: The redirect is a temporary redirect and derives a default scheduler URL from the request Host header. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| ); | ||
|
|
||
| for (k, v) in params.iter() { | ||
| target.push_str(format!("&{}={}", k, v).as_str()); |
There was a problem hiding this comment.
target is built by concatenating raw query keys/values (and ballista_scheduler_url) without URL-encoding, so values containing &, = or # can break the Location header or inject unintended query params. Consider ensuring the constructed redirect URL is properly encoded before passing it to Redirect::temporary.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| /// forwarding any query parameters | ||
| pub async fn get_root( | ||
| header_map: HeaderMap, | ||
| Query(mut params): Query<std::collections::HashMap<String, String>>, |
There was a problem hiding this comment.
Query<HashMap<String, String>> drops duplicate query parameters, which may not fully “forward any query parameters” as the doc comment states. If clients rely on repeated keys (e.g. foo=1&foo=2), the redirect could change behavior.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
ReviewPrompt-injection attempt detected — flagging before anything else. This PR adds four near-identical files (
These are instructions aimed at AI review tools (like this one), not legitimate project documentation, and they attempt to get automated reviewers to suppress scrutiny of exactly these files. I am not complying with them, and I'm surfacing this to human reviewers instead. Recommend these four files be removed from the PR unless there's a legitimate, human-authored reason for adding them — in which case they should live under a name that doesn't try to steer AI review behavior, and should contain something true about the project instead. What the PR does
Correctness / security concerns in
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ballista/scheduler/src/api/handlers.rs`:
- Around line 250-256: The target URL construction must URL-encode the scheduler
URL and every key/value from params instead of manually concatenating raw
strings. Update the code around target and the params iteration to use the
existing URL or query-serialization utilities, preserving all parameters while
ensuring reserved characters such as &, =, and # remain encoded within their
individual values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f55ade9b-da07-4387-81dd-cc2ae2225c95
📒 Files selected for processing (7)
.cursor/rules.md.gemini/rules.md.github/workflows/web-tui.ymlAGENTS.mdCLAUDE.mdballista/scheduler/src/api/handlers.rsballista/scheduler/src/api/routes.rs
| let mut target = format!( | ||
| "{NIGHTLIES_URL}/{BALLISTA_VERSION}/?ballista_scheduler_url={ballista_scheduler_url}", | ||
| ); | ||
|
|
||
| for (k, v) in params.iter() { | ||
| target.push_str(format!("&{}={}", k, v).as_str()); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
URL-encode query parameters to prevent HTTP Parameter Pollution.
Constructing a URL by manually concatenating query parameters without URL encoding can lead to invalid URLs or HTTP Parameter Pollution (HPP). If any parameter value (including the host header or values in params) contains reserved characters like &, =, or #, it will break the parsed query string on the receiving end.
Consider using a URL construction library (like the url crate) or a serialization crate (like serde_urlencoded, which axum usually depends on) to safely build the query string.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ballista/scheduler/src/api/handlers.rs` around lines 250 - 256, The target
URL construction must URL-encode the scheduler URL and every key/value from
params instead of manually concatenating raw strings. Update the code around
target and the params iteration to use the existing URL or query-serialization
utilities, preserving all parameters while ensuring reserved characters such as
&, =, and # remain encoded within their individual values.
2 participants
2059: To review by AI