Skip to content

Commit 1290a2e

Browse files
intr3p1dintr3p1d
committed
[#noissue] Return bad request reason in response body
1 parent 363c650 commit 1290a2e

File tree

2 files changed

+64
-48
lines changed

2 files changed

+64
-48
lines changed

‎metric-module/metric/src/main/java/com/navercorp/pinpoint/metric/collector/controller/TelegrafMetricController.java‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,8 @@ public class TelegrafMetricController {
6060
private final TenantProvider tenantProvider;
6161

6262
private static final String[] ignoreTags = {"host"};
63-
private static final Pattern VALID_NAME = Pattern.compile("[a-zA-Z0-9._\\-]+");
64-
private static final Pattern CONTROL_CHARS = Pattern.compile("[\\p{Cntrl}]");
65-
private static final int MAX_LOG_LENGTH = 100;
63+
private static final Pattern VALID_HOST_NAME = Pattern.compile("[a-z]([a-z0-9\\-._]{0,254}[a-z0-9])?");
64+
private static final Pattern VALID_GROUP_NAME = Pattern.compile("[A-Za-z0-9][A-Za-z0-9.\\-_]*");
6665

6766
public TelegrafMetricController(SystemMetricService systemMetricService,
6867
SystemMetricDataTypeService systemMetricMetadataService,
@@ -76,33 +75,34 @@ public TelegrafMetricController(SystemMetricService systemMetricService,
7675

7776

7877
@PostMapping(value = "/telegraf")
79-
public ResponseEntity<Void> saveSystemMetric(
78+
public ResponseEntity<String> saveSystemMetric(
8079
@RequestHeader(value = "hostGroupName") String hostGroupName,
8180
@RequestBody TelegrafMetrics telegrafMetrics, BindingResult bindingResult
8281
) throws BindException {
8382
if (bindingResult.hasErrors()) {
8483
SimpleErrorMessage simpleErrorMessage = new SimpleErrorMessage(bindingResult);
85-
logger.warn("metric binding error. header=hostGroupName:{} errorCount:{} {}", sanitizeForLog(hostGroupName), bindingResult.getErrorCount(), simpleErrorMessage);
84+
logger.warn("metric binding error. header=hostGroupName:{} errorCount:{} {}", hostGroupName, bindingResult.getErrorCount(), simpleErrorMessage);
8685
throw new BindException(bindingResult);
8786
}
8887

89-
if (!VALID_NAME.matcher(hostGroupName).matches()) {
90-
logger.warn("invalid hostGroupName='{}'", sanitizeForLog(hostGroupName));
88+
if (!isValidGroupName(hostGroupName)) {
89+
logger.warn("invalid hostGroupName='{}'", hostGroupName);
90+
return ResponseEntity.badRequest().body("invalid hostGroupName='" + hostGroupName + "'");
9191
}
9292

9393
String hostName = getHost(telegrafMetrics);
9494
if (StringUtils.isEmpty(hostName)) {
9595
// hostname null check
96-
logger.info("hostName is empty. hostGroupName={}", sanitizeForLog(hostGroupName));
97-
return ResponseEntity.badRequest().build();
96+
logger.info("hostName is empty. hostGroupName={}", hostGroupName);
97+
return ResponseEntity.badRequest().body("hostName is empty");
9898
}
9999

100-
if (!VALID_NAME.matcher(hostName).matches()) {
101-
logger.warn("invalid hostName='{}', hostGroupName='{}'", sanitizeForLog(hostName), sanitizeForLog(hostGroupName));
100+
if (!isValidHostName(hostName)) {
101+
logger.warn("invalid hostName='{}', hostGroupName='{}'", hostName, hostGroupName);
102102
}
103103

104104
if (logger.isDebugEnabled()) {
105-
logger.debug("hostGroupName:{} host:{} size:{}", sanitizeForLog(hostGroupName), sanitizeForLog(hostName), telegrafMetrics.size());
105+
logger.debug("hostGroupName:{} host:{} size:{}", hostGroupName, hostName, telegrafMetrics.size());
106106
}
107107

108108
String tenantId = tenantProvider.getTenantId();
@@ -112,7 +112,7 @@ public ResponseEntity<Void> saveSystemMetric(
112112
updateMetadata(systemMetric);
113113
systemMetricService.insert(systemMetric);
114114

115-
return ResponseEntity.ok().build();
115+
return ResponseEntity.ok(null);
116116
}
117117

118118
private String getHost(TelegrafMetrics metrics) {
@@ -176,15 +176,12 @@ private Tag getHost(List<Tag> tTags) {
176176
return null;
177177
}
178178

179-
static String sanitizeForLog(String value) {
180-
if (value == null) {
181-
return null;
182-
}
183-
String sanitized = CONTROL_CHARS.matcher(value).replaceAll("_");
184-
if (sanitized.length() > MAX_LOG_LENGTH) {
185-
sanitized = sanitized.substring(0, MAX_LOG_LENGTH) + "...(truncated)";
186-
}
187-
return sanitized;
179+
static boolean isValidHostName(String value) {
180+
return VALID_HOST_NAME.matcher(value).matches();
181+
}
182+
183+
static boolean isValidGroupName(String value) {
184+
return VALID_GROUP_NAME.matcher(value).matches();
188185
}
189186

190187
static List<Tag> filterTag(List<Tag> tTags, String[] ignoreTagName) {

‎metric-module/metric/src/test/java/com/navercorp/pinpoint/metric/collector/controller/TelegrafMetricControllerTest.java‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,43 +10,62 @@
1010
class TelegrafMetricControllerTest {
1111

1212
@Test
13-
void filterTag() {
14-
List<Tag> tags = List.of(
15-
new Tag("tag1", "value1"),
16-
new Tag("host", "host1")
17-
);
18-
List<Tag> tags1 = TelegrafMetricController.filterTag(tags, new String[]{"host"});
19-
assertThat(tags1)
20-
.hasSize(1)
21-
.containsExactly(new Tag("tag1", "value1"));
13+
void isValidHostName_valid() {
14+
assertThat(TelegrafMetricController.isValidHostName("hostname")).isTrue();
15+
assertThat(TelegrafMetricController.isValidHostName("host-name")).isTrue();
16+
assertThat(TelegrafMetricController.isValidHostName("host1")).isTrue();
17+
assertThat(TelegrafMetricController.isValidHostName("a")).isTrue();
18+
assertThat(TelegrafMetricController.isValidHostName("a1")).isTrue();
19+
assertThat(TelegrafMetricController.isValidHostName("abc-123-def")).isTrue();
20+
assertThat(TelegrafMetricController.isValidHostName("host.name")).isTrue();
21+
assertThat(TelegrafMetricController.isValidHostName("host_name")).isTrue();
22+
}
23+
24+
@Test
25+
void isValidHostName_invalid() {
26+
assertThat(TelegrafMetricController.isValidHostName("-hostname")).isFalse();
27+
assertThat(TelegrafMetricController.isValidHostName("hostname-")).isFalse();
28+
assertThat(TelegrafMetricController.isValidHostName("1hostname")).isFalse();
29+
assertThat(TelegrafMetricController.isValidHostName("HostName")).isFalse();
30+
assertThat(TelegrafMetricController.isValidHostName("host name")).isFalse();
31+
assertThat(TelegrafMetricController.isValidHostName("")).isFalse();
2232
}
2333

2434
@Test
25-
void sanitizeForLog_null() {
26-
assertThat(TelegrafMetricController.sanitizeForLog(null)).isNull();
35+
void isValidHostName_exceedsMaxLabelLength() {
36+
String longName = "a".repeat(300);
37+
assertThat(TelegrafMetricController.isValidHostName(longName)).isFalse();
2738
}
2839

2940
@Test
30-
void sanitizeForLog_replaceControlChars() {
31-
assertThat(TelegrafMetricController.sanitizeForLog("host\ninjected"))
32-
.isEqualTo("host_injected");
33-
assertThat(TelegrafMetricController.sanitizeForLog("host\r\ninjected"))
34-
.isEqualTo("host__injected");
35-
assertThat(TelegrafMetricController.sanitizeForLog("normal-host_name.01"))
36-
.isEqualTo("normal-host_name.01");
41+
void isValidGroupName_valid() {
42+
assertThat(TelegrafMetricController.isValidGroupName("group1")).isTrue();
43+
assertThat(TelegrafMetricController.isValidGroupName("Group-Name")).isTrue();
44+
assertThat(TelegrafMetricController.isValidGroupName("group.name")).isTrue();
45+
assertThat(TelegrafMetricController.isValidGroupName("Group.Name-01")).isTrue();
46+
assertThat(TelegrafMetricController.isValidGroupName("group_name")).isTrue();
47+
assertThat(TelegrafMetricController.isValidGroupName("A")).isTrue();
3748
}
3849

3950
@Test
40-
void sanitizeForLog_truncate() {
41-
String longValue = "a".repeat(150);
42-
String result = TelegrafMetricController.sanitizeForLog(longValue);
43-
assertThat(result).hasSize(100 + "...(truncated)".length());
44-
assertThat(result).endsWith("...(truncated)");
51+
void isValidGroupName_invalid() {
52+
assertThat(TelegrafMetricController.isValidGroupName("{groupname}")).isFalse();
53+
assertThat(TelegrafMetricController.isValidGroupName("-group")).isFalse();
54+
assertThat(TelegrafMetricController.isValidGroupName(".group")).isFalse();
55+
assertThat(TelegrafMetricController.isValidGroupName("group name")).isFalse();
56+
assertThat(TelegrafMetricController.isValidGroupName("")).isFalse();
4557
}
4658

4759
@Test
48-
void sanitizeForLog_exactlyMaxLength() {
49-
String value = "a".repeat(100);
50-
assertThat(TelegrafMetricController.sanitizeForLog(value)).isEqualTo(value);
60+
void filterTag() {
61+
List<Tag> tags = List.of(
62+
new Tag("tag1", "value1"),
63+
new Tag("host", "host1")
64+
);
65+
List<Tag> tags1 = TelegrafMetricController.filterTag(tags, new String[]{"host"});
66+
assertThat(tags1)
67+
.hasSize(1)
68+
.containsExactly(new Tag("tag1", "value1"));
5169
}
70+
5271
}

0 commit comments

Comments
 (0)