Skip to content

fix: a few regressions from previous PRs - #21

Open
tomerqodo wants to merge 5 commits into
sentry_combined_20260121_augment_sentry_coderabbit_1_base_fix_a_few_regressions_from_previous_prs_pr170from
sentry_combined_20260121_augment_sentry_coderabbit_1_head_fix_a_few_regressions_from_previous_prs_pr170
Open

tomerqodo wants to merge 5 commits into
sentry_combined_20260121_augment_sentry_coderabbit_1_base_fix_a_few_regressions_from_previous_prs_pr170from
sentry_combined_20260121_augment_sentry_coderabbit_1_head_fix_a_few_regressions_from_previous_prs_pr170

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#170

*self.window_id.lock().unwrap(),
self.webview_id,
WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
WebviewMessage::SetCookie(cookie.into_owned()),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The delete_cookie function incorrectly sends a WebviewMessage::SetCookie message instead of WebviewMessage::DeleteCookie, causing it to set cookies instead of deleting them.
Severity: HIGH

Suggested Fix

In the delete_cookie function, change the message being sent from WebviewMessage::SetCookie(cookie.into_owned()) to WebviewMessage::DeleteCookie(cookie.into_owned()) to match the function's intent and the corresponding message handler.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: crates/tauri-runtime-wry/src/lib.rs#L1718

Potential issue: The `delete_cookie` method in `WryWebviewDispatcher<T>` is implemented
to send a `WebviewMessage::SetCookie` message. However, the message handler has a
distinct and separate logic path for `WebviewMessage::DeleteCookie`. As a result, any
application call to `delete_cookie` will perform the opposite of its intended operation
by setting or updating the cookie instead of removing it. This breaks a fundamental
browser operation, preventing applications from clearing session data, which can have
privacy and security implications.

Did we get this right? 👍 / 👎 to inform future reviews.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants