Skip to content

Commit d8db1fe

Browse files
committed
Avoid including raw content in AbstractPushCommand.toString()
Motivation: Some push requests may contain large content exceeding 1MB. Including such large values in `AbstractPushCommand.toString()` is risky. It could overwhelm the logging buffer or even cause memory leaks if cached. Modifications: - Include only contentLength when generating a string representation of a push command. Result: `*PushCommand.toString()` no longer includes raw content.
1 parent 923af4f commit d8db1fe

2 files changed

Lines changed: 70 additions & 1 deletion

File tree

server/src/main/java/com/linecorp/centraldogma/server/command/AbstractPushCommand.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,28 @@ public int hashCode() {
122122

123123
@Override
124124
ToStringHelper toStringHelper() {
125+
// Build a summary of changes to avoid overly long toString() result.
126+
final StringBuilder changesBuilder = new StringBuilder("[");
127+
for (int i = 0; i < changes.size(); i++) {
128+
final Change<?> change = changes.get(i);
129+
final String content = change.contentAsText();
130+
changesBuilder.append("{type: ").append(change.type())
131+
.append(", path: ").append(change.path())
132+
.append(", contentLength: ")
133+
.append(content != null ? content.length() : 0)
134+
.append('}');
135+
if (i != changes.size() - 1) {
136+
changesBuilder.append(", ");
137+
}
138+
}
139+
changesBuilder.append(']');
140+
125141
return super.toStringHelper()
126142
.add("baseRevision", baseRevision)
127143
.add("summary", summary)
128144
.add("detail", detail)
129145
.add("markup", markup)
130-
.add("changes", changes);
146+
.add("numChanges", changes.size())
147+
.add("changes", changesBuilder.toString());
131148
}
132149
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright 2025 LY Corporation
3+
*
4+
* LY Corporation licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
17+
package com.linecorp.centraldogma.server.command;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import java.util.List;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
import com.fasterxml.jackson.databind.JsonNode;
26+
import com.google.common.collect.ImmutableList;
27+
28+
import com.linecorp.centraldogma.common.Author;
29+
import com.linecorp.centraldogma.common.Change;
30+
import com.linecorp.centraldogma.common.Markup;
31+
import com.linecorp.centraldogma.common.Revision;
32+
33+
class PushCommandTest {
34+
35+
@Test
36+
void shouldNotContainRawContentInToString() {
37+
final Change<JsonNode> json = Change.ofJsonUpsert("/a.json", "{ \"foo\": \"bar\" }");
38+
final Change<String> text = Change.ofTextUpsert("/a.txt", "Hello");
39+
final Change<Void> removal = Change.ofRemoval("/b.txt");
40+
final List<Change<?>> changes = ImmutableList.of(json, text, removal);
41+
42+
final Command<CommitResult> pushCommand =
43+
Command.push(Author.SYSTEM, "myProject", "myRepo",
44+
Revision.HEAD,
45+
"summary", "detail",
46+
Markup.PLAINTEXT, changes);
47+
assertThat(pushCommand.toString()).contains(
48+
"changes=[{type: UPSERT_JSON, path: /a.json, contentLength: " + json.contentAsText().length() +
49+
"}, {type: UPSERT_TEXT, path: /a.txt, contentLength: " + text.contentAsText().length() +
50+
"}, {type: REMOVE, path: /b.txt, contentLength: 0}]");
51+
}
52+
}

0 commit comments

Comments
 (0)