Skip to content

Commit 9a4955a

Browse files
committed
Canonicalize option values as numeric types before DSL generation
Parse k as integer, max_distance and min_score as double before they reach buildKnnQuery(). Rejects non-numeric and non-finite values with clear errors. This closes the residual JSON-injection path through option values without requiring full XContent migration. Also fixes toString() to be consistent with the named-arg guard (no longer blindly casts to NamedArgumentExpression). Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 63253e3 commit 9a4955a

2 files changed

Lines changed: 72 additions & 5 deletions

File tree

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/VectorSearchTableFunctionImplementation.java

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,13 @@ public String toString() {
7575
List<String> args =
7676
arguments.stream()
7777
.map(
78-
arg ->
79-
String.format(
80-
"%s=%s",
81-
((NamedArgumentExpression) arg).getArgName(),
82-
((NamedArgumentExpression) arg).getValue().toString()))
78+
arg -> {
79+
if (arg instanceof NamedArgumentExpression) {
80+
NamedArgumentExpression named = (NamedArgumentExpression) arg;
81+
return String.format("%s=%s", named.getArgName(), named.getValue().toString());
82+
}
83+
return arg.toString();
84+
})
8385
.collect(Collectors.toList());
8486
return String.format("%s(%s)", functionName, String.join(", ", args));
8587
}
@@ -147,6 +149,10 @@ private void validateFieldName(String fieldName) {
147149
}
148150
}
149151

152+
/**
153+
* Validates and canonicalizes option values. All P0 option values must be numeric. Parsing them
154+
* here prevents non-numeric strings from reaching the raw JSON construction in buildKnnQuery().
155+
*/
150156
private void validateOptions(Map<String, String> options) {
151157
// Reject unknown option keys — only P0 keys are allowed
152158
for (String key : options.keySet()) {
@@ -162,6 +168,40 @@ private void validateOptions(Map<String, String> options) {
162168
throw new ExpressionEvaluationException(
163169
"Missing required option: one of k, max_distance, or min_score");
164170
}
171+
// Parse and canonicalize numeric values — closes JSON injection via option values
172+
if (hasK) {
173+
parseIntOption(options, "k");
174+
}
175+
if (hasMaxDistance) {
176+
parseDoubleOption(options, "max_distance");
177+
}
178+
if (hasMinScore) {
179+
parseDoubleOption(options, "min_score");
180+
}
181+
}
182+
183+
private void parseIntOption(Map<String, String> options, String key) {
184+
try {
185+
int value = Integer.parseInt(options.get(key));
186+
options.put(key, Integer.toString(value));
187+
} catch (NumberFormatException e) {
188+
throw new ExpressionEvaluationException(
189+
String.format("Option '%s' must be an integer, got '%s'", key, options.get(key)));
190+
}
191+
}
192+
193+
private void parseDoubleOption(Map<String, String> options, String key) {
194+
try {
195+
double value = Double.parseDouble(options.get(key));
196+
if (!Double.isFinite(value)) {
197+
throw new ExpressionEvaluationException(
198+
String.format("Option '%s' must be a finite number, got '%s'", key, options.get(key)));
199+
}
200+
options.put(key, Double.toString(value));
201+
} catch (NumberFormatException e) {
202+
throw new ExpressionEvaluationException(
203+
String.format("Option '%s' must be a number, got '%s'", key, options.get(key)));
204+
}
165205
}
166206

167207
private String getArgumentValue(String name) {

opensearch/src/test/java/org/opensearch/sql/opensearch/storage/VectorSearchTableFunctionImplementationTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,33 @@ void testNestedFieldNameAllowed() {
152152
assertTrue(table instanceof VectorSearchIndex);
153153
}
154154

155+
@Test
156+
void testNonNumericKThrows() {
157+
VectorSearchTableFunctionImplementation impl =
158+
createImplWithArgs("my-index", "embedding", "[1.0, 2.0]", "k=abc");
159+
ExpressionEvaluationException ex =
160+
assertThrows(ExpressionEvaluationException.class, () -> impl.applyArguments());
161+
assertTrue(ex.getMessage().contains("must be an integer"));
162+
}
163+
164+
@Test
165+
void testNonNumericMaxDistanceThrows() {
166+
VectorSearchTableFunctionImplementation impl =
167+
createImplWithArgs("my-index", "embedding", "[1.0, 2.0]", "max_distance=notanumber");
168+
ExpressionEvaluationException ex =
169+
assertThrows(ExpressionEvaluationException.class, () -> impl.applyArguments());
170+
assertTrue(ex.getMessage().contains("must be a number"));
171+
}
172+
173+
@Test
174+
void testInfiniteMinScoreThrows() {
175+
VectorSearchTableFunctionImplementation impl =
176+
createImplWithArgs("my-index", "embedding", "[1.0, 2.0]", "min_score=Infinity");
177+
ExpressionEvaluationException ex =
178+
assertThrows(ExpressionEvaluationException.class, () -> impl.applyArguments());
179+
assertTrue(ex.getMessage().contains("must be a finite number"));
180+
}
181+
155182
@Test
156183
void testNonNamedArgThrows() {
157184
FunctionName functionName = FunctionName.of("vectorsearch");

0 commit comments

Comments
 (0)