Reject non-positive @PPI hints

Codex review flagged that a mistyped "@0" or "@-1" parsed to a non-nil
ppi that persisted and was fed straight into
VZMacGraphicsDisplayConfiguration, bypassing the 72 fallback and risking
a display that fails to build until the user manually resets it.

Guard both ends: the --display parser now keeps only a positive PPI
(zero/negative ignored just like non-numeric input), and
effectivePixelsPerInch treats a non-positive stored value (e.g. from a
hand-edited config) as unset, falling back to 72.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Eugene Petrenko 2026-07-28 15:49:28 +02:00
parent 521cc16e44
commit 60bf256345
3 changed files with 28 additions and 6 deletions

View File

@ -95,9 +95,11 @@ extension VMDisplayConfig: ExpressibleByArgument {
var ppi: Int? = nil
// Optional "@PPI" pixels-per-inch hint, e.g. "3200x1800px@220". Parsed
// before the unit suffix since it always trails the whole spec.
// before the unit suffix since it always trails the whole spec. Only a
// positive value is kept; zero/negative (or non-numeric) is ignored so the
// display falls back to the 72 default rather than a broken configuration.
if let atIndex = argument.lastIndex(of: "@") {
ppi = Int(argument[argument.index(after: atIndex)...])
ppi = Int(argument[argument.index(after: atIndex)...]).flatMap { $0 > 0 ? $0 : nil }
argument = String(argument[..<atIndex])
}

View File

@ -44,11 +44,16 @@ struct VMDisplayConfig: Codable, Equatable {
var ppi: Int?
// Pixels-per-inch handed to the guest's pixel-unit framebuffer. Defaults to
// 72 (non-Retina) when no hint is given, matching Virtualization.framework's
// historical assumption; a Retina-class value (e.g. 220) makes the guest
// expose a HiDPI (scaling:on) mode regardless of the host display.
// 72 (non-Retina) when no positive hint is given, matching
// Virtualization.framework's historical assumption; a Retina-class value
// (e.g. 220) makes the guest expose a HiDPI (scaling:on) mode regardless of
// the host display. A non-positive stored value (e.g. from a hand-edited
// config) is treated as unset so it never reaches the display configuration.
var effectivePixelsPerInch: Int {
ppi ?? 72
if let ppi, ppi > 0 {
return ppi
}
return 72
}
}

View File

@ -57,4 +57,19 @@ final class VMConfigTests: XCTestCase {
XCTAssertEqual(VMDisplayConfig(width: 3200, height: 1800, unit: .pixel, ppi: nil), config)
XCTAssertEqual(72, config.effectivePixelsPerInch)
}
func testRejectsNonPositivePixelsPerInchWhenParsing() throws {
// A zero or negative PPI is meaningless and must not persist it would
// otherwise reach VZMacGraphicsDisplayConfiguration and fail to build the
// display instead of falling back to the 72 default.
XCTAssertNil(VMDisplayConfig(argument: "3200x1800px@0").ppi)
XCTAssertNil(VMDisplayConfig(argument: "3200x1800px@-5").ppi)
}
func testEffectivePixelsPerInchIgnoresNonPositiveStoredValue() throws {
// Defense for a hand-edited config.json: a stored value <= 0 falls back to
// 72 rather than being handed to VZMacGraphicsDisplayConfiguration.
XCTAssertEqual(72, VMDisplayConfig(width: 100, height: 100, unit: .pixel, ppi: 0).effectivePixelsPerInch)
XCTAssertEqual(72, VMDisplayConfig(width: 100, height: 100, unit: .pixel, ppi: -5).effectivePixelsPerInch)
}
}