Skip to content

fix: a few regressions from previous PRs - #10

Open
tomerqodo wants to merge 5 commits into
sentry_full_base_fix_a_few_regressions_from_previous_prs_pr10from
sentry_full_head_fix_a_few_regressions_from_previous_prs_pr10
Open

tomerqodo wants to merge 5 commits into
sentry_full_base_fix_a_few_regressions_from_previous_prs_pr10from
sentry_full_head_fix_a_few_regressions_from_previous_prs_pr10

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#10

*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, which causes cookies to be set or updated instead of deleted.
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 ensure the correct delete operation is invoked.

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: In `crates/tauri-runtime-wry/src/lib.rs`, the `delete_cookie` function
incorrectly sends a `WebviewMessage::SetCookie` message instead of the expected
`WebviewMessage::DeleteCookie`. This means that when an application calls the
`delete_cookie()` API, the cookie will be set or updated rather than removed. This
breaks the core functionality of deleting cookies and could lead to security issues
where sensitive cookies cannot be properly cleared.

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