xiangfu0 commented on code in PR #18411:
URL: https://github.com/apache/pinot/pull/18411#discussion_r3253006218
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -837,9 +873,11 @@ public ObjectNode checkTableConfig(String tableConfigStr,
@QueryParam("validationTypesToSkip") @Nullable String typesToSkip,
@Context HttpHeaders httpHeaders,
@Context Request request) {
Pair<TableConfig, Map<String, Object>>
tableConfigAndUnrecognizedProperties;
+ JsonNode tableConfigJson;
try {
tableConfigAndUnrecognizedProperties =
JsonUtils.stringToObjectAndUnrecognizedProperties(tableConfigStr,
TableConfig.class);
+ tableConfigJson = JsonUtils.stringToJsonNode(tableConfigStr);
} catch (IOException e) {
String msg = String.format("Invalid table config json string: %s.
Reason: %s", tableConfigStr, e.getMessage());
Review Comment:
Fixed in 05def210a0: scrubbed the raw tableConfigStr from both the 400
response and the controller log. The catch now uses concatenation (`"Invalid
table config json string. " + e.getMessage()`), matching the addTable/copyTable
scrub pattern already in this file.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -461,19 +493,35 @@ public String validateConfig(String tableConfigsStr,
@QueryParam("validationTypesToSkip") @Nullable String typesToSkip,
@Context HttpHeaders httpHeaders,
@Context Request request) {
Pair<TableConfigs, Map<String, Object>> tableConfigsAndUnrecognizedProps;
+ JsonNode tableConfigsJson;
try {
tableConfigsAndUnrecognizedProps =
JsonUtils.stringToObjectAndUnrecognizedProperties(tableConfigsStr,
TableConfigs.class);
+ tableConfigsJson = JsonUtils.stringToJsonNode(tableConfigsStr);
} catch (IOException e) {
throw new ControllerApplicationException(LOGGER,
String.format("Invalid TableConfigs json string: %s. Reason: %s",
tableConfigsStr, e.getMessage()),
Review Comment:
Fixed in 05def210a0: switched to `"Invalid TableConfigs json string. " +
e.getMessage()` to match `addConfig`. Also reordered `/tableConfigs/validate`
so the permission check runs BEFORE the ZK-reading validateNoDeprecatedConfigs
path, mirroring PinotTableRestletResource.checkTableConfig and preventing an
unauthenticated caller from probing table existence.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
