Skip to content

Commit ca8b666

Browse files
robertvokacrobertvokac
committed
fix(console): seven doors reject invalid arguments (#2166)
#2165 pinned all of this as behaviour it deliberately did not change, because ".NET's answer for it is recollection rather than measurement". /rv supplies the measurement, and it CORRECTS THE FRAMING on the most important row. THE CURSOR BOUND IS NOT A BELIEF ABOUT A PLATFORM LAYER. .NET validates a cursor coordinate in Console.cs ITSELF, before dispatch: // Basic argument validation. The PAL implementation may provide further validation. if (left < 0 || left >= short.MaxValue) throw new ArgumentOutOfRangeException(nameof(left), left, SR.ArgumentOutOfRange_ConsoleBufferBoundaries); -- Console.cs:550-559 so it applies on every platform, and the comparison is `>=`: 32766 is the last accepted column and 32767 is the FIRST REJECTED one. That off-by-one is the easy mistake and both sides of it are pinned. THE OTHER SIX ARE A DIFFERENT CASE, AND THE DIFFERENCE IS STATED RATHER THAN GLOSSED. .NET's UNIX pal throws PlatformNotSupportedException for every one of them -- CursorSize's setter (ConsolePal.Unix.cs:193-197), SetWindowSize (:387-394), SetWindowPosition (:744-747), SetBufferSize (:727-730), the buffer setters (:305-315) and MoveBufferArea (:717-725). So on this port's runtime platform .NET states NO RANGE for them at all. The ranges adopted are .NET's WINDOWS pal's, because they are the only ones .NET defines. Copying the Unix answer would REMOVE A FEATURE THIS PORT OFFERS rather than repair one -- SetWindowSize and SetWindowPosition really do emit xterm escape sequences here -- which is the wrong direction for a validation ticket. WHAT IS DELIBERATELY NOT REPRODUCED. Several of those Windows checks are buffer-relative: SetBufferSize's lower bound is the current window's right edge, SetWindowPosition's upper bound is the buffer width, MoveBufferArea's bounds are the buffer's dimensions. This port has no buffer geometry to compare against, so it enforces the half of each check that needs none -- the sign and the short.MaxValue ceiling. That is a subset of .NET's rejection, never a superset: nothing .NET accepts is refused here. Every rejection carries .NET's parameter name, the offending value as the actual value, and .NET's exact resource text (ConsoleBufferBoundaries, CursorSize, ConsoleWindowPos, ConsoleBufferLessThanWindowSize). This is what #2163 and #2164 could not do: their exception TYPES were probe-verified and their TEXTS were not, and the ticket recorded that the texts rode along on this one. A rejected CursorSize is not stored, so a caller reads back its previous value rather than the invalid one. Six mutations caught: the cursor ceiling becomes inclusive; the cursor size upper bound is dropped; store the cursor size before validating; MoveBufferArea checks only its first argument; SetWindowSize accepts zero; the buffer extent lower bound becomes 0. Two of those were invalid as first written -- one a no-op, one rejected by -Werror -- and a third was MASKED, because changing only SetWindowSize's width check left height rejecting (0,0) anyway. All three were reformulated rather than counted as passes. ALSO REPAIRED: a flake this session introduced. #2105's unpaired-rename test assumed the two events share one inotify batch; it passed three times in isolation and then FAILED inside this ticket's gate run, where the machine is busy enough to split the batch. It now counts only deliveries arriving AFTER the stop, which is the property under test and cannot fail when the batch splits. A test that is intermittently green is not evidence (#2352). Three pins inverted in place, three cases replacing three, so the count does not move. Downstream: neither cna nor mobile-eggbert calls any of these doors -- zero sites in both. Gate: 17,323 run, 17,323 passed, 0 failed, 0 skipped across 38 executables, GREEN, confirmed by two consecutive full runs. docs/Migration-ConsoleArgumentDomains.md
1 parent 997dd22 commit ca8b666

6 files changed

Lines changed: 361 additions & 47 deletions

File tree

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎docs/Migration-ConsoleArgumentDomains.md‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — seven `Console` doors now reject invalid arguments (ticket #2166)
5+
6+
*2026-08-18.* `SetCursorPosition`, `CursorSize`, `SetWindowSize`, `SetWindowPosition`,
7+
`SetBufferSize`, the two buffer-extent setters and both `MoveBufferArea` overloads now raise
8+
`ArgumentOutOfRangeException` for arguments they used to accept silently.
9+
10+
Landed under `docs/StandingApprovals.md` SA-5. No signature, layout or `noexcept` change.
11+
12+
---
13+
14+
## 1. What changed
15+
16+
| Call | Was | Is |
17+
|---|---|---|
18+
| `SetCursorPosition(32767, 0)` | accepted | `ArgumentOutOfRangeException("left")` |
19+
| `SetCursorPosition(INT_MAX, 0)` | accepted | rejected |
20+
| `SetCursorPosition(32766, 32766)` | accepted | **accepted** — the bound is exclusive |
21+
| `setCursorSizeProperty(0)`, `(-1)`, `(101)` | stored and read back | `ArgumentOutOfRangeException("value")` |
22+
| `SetWindowSize(-1,-1)`, `(0,0)` | accepted | rejected |
23+
| `SetWindowPosition(-1,-1)` | accepted | rejected |
24+
| `SetBufferSize(-1,-1)`, `(0,1)`, `(32767,1)` | accepted | rejected |
25+
| `setBufferWidthProperty(-1)`, `setBufferHeightProperty(-1)` | accepted | rejected |
26+
| `MoveBufferArea(-1,…)` — any of the six | accepted | rejected, each naming itself |
27+
| any valid argument | — | **unchanged** |
28+
29+
## 2. The cursor bound is not a belief about a platform layer
30+
31+
#2165 pinned all of this as *"behaviour this review deliberately did NOT change, because .NET's
32+
answer for it is recollection rather than measurement"*. The reference supplies the measurement,
33+
and it corrects the framing on the most important row:
34+
35+
```csharp
36+
public static void SetCursorPosition(int left, int top)
37+
{
38+
// Basic argument validation. The PAL implementation may provide further validation.
39+
if (left < 0 || left >= short.MaxValue)
40+
throw new ArgumentOutOfRangeException(nameof(left), left, SR.ArgumentOutOfRange_ConsoleBufferBoundaries);
41+
if (top < 0 || top >= short.MaxValue)
42+
throw new ArgumentOutOfRangeException(nameof(top), top, SR.ArgumentOutOfRange_ConsoleBufferBoundaries);
43+
ConsolePal.SetCursorPosition(left, top);
44+
}
45+
```
46+
*(`Console.cs:550-559`.)*
47+
48+
That is in `Console.cs` itself, **before** dispatch, so it applies on every platform. The
49+
comparison is `>=`, so **32766 is the last accepted column and 32767 is the first rejected one** —
50+
the off-by-one that is easy to get wrong, and both sides of it are pinned.
51+
52+
## 3. The other six are a different case, and the difference is stated rather than glossed
53+
54+
**.NET's Unix pal throws `PlatformNotSupportedException` for every one of them**: `CursorSize`'s
55+
setter (`ConsolePal.Unix.cs:193-197`), `SetWindowSize` (`:387-394`), `SetWindowPosition`
56+
(`:744-747`), `SetBufferSize` (`:727-730`), the buffer setters (`:305-315`) and `MoveBufferArea`
57+
(`:717-725`). So on this port's runtime platform .NET states **no range for them at all**.
58+
59+
The ranges adopted are .NET's **Windows** pal's, because they are the only ones .NET defines:
60+
61+
| Door | Check | .NET |
62+
|---|---|---|
63+
| `CursorSize` | `[1, 100]` | `ConsolePal.Windows.cs:588-590` |
64+
| `SetWindowSize` | both `> 0` | `:1020-1021` |
65+
| `SetWindowPosition` | both `>= 0` | `:995-1001` |
66+
| `SetBufferSize`, buffer setters | `[1, short.MaxValue)` | `:897-901` |
67+
| `MoveBufferArea` | all six `[0, short.MaxValue)` | `:735-751` |
68+
69+
**Refusing outright — copying .NET's Unix answer — would remove a feature this port offers rather
70+
than repair one**, which is the wrong direction for a validation ticket. `SetWindowSize` and
71+
`SetWindowPosition` really do emit xterm escape sequences here.
72+
73+
## 4. What is deliberately not reproduced
74+
75+
Several of .NET's Windows checks are **buffer-relative**: `SetBufferSize`'s lower bound is the
76+
current window's right edge, `SetWindowPosition`'s upper bound is the buffer's width, and
77+
`MoveBufferArea`'s bounds are the buffer's dimensions. This port has no buffer geometry to compare
78+
against, so it enforces the half of each check that needs none — the sign and the `short.MaxValue`
79+
ceiling. That is a subset of .NET's rejection, never a superset: nothing .NET accepts is refused.
80+
81+
## 5. The exception identity
82+
83+
Every rejection is `ArgumentOutOfRangeException` with .NET's parameter name, the offending value
84+
as the actual value (so the composed message carries `Actual value was N.`) and .NET's exact
85+
resource text:
86+
87+
* `ArgumentOutOfRange_ConsoleBufferBoundaries` — *"The value must be greater than or equal to zero
88+
and less than the console's buffer size in that dimension."*
89+
* `ArgumentOutOfRange_CursorSize` — *"The cursor size is invalid. It must be a percentage between
90+
1 and 100."*
91+
* `ArgumentOutOfRange_ConsoleWindowPos` — *"The window position must be set such that the current
92+
window size fits within the console's buffer, and the numbers must not be negative."*
93+
* `ArgumentOutOfRange_ConsoleBufferLessThanWindowSize` — *"The console buffer size must not be less
94+
than the current size and position of the console window, nor greater than or equal to
95+
short.MaxValue."*
96+
97+
This is what #2163 and #2164 could not do: their exception *types* were probe-verified and their
98+
*texts* were not, and the ticket recorded that the texts rode along on this one.
99+
100+
## 6. To migrate
101+
102+
Clamp before calling, or catch. A rejected `CursorSize` is **not stored**, so a caller that reads
103+
the property back gets its previous value rather than the invalid one.
104+
105+
## 7. Evidence
106+
107+
| Mutation | Caught |
108+
|---|---|
109+
| The cursor ceiling becomes inclusive (the off-by-one) | ✅ |
110+
| The cursor size upper bound is dropped | ✅ |
111+
| Store the cursor size **before** validating | ✅ |
112+
| `MoveBufferArea` checks only its first argument | ✅ |
113+
| `SetWindowSize` accepts zero (both parameters) | ✅ |
114+
| The buffer extent lower bound becomes 0 instead of 1 | ✅ |
115+
116+
Two mutations were invalid as first written and were reformulated rather than counted: one was a
117+
no-op (`(void)0;` before an unchanged store) and one was rejected by `-Werror` as an unused
118+
parameter. A third was **masked** — changing only `SetWindowSize`'s `width` check left `height`
119+
rejecting `(0,0)` anyway — so it had to change both.
120+
121+
## 8. Downstream
122+
123+
Neither `cna` nor `mobile-eggbert` calls any of these doors — zero sites in both.

‎modules/console/include/System/Console.hpp‎

Lines changed: 118 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -392,24 +392,80 @@ namespace System {
392392
// Cursor / Window
393393
// -----------------------------------------------------------------------
394394

395+
/**
396+
* @brief The console's coordinate ceiling, `short.MaxValue`, EXCLUSIVE.
397+
*
398+
* Ticket #2166 (2026-08-18). .NET validates a cursor coordinate in `Console.cs` itself,
399+
* before it reaches any platform layer — `if (left < 0 || left >= short.MaxValue)`
400+
* (`Console.cs:553-556`) — so this bound is platform-independent and is the one thing
401+
* about these doors that .NET states unconditionally. `SetCursorPosition(32767, 0)` is
402+
* therefore a rejection, not an acceptance: the comparison is `>=`.
403+
*/
404+
static constexpr intcs kConsoleCoordinateCeiling = 32767;
405+
406+
/** @brief .NET's `SR.ArgumentOutOfRange_ConsoleBufferBoundaries` (Strings.resx:200). */
407+
static constexpr const char* kBufferBoundariesMessage =
408+
"The value must be greater than or equal to zero and less than the console's buffer "
409+
"size in that dimension.";
410+
411+
/** @brief .NET's `SR.ArgumentOutOfRange_ConsoleWindowPos` (Strings.resx:203). */
412+
static constexpr const char* kWindowPositionMessage =
413+
"The window position must be set such that the current window size fits within the "
414+
"console's buffer, and the numbers must not be negative.";
415+
416+
/** @brief .NET's `SR.ArgumentOutOfRange_ConsoleBufferLessThanWindowSize` (Strings.resx:209). */
417+
static constexpr const char* kBufferExtentMessage =
418+
"The console buffer size must not be less than the current size and position of the "
419+
"console window, nor greater than or equal to short.MaxValue.";
420+
421+
/**
422+
* @brief Rejects a buffer extent outside `[1, short.MaxValue)`.
423+
*
424+
* .NET's Windows pal checks `width < srWindow.Right + 1 || width >= short.MaxValue`
425+
* (`ConsolePal.Windows.cs:897-901`). The lower half is window-relative and this port has
426+
* no window geometry to compare against, so it enforces the part that needs none: an
427+
* extent must be at least one column and below `short.MaxValue`.
428+
*/
429+
static void throwIfInvalidBufferExtent(intcs value, const char* paramName) {
430+
if (value < 1 || value >= kConsoleCoordinateCeiling) {
431+
throw System::ArgumentOutOfRangeException(paramName, std::to_string(value),
432+
kBufferExtentMessage);
433+
}
434+
}
435+
436+
/**
437+
* @brief Rejects a coordinate outside `[0, short.MaxValue)`.
438+
*
439+
* .NET passes the offending value as the exception's actual value, so the composed
440+
* message carries an `Actual value was N.` clause. Reproduced.
441+
*/
442+
static void throwIfOutsideBufferBoundaries(intcs value, const char* paramName) {
443+
if (value < 0 || value >= kConsoleCoordinateCeiling) {
444+
throw System::ArgumentOutOfRangeException(paramName, std::to_string(value),
445+
kBufferBoundariesMessage);
446+
}
447+
}
448+
395449
/**
396450
* @brief Sets the cursor position using an ANSI escape sequence (0-based).
397451
* @param left Column index (0-based).
398452
* @param top Row index (0-based).
399-
* @throws System::ArgumentOutOfRangeException if @p left or @p top is negative. The
400-
* coordinate is rejected before the cache is written or anything is emitted.
453+
* @throws System::ArgumentOutOfRangeException if @p left or @p top is outside
454+
* `[0, short.MaxValue)`. The coordinate is rejected before the cache is written
455+
* or anything is emitted.
401456
*
402-
* @note **No upper bound is enforced**, deliberately. Current .NET is believed to reject a
403-
* coordinate at or above `short.MaxValue`, but the audit measured only the negative case
404-
* and the `/rv` reference tree is absent here, so `SetCursorPosition(32767, 0)` and
405-
* `(INT_MAX, 0)` are still accepted and are pinned as such by test. Ticket #2166 owns
406-
* the question.
457+
* @note **Ticket #2166 (2026-08-18) added the upper bound.** #2164 enforced only the
458+
* negative case and recorded that the .NET limit was "believed" but unmeasured. It is
459+
* measured now, and it is not a belief about a platform layer: .NET validates in
460+
* `Console.cs` itself, before dispatch (`Console.cs:550-558`), so the bound applies on
461+
* every platform. `SetCursorPosition(32767, 0)` is a **rejection**, because the
462+
* comparison is `>=`.
407463
*/
408464
static void SetCursorPosition(intcs left, intcs top) {
409465
// The cache is a documented local reduction, so an invalid coordinate would survive in
410466
// the process even where the terminal ignores the sequence it produces (SR-AUD-244).
411-
System::ArgumentOutOfRangeException::ThrowIfNegative(left, "left");
412-
System::ArgumentOutOfRangeException::ThrowIfNegative(top, "top");
467+
throwIfOutsideBufferBoundaries(left, "left");
468+
throwIfOutsideBufferBoundaries(top, "top");
413469
cursorLeft_ = left;
414470
cursorTop_ = top;
415471
// The +1 converts 0-based to the ANSI 1-based convention, and it is computed in a wider
@@ -477,8 +533,29 @@ namespace System {
477533
* @return The stored cursor size (default 25).
478534
*/
479535
[[nodiscard]] static intcs getCursorSizeProperty() { return cursorSize_; }
480-
/** @brief Sets the cursor size (stored; not visually applied in this implementation). */
481-
static void setCursorSizeProperty(intcs v) { cursorSize_ = v; }
536+
/**
537+
* @brief Sets the cursor size, as a percentage in `[1, 100]`.
538+
*
539+
* Ticket #2166. The value used to be stored unchecked against a domain the doc-comment
540+
* itself declared, so `0`, `-1` and `101` were all readable back.
541+
*
542+
* The range and the message are .NET's Windows pal
543+
* (`ConsolePal.Windows.cs:588-590`, `SR.ArgumentOutOfRange_CursorSize`). **On Unix .NET's
544+
* setter throws `PlatformNotSupportedException` outright** (`ConsolePal.Unix.cs:193-197`),
545+
* so it defines no range there at all — this port keeps the property working and borrows
546+
* the only range .NET states. Refusing outright would remove a feature this port offers
547+
* rather than repair one, which is the wrong direction for a validation ticket.
548+
*
549+
* @throws System::ArgumentOutOfRangeException if @p v is outside `[1, 100]`.
550+
*/
551+
static void setCursorSizeProperty(intcs v) {
552+
if (v < 1 || v > 100) {
553+
throw System::ArgumentOutOfRangeException(
554+
"value", std::to_string(v),
555+
"The cursor size is invalid. It must be a percentage between 1 and 100.");
556+
}
557+
cursorSize_ = v;
558+
}
482559

483560
/**
484561
* @brief Gets a value indicating whether the cursor is visible.
@@ -536,6 +613,11 @@ namespace System {
536613
* @param height New window height in rows.
537614
*/
538615
static void SetWindowSize(intcs width, intcs height) {
616+
// Ticket #2166. .NET's Windows pal opens with ThrowIfNegativeOrZero on both
617+
// (ConsolePal.Windows.cs:1020-1021); its Unix pal throws PlatformNotSupportedException
618+
// and states no range. A window of zero or negative columns is not a window.
619+
System::ArgumentOutOfRangeException::ThrowIfNegativeOrZero(width, "width");
620+
System::ArgumentOutOfRangeException::ThrowIfNegativeOrZero(height, "height");
539621
std::printf("\033[8;%d;%dt", static_cast<int>(height), static_cast<int>(width));
540622
std::fflush(stdout);
541623
}
@@ -546,6 +628,18 @@ namespace System {
546628
* @param top New top position in pixels.
547629
*/
548630
static void SetWindowPosition(intcs left, intcs top) {
631+
// Ticket #2166. .NET's Windows pal rejects a negative coordinate with
632+
// SR.ArgumentOutOfRange_ConsoleWindowPos (ConsolePal.Windows.cs:995-1001). The rest of
633+
// that check is buffer-relative and this port has no buffer geometry to check against,
634+
// so only the half that needs none is reproduced -- see the migration note.
635+
if (left < 0) {
636+
throw System::ArgumentOutOfRangeException("left", std::to_string(left),
637+
kWindowPositionMessage);
638+
}
639+
if (top < 0) {
640+
throw System::ArgumentOutOfRangeException("top", std::to_string(top),
641+
kWindowPositionMessage);
642+
}
549643
std::printf("\033[3;%d;%dt", static_cast<int>(top), static_cast<int>(left));
550644
std::fflush(stdout);
551645
}
@@ -569,28 +663,35 @@ namespace System {
569663
* @brief Sets the screen buffer width (no-op in this implementation).
570664
* @param v New buffer width.
571665
*/
572-
static void setBufferWidthProperty(intcs v) { (void)v; }
666+
static void setBufferWidthProperty(intcs v) { throwIfInvalidBufferExtent(v, "width"); }
573667
/**
574668
* @brief Sets the screen buffer height (no-op in this implementation).
575669
* @param v New buffer height.
576670
*/
577-
static void setBufferHeightProperty(intcs v) { (void)v; }
671+
static void setBufferHeightProperty(intcs v) { throwIfInvalidBufferExtent(v, "height"); }
578672

579673
/**
580674
* @brief Sets the screen buffer size (no-op in this implementation).
581675
* @param width New buffer width.
582676
* @param height New buffer height.
583677
*/
584-
static void SetBufferSize(intcs width, intcs height) { (void)width; (void)height; }
678+
static void SetBufferSize(intcs width, intcs height) {
679+
throwIfInvalidBufferExtent(width, "width");
680+
throwIfInvalidBufferExtent(height, "height");
681+
}
585682

586683
/**
587684
* @brief Copies a rectangular region of the buffer to another location (no-op stub).
588685
*/
589686
static void MoveBufferArea(intcs sourceLeft, intcs sourceTop,
590687
intcs sourceWidth, intcs sourceHeight,
591688
intcs targetLeft, intcs targetTop) {
592-
(void)sourceLeft; (void)sourceTop; (void)sourceWidth;
593-
(void)sourceHeight; (void)targetLeft; (void)targetTop;
689+
throwIfOutsideBufferBoundaries(sourceLeft, "sourceLeft");
690+
throwIfOutsideBufferBoundaries(sourceTop, "sourceTop");
691+
throwIfOutsideBufferBoundaries(sourceWidth, "sourceWidth");
692+
throwIfOutsideBufferBoundaries(sourceHeight, "sourceHeight");
693+
throwIfOutsideBufferBoundaries(targetLeft, "targetLeft");
694+
throwIfOutsideBufferBoundaries(targetTop, "targetTop");
594695
}
595696

596697
/**
@@ -602,8 +703,7 @@ namespace System {
602703
char sourceChar,
603704
ConsoleColor sourceForeColor,
604705
ConsoleColor sourceBackColor) {
605-
(void)sourceLeft; (void)sourceTop; (void)sourceWidth;
606-
(void)sourceHeight; (void)targetLeft; (void)targetTop;
706+
MoveBufferArea(sourceLeft, sourceTop, sourceWidth, sourceHeight, targetLeft, targetTop);
607707
(void)sourceChar; (void)sourceForeColor; (void)sourceBackColor;
608708
}
609709

0 commit comments

Comments
 (0)