Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion Sources/SentryDistribution/Updater.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public final class Updater: Sendable {
components.queryItems?.append(URLQueryItem(name: "build_version", value: shortVersionString))
}
if let bundleVersion = Bundle.main.infoDictionary?["CFBundleVersion"] as? String {
components.queryItems?.append(URLQueryItem(name: "build_number", value: bundleVersion))
components.queryItems?.append(URLQueryItem(name: "build_number", value: normalizeBuildNumber(bundleVersion)))
}
if let installGroups = params.installGroupsOverride {
for group in installGroups {
Expand All @@ -115,6 +115,33 @@ public final class Updater: Sendable {
}
}

private func normalizeBuildNumber(_ buildNumber: String) -> String {
let components = buildNumber.split(separator: ".", omittingEmptySubsequences: false)
guard (2...3).contains(components.count),
components.allSatisfy({ component in
!component.isEmpty && component.allSatisfy(\.isNumber)
}) else {
return buildNumber
Comment on lines +120 to +124

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 normalizeBuildNumber function fails to normalize single-component build numbers (e.g., "42"), returning the raw string instead of the expected format required by the API.
Severity: HIGH

Suggested Fix

Modify the guard condition in normalizeBuildNumber to accept single-component version strings by changing the check to (1...3).contains(components.count). Then, adjust the logic to correctly pad and convert single-component numbers into the required base-1,000,000 format, ensuring they are handled consistently with multi-component versions.

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: Sources/SentryDistribution/Updater.swift#L120-L124

Potential issue: The function `normalizeBuildNumber` is designed to convert version
strings into a normalized base-1,000,000 format for the Sentry API. However, a guard
condition `(2...3).contains(components.count)` causes it to reject and bypass
normalization for single-component build numbers, such as "42". This is a valid and
common format for `CFBundleVersion` in iOS apps. As a result, these build numbers are
sent to the API unnormalized, which prevents the server from correctly comparing
versions and defeats the purpose of the change for apps using simple integer build
numbers.

Did we get this right? πŸ‘ / πŸ‘Ž to inform future reviews.

}

// Sentry stores each version component in a six-digit base-1,000,000 slot.
let componentBase: UInt64 = 1_000_000
let numericComponents = components.compactMap { UInt64($0) }
guard numericComponents.count == components.count,
numericComponents.allSatisfy({ $0 < componentBase }) else {
return buildNumber
}

let paddedComponents = numericComponents + Array(
repeating: 0,
count: 3 - numericComponents.count)
let normalizedBuildNumber = paddedComponents.reduce(UInt64(0)) { result, component in
result * componentBase + component
}

return String(normalizedBuildNumber)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing tests for build normalization

Medium Severity

normalizeBuildNumber adds non-trivial packing and fallback logic, but no unit or request-level tests cover it. That violates the REVIEWS.md rule that new or changed code must have corresponding tests, and leaves the dotted-version conversion unverified in CI.

Fix in CursorΒ Fix in Web

Reviewed by Cursor Bugbot for commit 29cfb3d. Configure here.


// MARK: - Private
static let shared = Updater()
private let session: URLSession
Expand Down
Loading