fix: reject invalid --headers JSON, empty frame commands, and --cdp + --extension combo (#488)
## Summary
- Return a `ParseError` when `--headers` receives invalid JSON instead of silently dropping the headers and proceeding
- Reject `frame` commands that provide no `selector`, `name`, or `url` (previously returned `{ switched: true }` without doing anything)
- Add missing mutual exclusion check for `--cdp` + `--extension` (extensions require a local browser, not a CDP connection)
This commit is contained in:
+12
-6
@@ -103,9 +103,12 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
|
|||||||
let mut nav_cmd = json!({ "id": id, "action": "navigate", "url": url });
|
let mut nav_cmd = json!({ "id": id, "action": "navigate", "url": url });
|
||||||
// If --headers flag is set, include headers (scoped to this origin)
|
// If --headers flag is set, include headers (scoped to this origin)
|
||||||
if let Some(ref headers_json) = flags.headers {
|
if let Some(ref headers_json) = flags.headers {
|
||||||
if let Ok(headers) = serde_json::from_str::<serde_json::Value>(headers_json) {
|
let headers = serde_json::from_str::<serde_json::Value>(headers_json)
|
||||||
nav_cmd["headers"] = headers;
|
.map_err(|_| ParseError::InvalidValue {
|
||||||
}
|
message: format!("Invalid JSON for --headers: {}", headers_json),
|
||||||
|
usage: "open <url> --headers '{\"Key\": \"Value\"}'",
|
||||||
|
})?;
|
||||||
|
nav_cmd["headers"] = headers;
|
||||||
}
|
}
|
||||||
// Include iOS device info if specified (needed for auto-launch with existing daemon)
|
// Include iOS device info if specified (needed for auto-launch with existing daemon)
|
||||||
if flags.provider.as_deref() == Some("ios") {
|
if flags.provider.as_deref() == Some("ios") {
|
||||||
@@ -1830,9 +1833,12 @@ mod tests {
|
|||||||
fn test_navigate_with_invalid_headers_json() {
|
fn test_navigate_with_invalid_headers_json() {
|
||||||
let mut flags = default_flags();
|
let mut flags = default_flags();
|
||||||
flags.headers = Some("not valid json".to_string());
|
flags.headers = Some("not valid json".to_string());
|
||||||
let cmd = parse_command(&args("open api.example.com"), &flags).unwrap();
|
let result = parse_command(&args("open api.example.com"), &flags);
|
||||||
// Invalid JSON should result in no headers field (graceful handling)
|
// Invalid JSON should return a ParseError, not silently drop headers
|
||||||
assert!(cmd.get("headers").is_none());
|
assert!(result.is_err());
|
||||||
|
let err = result.unwrap_err();
|
||||||
|
let msg = err.format();
|
||||||
|
assert!(msg.contains("Invalid JSON for --headers"));
|
||||||
}
|
}
|
||||||
|
|
||||||
// === Set Headers Tests ===
|
// === Set Headers Tests ===
|
||||||
|
|||||||
@@ -336,6 +336,16 @@ fn main() {
|
|||||||
exit(1);
|
exit(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if flags.cdp.is_some() && !flags.extensions.is_empty() {
|
||||||
|
let msg = "Cannot use --extension with --cdp (extensions require local browser)";
|
||||||
|
if flags.json {
|
||||||
|
println!(r#"{{"success":false,"error":"{}"}}"#, msg);
|
||||||
|
} else {
|
||||||
|
eprintln!("{} {}", color::error_indicator(), msg);
|
||||||
|
}
|
||||||
|
exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
// Auto-connect to existing browser
|
// Auto-connect to existing browser
|
||||||
if flags.auto_connect {
|
if flags.auto_connect {
|
||||||
let mut launch_cmd = json!({
|
let mut launch_cmd = json!({
|
||||||
|
|||||||
@@ -733,6 +733,21 @@ describe('parseCommand', () => {
|
|||||||
expect(result.success).toBe(true);
|
expect(result.success).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should reject frame with no selector, name, or url', () => {
|
||||||
|
const result = parseCommand(cmd({ id: '1', action: 'frame' }));
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should parse frame with name', () => {
|
||||||
|
const result = parseCommand(cmd({ id: '1', action: 'frame', name: 'myframe' }));
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should parse frame with url', () => {
|
||||||
|
const result = parseCommand(cmd({ id: '1', action: 'frame', url: 'https://example.com' }));
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('should parse mainframe', () => {
|
it('should parse mainframe', () => {
|
||||||
const result = parseCommand(cmd({ id: '1', action: 'mainframe' }));
|
const result = parseCommand(cmd({ id: '1', action: 'mainframe' }));
|
||||||
expect(result.success).toBe(true);
|
expect(result.success).toBe(true);
|
||||||
|
|||||||
@@ -1001,6 +1001,14 @@ export function parseCommand(input: string): ParseResult {
|
|||||||
return { success: false, error: 'Either content or url must be provided', id };
|
return { success: false, error: 'Either content or url must be provided', id };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (command.action === 'frame' && !command.selector && !command.name && !command.url) {
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: 'frame command requires at least one of: selector, name, or url',
|
||||||
|
id,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
return { success: true, command };
|
return { success: true, command };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user