Skip to content

Conversation

@a10y
Copy link
Contributor

@a10y a10y commented Oct 14, 2025

Found this while fuzzing, the failure was

image

Was confused how the expression was getting reverse. The previous logic was just discarding information and picking the larger lit as the upper bound. This version should be more robust.

I've added a unit test for this case, it fails with the old version of find_between and succeeds now. I've also confirmed my fuzz artifact succeeds when I run with new code:

image

@a10y a10y added the changelog/fix A bug fix label Oct 14, 2025
@codecov
Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.01%. Comparing base (d37444a) to head (221b7cd).
⚠️ Report is 4 commits behind head on develop.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

) {
(true, false) => lhs.clone(),
(false, true) => BinaryExpr::new(lhs.rhs().clone(), lhs.op().swap()?, lhs.lhs().clone()),
_ => return None,
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that a variable could be between two other variables?
a <= x <= b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function only does the rewrite if it's variable between two lits

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I guess we can leave it like than, but there is not reason this needs to be the case

Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y a10y force-pushed the aduffy/between-bug branch from 891d8d3 to 221b7cd Compare October 14, 2025 17:50
_ => return None,
};

let (left, left_op, right, right_op) = if left_lit.value() > right_lit.value() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was the bad piece

Copy link
Contributor

@joseph-isaacs joseph-isaacs left a comment

Choose a reason for hiding this comment

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

LG

@a10y a10y enabled auto-merge (squash) October 14, 2025 18:01
@a10y a10y merged commit 408bc57 into develop Oct 14, 2025
44 checks passed
@a10y a10y deleted the aduffy/between-bug branch October 14, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants