Copilot commented on code in PR #10710:
URL: https://github.com/apache/ozone/pull/10710#discussion_r3557165490
##########
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/fs/http/server/FSOperations.java:
##########
@@ -1793,9 +1784,9 @@ public FSCreateSnapshot(String path, String snapshotName)
{
@Override
public String execute(FileSystem fs) throws IOException {
Path snapshotPath = fs.createSnapshot(path, snapshotName);
- JSONObject json = toJSON(HttpFSConstants.HOME_DIR_JSON,
+ Map<String, Object> json = toJSON(HttpFSConstants.HOME_DIR_JSON,
snapshotPath.toString());
- return json.toJSONString().replaceAll("\\\\", "");
+ return JsonUtil.toJsonString(json).replaceAll("\\\\", "");
Review Comment:
Using `replaceAll("\\\\", "")` removes *all* backslashes from the serialized
JSON, which can corrupt JSON escaping (eg values containing `\"`, `\n`, `\\`)
and potentially produce invalid JSON. If the goal is only to undo optional
forward-slash escaping (`\/`), restrict the replacement to that sequence (and
avoid regex).
##########
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/lib/service/instrumentation/InstrumentationService.java:
##########
@@ -295,7 +281,7 @@ public void addCron(String group, String name,
Instrumentation.Cron cron) {
timer.addCron((Cron) cron);
}
- static class VariableHolder<E> implements JSONAware, JSONStreamAware {
+ static class VariableHolder<E> {
// Supressed, because it is only used in this class or in test files,
// but the tests will be removed later.
Review Comment:
The comment here has a spelling error ("Supressed") and is now misleading:
the visibility suppression explicitly mentions tests being removed later, but
tests exist for this type (eg `TestInstrumentationSerialization`) and there’s
nothing in this change indicating they are temporary. Please update the comment
to match current intent.
--
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]
