Skip to content

Commit d52a9da

Browse files
committed
fix(config): reject over-int connect_timeout instead of wrapping through (int) cast
Both ws config paths read connect_timeout via view.getLong and cast to int: connect_timeout=4294967297 silently became a 1 ms timeout, and 2147483648 wrapped to Integer.MIN_VALUE, producing the misleading 'connect_timeout must be > 0: -2147483648'. - ConfigView.getInt now parses in the long domain: schema range still reports first (tighter, key-specific), then an over-int value fails with the actual limit ('connect_timeout must be <= 2147483647: ...') instead of a generic 'invalid' or a wrapped value. - Sender.fromConfigWebSocket and QwpQueryClient.fromConfig/validateConfig read connect_timeout via getInt, aligning with the legacy http/tcp parseIntValue path that already rejected over-int values. Covered at ConfigView level (ConfigViewTest) and end-to-end on both facades (QwpConfigKeysTest.testConnectTimeoutOverIntRejectedOnBoth).
1 parent 9d5451c commit d52a9da

5 files changed

Lines changed: 67 additions & 6 deletions

File tree

core/src/main/java/io/questdb/client/Sender.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3599,7 +3599,9 @@ private LineSenderBuilder fromConfigWebSocket(CharSequence configurationString)
35993599
authTimeoutMillis(view.getLong("auth_timeout_ms", 0));
36003600
}
36013601
if (view.has("connect_timeout")) {
3602-
connectTimeoutMillis((int) view.getLong("connect_timeout", 0));
3602+
// getInt (not getLong + cast): connectTimeoutMillis takes an
3603+
// int, and an over-int value must reject, not wrap.
3604+
connectTimeoutMillis(view.getInt("connect_timeout", 0));
36033605
}
36043606

36053607
s = view.getStr("auto_flush_rows");

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpQueryClient.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,9 @@ public static QwpQueryClient fromConfig(CharSequence configurationString) {
390390
Long failoverMaxDurationMs = view.has("failover_max_duration_ms")
391391
? view.getLong("failover_max_duration_ms", 0) : null;
392392
Long authTimeoutMs = view.has("auth_timeout_ms") ? view.getLong("auth_timeout_ms", 0) : null;
393-
Integer connectTimeout = view.has("connect_timeout") ? (int) view.getLong("connect_timeout", 0) : null;
393+
// getInt (not getLong + cast): withConnectTimeout takes an int, and an
394+
// over-int value must reject, not wrap.
395+
Integer connectTimeout = view.has("connect_timeout") ? view.getInt("connect_timeout", 0) : null;
394396
Long initialCredit = view.has("initial_credit") ? view.getLong("initial_credit", 0) : null;
395397
int poolSize = view.getInt("buffer_pool_size", DEFAULT_IO_BUFFER_POOL_SIZE);
396398
String compression = view.getEnum("compression");
@@ -504,7 +506,9 @@ public static void validateConfig(ConfigView view, boolean tls) {
504506
view.getLong("failover_max_duration_ms", -1);
505507
view.getLong("initial_credit", -1);
506508
view.getLong("auth_timeout_ms", -1);
507-
view.getLong("connect_timeout", -1);
509+
// getInt: connect_timeout feeds an int API, so validation must also
510+
// reject values that fit a long but not an int.
511+
view.getInt("connect_timeout", -1);
508512
String username = view.getStr("username");
509513
String password = view.getStr("password");
510514
String token = view.getStr("token");

core/src/main/java/io/questdb/client/impl/ConfigView.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,17 +178,29 @@ public int getInt(String key, int unset) {
178178
if (v == null) {
179179
return unset;
180180
}
181-
int parsed;
181+
// Parse in the long domain so a numeric-but-over-int value (e.g.
182+
// 4294967297) is distinguishable from garbage: it must fail with the
183+
// actual int limit, not wrap through an (int) cast or report a generic
184+
// "invalid" for a perfectly numeric string.
185+
long parsed;
182186
try {
183-
parsed = Numbers.parseInt(v);
187+
parsed = Numbers.parseLong(v);
184188
} catch (NumericException e) {
185189
throw new IllegalArgumentException("invalid " + key + ": " + v);
186190
}
187191
ConfigSchema.KeySpec spec = ConfigSchema.spec(key);
192+
// Schema range first: it is the tighter, key-specific constraint, so
193+
// "compression_level=4294967297" says "must be in [1, 22]".
188194
if (outOfRange(spec, parsed)) {
189195
throw new IllegalArgumentException(rangeMessage(spec));
190196
}
191-
return parsed;
197+
if (parsed > Integer.MAX_VALUE) {
198+
throw new IllegalArgumentException(key + " must be <= " + Integer.MAX_VALUE + ": " + v);
199+
}
200+
if (parsed < Integer.MIN_VALUE) {
201+
throw new IllegalArgumentException(key + " must be >= " + Integer.MIN_VALUE + ": " + v);
202+
}
203+
return (int) parsed;
192204
}
193205

194206
public long getLong(String key, long unset) {

core/src/test/java/io/questdb/client/test/impl/ConfigViewTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,29 @@ public void testGetLongStrictLowerBound() {
115115
"auth_timeout_ms must be > 0");
116116
}
117117

118+
@Test
119+
public void testGetIntOverIntRangeRejectedActionably() {
120+
// An int key fed a numeric value beyond int range must name the real
121+
// limit -- not report a wrapped/garbage value. 2^32+1 would wrap to 1
122+
// (a silent 1 ms timeout) through a bare (int) cast; 2^31 would wrap
123+
// to Integer.MIN_VALUE and produce a misleading "must be > 0" error.
124+
assertParseError("ws::addr=h:9000;connect_timeout=4294967297;",
125+
v -> v.getInt("connect_timeout", -1),
126+
"connect_timeout must be <= 2147483647: 4294967297");
127+
assertParseError("ws::addr=h:9000;connect_timeout=2147483648;",
128+
v -> v.getInt("connect_timeout", -1),
129+
"connect_timeout must be <= 2147483647: 2147483648");
130+
}
131+
132+
@Test
133+
public void testGetIntSpecRangeStillWinsOverIntBound() {
134+
// A value that is both over the schema max and over int range reports
135+
// the schema range -- the tighter, key-specific constraint.
136+
assertParseError("ws::addr=h:9000;compression_level=4294967297;",
137+
v -> v.getInt("compression_level", -1),
138+
"compression_level must be in [1, 22]");
139+
}
140+
118141
@Test
119142
public void testGetIntNonNumericRejected() {
120143
assertParseError("ws::addr=h:9000;compression_level=abc;",

core/src/test/java/io/questdb/client/test/impl/QwpConfigKeysTest.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,26 @@ public void testEverySchemaKeyIsRecognizedByBothClients() throws Exception {
5454
});
5555
}
5656

57+
@Test
58+
public void testConnectTimeoutOverIntRejectedOnBoth() throws Exception {
59+
TestUtils.assertMemoryLeak(() -> {
60+
// connect_timeout flows into an int API on both clients. A bare
61+
// (int) cast would silently wrap 2^32+1 to a 1 ms timeout and turn
62+
// 2^31 into a misleading "must be > 0: -2147483648" error. Both
63+
// must reject up front, naming the actual limit.
64+
assertRejected("ws::addr=h:9000;connect_timeout=4294967297;",
65+
"connect_timeout must be <= 2147483647: 4294967297");
66+
assertRejected("ws::addr=h:9000;connect_timeout=2147483648;",
67+
"connect_timeout must be <= 2147483647: 2147483648");
68+
// The schema lower bound is intact end-to-end...
69+
assertRejected("ws::addr=h:9000;connect_timeout=0;",
70+
"connect_timeout must be > 0");
71+
// ...and a valid value still builds on both clients.
72+
Sender.builder("ws::addr=h:9000;connect_timeout=7000;");
73+
QwpQueryClient.fromConfig("ws::addr=h:9000;connect_timeout=7000;").close();
74+
});
75+
}
76+
5777
@Test
5878
public void testJunkKeyRejectedOnBoth() throws Exception {
5979
TestUtils.assertMemoryLeak(() -> {

0 commit comments

Comments
 (0)