Skip to content

Conversation

z-Fng
Copy link

@z-Fng z-Fng commented Sep 17, 2025

Description

Motivation and Context

Closes

When running checkver.ps1 for multiple apps, if the checkver is script and throws, it may return an incorrect version due to variables leakage from the previous iteration.

Changes

This PR makes the following changes:

  • fix(checkver): Prevent variable values from carrying over between iterations.

Testcase

$testcases = @(
    @{
        app   = "QTCreater"
        url   = "https://www.qt.io/offline-installers"
        regex = "Qt Creator\s+([\d.]+)\s+for Windows"
    }
    @{
        app    = "QQ"
        script = @(
            "`$url = 'https://im.qq.com/pcqq/index.shtml'",
            "`$resp = Invoke-WebRequest -Uri `$url",
            "`$cont = `$resp.Content",
            "`$pattern = 'https://qq-web.cdn-go.cn.*windowsDownloadUrl.js'",
            "`$jsUrl = [regex]::Match(`$cont, `$pattern).Value",
            "Invoke-WebRequest -Uri `$jsUrl"
        )
        regex  = "PCQQ([\d\.]+)/QQ([\d\.]+).exe"
    }
)

foreach ($testcase in $testcases) {
    $app = $testcase.app
    $re = New-Object System.Text.RegularExpressions.Regex($testcase.regex)

    # Prevent variable leakage from previous iteration
    # $page, $match = $null

    Write-Host "$app | ErrorActionPreference: $ErrorActionPreference"

    if ($testcase.script) {
        $page = Invoke-Command ([scriptblock]::Create($script -join "`r`n"))

        Write-Host "$app | Using script, page length $($page.Length)"
    } else {        
        $page = (Invoke-WebRequest -Uri $testcase.url -UseBasicParsing).Content

        Write-Host "$app | Using URL, page length $($page.Length)"
    }

    $match = $re.Matches($page) | Select-Object -First 1

    Write-Host "$app | Found: $match `r`n"
}
image image

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • Bug Fixes

    • Prevented intermittent cross-iteration leakage during asynchronous checks that could cause incorrect parsing or version detection; each iteration now processes results independently for more reliable multi-URL operations.
  • Chores

    • Minor internal cleanup and clarifying comment to reduce stale state risk; no user-facing behavior or configuration changes.

Copy link

coderabbitai bot commented Sep 17, 2025

Walkthrough

Resets per-iteration state variables ($page, $match, and $matchesHashtable) to $null in bin/checkver.ps1’s asynchronous download loop immediately after assigning $ver from $Version; adds a descriptive comment. No control-flow, error-handling, or API/signature changes.

Changes

Cohort / File(s) Summary of Changes
Async loop state cleanup
bin/checkver.ps1
Added explicit resets of $page, $match, and $matchesHashtable to $null within the async download loop (performed immediately after assigning $ver from $Version) and added a descriptive comment. No control-flow or API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I’m a rabbit in the loop, I hop and I clear,
Wiping old traces so each run is near;
Page and matches vanish, tidy and bright,
Fresh parsing awaits with each new byte. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly identifies the primary change—preventing variable leakage in checkver—and directly matches the PR description and code summary (resetting $page, $match, and $matchesHashtable in bin/checkver.ps1). It is specific, concise, and clear for a reviewer scanning history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04b7ce7 and 795673a.

📒 Files selected for processing (1)
  • bin/checkver.ps1 (1 hunks)
🔇 Additional comments (2)
bin/checkver.ps1 (2)

288-290: Good fix: clearing per-iteration state to prevent stale $page/$match.

This directly addresses the leakage described and ensures regex work on the intended payload each iteration.


409-415: No change needed — Invoke-AutoUpdate tolerates $null and empty @{ }

Get-VersionSubstitution in lib/autoupdate.ps1 checks $CustomMatches before calling GetEnumerator, so passing either $null or an empty hashtable is safe.

@z-Fng z-Fng changed the title fix(checkver): Prevent variable leakage from previous iteration fix(checkver): Prevent variable values from carrying over between iterations Sep 17, 2025
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.

1 participant