Skip to content

Commit

Permalink
Fail fast and loud for invalid GELF messages (#3972)
Browse files Browse the repository at this point in the history 
* Fail fast and loud for invalid GELF messages

Instead of waiting for a later stage and "silently" dropping (logged on DEBUG)
invalid messages, `GelfCodec` now actively checks for the existence and validity
of mandatory GELF message fields (such as "version", "host", "short_message", and
"timestamp", according to the GELF spec).

Refs http://docs.graylog.org/en/2.2/pages/gelf.html
Fixes #3970

* Don't check "version" field for backward-compatibility

There are still many GELF client libraries out there using either "1.0" or
no value at all for the GELF "version" field.

* Make GELF validation more lenient

* Add test for minimal GELF messages

* Fix logic for validating "host" message field
  • Loading branch information
@joschi @bernd
joschi authored and bernd committed Jul 5, 2017
1 parent 20b38d2 commit 090805b
Show file tree
Hide file tree
Showing 2 changed files with 258 additions and 0 deletions.
45 changes: 45 additions & 0 deletions graylog2-server/src/main/java/org/graylog2/inputs/codecs/GelfCodec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.graylog2.inputs.codecs;

import com.eaio.uuid.UUID;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -24,6 +25,7 @@
import org.graylog2.inputs.codecs.gelf.GELFMessage;
import org.graylog2.inputs.transports.TcpTransport;
import org.graylog2.plugin.Message;
import org.graylog2.plugin.ResolvableInetSocketAddress;
import org.graylog2.plugin.Tools;
import org.graylog2.plugin.configuration.Configuration;
import org.graylog2.plugin.configuration.ConfigurationRequest;
Expand Down Expand Up @@ -124,6 +126,8 @@ public Message decode(@Nonnull final RawMessage rawMessage) {
throw new IllegalStateException("JSON is null/could not be parsed (invalid JSON)", e);
}

validateGELFMessage(node, rawMessage.getId(), rawMessage.getRemoteAddress());

// Timestamp.
final double messageTimestamp = doubleValue(node, Message.FIELD_TIMESTAMP);
final DateTime timestamp;
Expand Down Expand Up @@ -218,6 +222,47 @@ public Message decode(@Nonnull final RawMessage rawMessage) {
return message;
}

private void validateGELFMessage(JsonNode jsonNode, UUID id, ResolvableInetSocketAddress remoteAddress) {
final String prefix = "GELF message <" + id + "> " + (remoteAddress == null ? "" : "(received from <" + remoteAddress + ">) ");

final JsonNode hostNode = jsonNode.path("host");
if (hostNode.isMissingNode()) {
log.warn(prefix + "is missing mandatory \"host\" field.");
} else {
if (!hostNode.isTextual()) {
throw new IllegalArgumentException(prefix + "has invalid \"host\": " + hostNode.asText());
}
if (StringUtils.isBlank(hostNode.asText())) {
throw new IllegalArgumentException(prefix + "has empty mandatory \"host\" field.");
}
}

final JsonNode shortMessageNode = jsonNode.path("short_message");
final JsonNode messageNode = jsonNode.path("message");
if (!shortMessageNode.isMissingNode()) {
if (!shortMessageNode.isTextual()) {
throw new IllegalArgumentException(prefix + "has invalid \"short_message\": " + shortMessageNode.asText());
}
if (StringUtils.isBlank(shortMessageNode.asText())) {
throw new IllegalArgumentException(prefix + "has empty mandatory \"short_message\" field.");
}
} else if (!messageNode.isMissingNode()) {
if (!messageNode.isTextual()) {
throw new IllegalArgumentException(prefix + "has invalid \"message\": " + messageNode.asText());
}
if (StringUtils.isBlank(messageNode.asText())) {
throw new IllegalArgumentException(prefix + "has empty mandatory \"message\" field.");
}
} else {
throw new IllegalArgumentException(prefix + "is missing mandatory \"short_message\" or \"message\" field.");
}

final JsonNode timestampNode = jsonNode.path("timestamp");
if (timestampNode.isValueNode() && !timestampNode.isNumber()) {
throw new IllegalArgumentException(prefix + "has invalid \"timestamp\": " + timestampNode.asText());
}
}

@Nullable
@Override
public CodecAggregator getAggregator() {
Expand Down
213 changes: 213 additions & 0 deletions graylog2-server/src/test/java/org/graylog2/inputs/codecs/GelfCodecTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;

import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.hamcrest.Matchers.isA;
import static org.junit.Assume.assumeTrue;

Expand Down Expand Up @@ -148,4 +150,215 @@ public void decodeLargeCompressedMessageFails() throws Exception {
public void getAggregatorReturnsGelfChunkAggregator() throws Exception {
assertThat(codec.getAggregator()).isSameAs(aggregator);
}

@Test
public void decodeSucceedsWithoutHost() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"short_message\": \"A short message that helps you identify what is going on\""
+ "}";
final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

final Message message = codec.decode(rawMessage);
assertThat(message).isNotNull();
}

@Test
public void decodeFailsWithWrongTypeForHost() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": 42,"
+ "\"short_message\": \"A short message that helps you identify what is going on\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has invalid \"host\": 42");
}

@Test
public void decodeFailsWithEmptyHost() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"\","
+ "\"short_message\": \"A short message that helps you identify what is going on\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has empty mandatory \"host\" field.");
}

@Test
public void decodeFailsWithBlankHost() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \" \","
+ "\"short_message\": \"A short message that helps you identify what is going on\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has empty mandatory \"host\" field.");
}

@Test
public void decodeFailsWithoutShortMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> is missing mandatory \"short_message\" or \"message\" field.");
}

@Test
public void decodeFSucceedsWithoutShortMessageButWithMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"message\": \"A short message that helps you identify what is going on\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

final Message message = codec.decode(rawMessage);
assertThat(message).isNotNull();
}

@Test
public void decodeFailsWithWrongTypeForShortMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"short_message\": 42"
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has invalid \"short_message\": 42");
}

@Test
public void decodeFailsWithWrongTypeForMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"message\": 42"
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has invalid \"message\": 42");
}

@Test
public void decodeFailsWithEmptyShortMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"short_message\": \"\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has empty mandatory \"short_message\" field.");
}

@Test
public void decodeFailsWithEmptyMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"message\": \"\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has empty mandatory \"message\" field.");
}

@Test
public void decodeFailsWithBlankShortMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"short_message\": \" \""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has empty mandatory \"short_message\" field.");
}

@Test
public void decodeFailsWithBlankMessage() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"message\": \" \""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has empty mandatory \"message\" field.");
}

@Test
public void decodeFailsWithWrongTypeForTimestamp() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\","
+ "\"short_message\": \"A short message that helps you identify what is going on\","
+ "\"timestamp\": \"Foobar\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> has invalid \"timestamp\": Foobar");
}

@Test
public void decodeIncludesSourceAddressIfItFails() throws Exception {
final String json = "{"
+ "\"version\": \"1.1\","
+ "\"host\": \"example.org\""
+ "}";

final RawMessage rawMessage = new RawMessage(json.getBytes(StandardCharsets.UTF_8), new InetSocketAddress("198.51.100.42", 24783));

assertThatIllegalArgumentException().isThrownBy(() -> codec.decode(rawMessage))
.withNoCause()
.withMessageMatching("GELF message <[0-9a-f-]+> \\(received from <198\\.51\\.100\\.42:24783>\\) is missing mandatory \"short_message\" or \"message\" field.");
}

@Test
public void decodeSucceedsWithMinimalMessages() throws Exception {
assertThat(codec.decode(new RawMessage("{\"short_message\":\"0\"}".getBytes(StandardCharsets.UTF_8)))).isNotNull();
assertThat(codec.decode(new RawMessage("{\"message\":\"0\"}".getBytes(StandardCharsets.UTF_8)))).isNotNull();
}
}

0 comments on commit 090805b

Please sign in to comment.