Skip to content

fix(toolset): handle underflow in version_sub function#6389

Merged
jdx merged 1 commit intojdx:mainfrom
koh-sh:fix/version-sub-underflow
Sep 23, 2025
Merged

fix(toolset): handle underflow in version_sub function#6389
jdx merged 1 commit intojdx:mainfrom
koh-sh:fix/version-sub-underflow

Conversation

@koh-sh
Copy link
Contributor

@koh-sh koh-sh commented Sep 23, 2025

Fix integer underflow when subtracting version components, ensuring sub-0.0.1:2.79.0 resolves to the previous version 2.78.0 instead of generating invalid version numbers.

Fixes: #6360

Fix integer underflow when subtracting version components, ensuring
sub-0.0.1:2.79.0 resolves to the previous version 2.78.0 instead of
generating invalid version numbers.

Fixes: jdx#6360
Copilot AI review requested due to automatic review settings September 23, 2025 09:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an integer underflow bug in the version_sub function that occurs when subtracting version components would result in negative values. The fix implements proper borrowing logic from higher-order version digits and ensures that underflow cases return valid version numbers instead of invalid results.

  • Implements borrowing mechanism to handle version component underflow
  • Adds comprehensive test coverage for underflow scenarios
  • Updates end-to-end tests to verify the fix works in practice

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/toolset/tool_request.rs Adds underflow handling logic with borrowing and comprehensive test cases
e2e/plugins/test_version_range Adds end-to-end test for the specific underflow case mentioned in the issue

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +319 to +327
for j in (0..i).rev() {
let prev_val = orig.chunks.0[j].single_digit().unwrap();
if prev_val > 0 {
orig.chunks.0[j] = Chunk::Numeric(prev_val - 1);
orig.chunks.0.truncate(j + 1);
return orig.to_string();
}
}
return "0".to_string();
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

The borrowing logic is incomplete. When a digit at position j has value 0, the algorithm should continue borrowing from higher digits (j-1, j-2, etc.) and set intermediate digits to their maximum borrowable value. Currently, it skips over zero digits without proper carry propagation, which could lead to incorrect results in cases like version_sub('1.0.0.1', '0.0.0.2').

Suggested change
for j in (0..i).rev() {
let prev_val = orig.chunks.0[j].single_digit().unwrap();
if prev_val > 0 {
orig.chunks.0[j] = Chunk::Numeric(prev_val - 1);
orig.chunks.0.truncate(j + 1);
return orig.to_string();
}
}
return "0".to_string();
// Propagate borrow through zeros and set intermediate digits to max value
let mut borrowed = false;
for j in (0..i).rev() {
let prev_val = orig.chunks.0[j].single_digit().unwrap();
if prev_val > 0 {
// Decrement the digit we borrow from
orig.chunks.0[j] = Chunk::Numeric(prev_val - 1);
// Set all digits between j+1 and i-1 to max value (9)
for k in (j+1)..i {
orig.chunks.0[k] = Chunk::Numeric(9);
}
borrowed = true;
break;
}
}
if !borrowed {
return "0".to_string();
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current implementation is actually correct. The algorithm properly handles the case you
mentioned:

For version_sub('1.0.0.1', '0.0.0.2'):

  1. Underflow occurs at position 3 (1 < 2)
  2. Loop searches backwards: j=2 (value=0, skip), j=1 (value=0, skip), j=0 (value=1, borrow)
  3. Borrows from position 0: decrements to 0, truncates to position 1
  4. Returns "0"

This behavior is intentional. Returning "0" allows the version resolution system to find the latest
available version within major version 0 (e.g., 0.9.x series), which is the expected behavior when
the subtraction would result in a negative version. This maintains consistency with mise's version
resolution logic where "0" acts as a fallback to resolve the latest version in the 0.x range.

If maintainers prefer different handling for such edge cases, I'm happy to adjust the implementation
accordingly.

@jdx
Copy link
Owner

jdx commented Sep 23, 2025

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!


@jdx jdx merged commit 424846b into jdx:main Sep 23, 2025
19 checks passed
@jdx jdx mentioned this pull request Sep 23, 2025
jdx added a commit that referenced this pull request Sep 24, 2025
### 🚀 Features

- **(java)** add support for Liberica NIK releases by @roele in
[#6382](#6382)

### 🐛 Bug Fixes

- **(toolset)** handle underflow in version_sub function by @koh-sh in
[#6389](#6389)

### 📚 Documentation

- document MISE_ENV behavior for global/system configs by @jdx in
[#6385](#6385)

### New Contributors

- @jc00ke made their first contribution in
[#6386](#6386)
- @koh-sh made their first contribution in
[#6389](#6389)

Co-authored-by: mise-en-dev <release@mise.jdx.dev>
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 25, 2025
## [2025.9.18](https://github.com/jdx/mise/compare/v2025.9.17..v2025.9.18) - 2025-09-24

### 📦 Registry

- replace amplify-cli github backend with ubi by @eggplants in [#6396](jdx/mise#6396)

### 🚀 Features

- **(template)** add read_file() function by @jdx in [#6400](jdx/mise#6400)

### 🐛 Bug Fixes

- **(aqua)** support github_artifact_attestations.enabled by @risu729 in [#6372](jdx/mise#6372)
- use /c instead of -c on windows in postinstall hook by @risu729 in [#6397](jdx/mise#6397)

### 🧪 Testing

- **(test-tool)** uninstall all versions and clear cache before installation by @jdx in [#6393](jdx/mise#6393)

### New Contributors

- @eggplants made their first contribution in [#6396](jdx/mise#6396)

## [2025.9.17](https://github.com/jdx/mise/compare/v2025.9.16..v2025.9.17) - 2025-09-24

### 🚀 Features

- **(java)** add support for Liberica NIK releases by @roele in [#6382](jdx/mise#6382)

### 🐛 Bug Fixes

- **(toolset)** handle underflow in version_sub function by @koh-sh in [#6389](jdx/mise#6389)

### 📚 Documentation

- document MISE_ENV behavior for global/system configs by @jdx in [#6385](jdx/mise#6385)

### New Contributors

- @jc00ke made their first contribution in [#6386](jdx/mise#6386)
- @koh-sh made their first contribution in [#6389](jdx/mise#6389)
riastradh pushed a commit to riastradh/pkgsrc-test20250901 that referenced this pull request Feb 8, 2026
## [2025.9.18](https://github.com/jdx/mise/compare/v2025.9.17..v2025.9.18) - 2025-09-24

### 📦 Registry

- replace amplify-cli github backend with ubi by @eggplants in [#6396](jdx/mise#6396)

### 🚀 Features

- **(template)** add read_file() function by @jdx in [#6400](jdx/mise#6400)

### 🐛 Bug Fixes

- **(aqua)** support github_artifact_attestations.enabled by @risu729 in [#6372](jdx/mise#6372)
- use /c instead of -c on windows in postinstall hook by @risu729 in [#6397](jdx/mise#6397)

### 🧪 Testing

- **(test-tool)** uninstall all versions and clear cache before installation by @jdx in [#6393](jdx/mise#6393)

### New Contributors

- @eggplants made their first contribution in [#6396](jdx/mise#6396)

## [2025.9.17](https://github.com/jdx/mise/compare/v2025.9.16..v2025.9.17) - 2025-09-24

### 🚀 Features

- **(java)** add support for Liberica NIK releases by @roele in [#6382](jdx/mise#6382)

### 🐛 Bug Fixes

- **(toolset)** handle underflow in version_sub function by @koh-sh in [#6389](jdx/mise#6389)

### 📚 Documentation

- document MISE_ENV behavior for global/system configs by @jdx in [#6385](jdx/mise#6385)

### New Contributors

- @jc00ke made their first contribution in [#6386](jdx/mise#6386)
- @koh-sh made their first contribution in [#6389](jdx/mise#6389)
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