Skip to content

Commit

Permalink
adding "/" and " " as valid chars in fieldnames
Browse files Browse the repository at this point in the history
  • Loading branch information
janheise committed Nov 6, 2024
1 parent 47a2280 commit 2b69867
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 37 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/pr-20142.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type = "a"
message = "allowing ' ' and '/' in fieldnames"

issues = []
pulls = ["20142"]
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Strings;
import io.swagger.annotations.ApiParam;
import org.graylog.plugins.views.search.searchtypes.pivot.SortSpec;
import org.graylog2.plugin.Message;
import org.graylog2.plugin.indexer.searches.timeranges.TimeRange;
Expand All @@ -26,15 +27,15 @@
import java.util.Set;
import java.util.stream.Collectors;

public record MessagesRequestSpec(@JsonProperty("query") String queryString,
@JsonProperty("streams") Set<String> streams,
@JsonProperty("stream_categories") Set<String> streamCategories,
@JsonProperty("timerange") TimeRange timerange,
@JsonProperty("sort") String sort,
@JsonProperty("sort_order") SortSpec.Direction sortOrder,
@JsonProperty("from") int from,
@JsonProperty("size") int size,
@JsonProperty("fields") List<String> fields) implements SearchRequestSpec {
public record MessagesRequestSpec(@ApiParam(name = "query", value = "Query (Lucene syntax)", required = true) @JsonProperty("query") String queryString,
@ApiParam(name = "streams", value = "Streams to search") @JsonProperty("streams") Set<String> streams,
@ApiParam(name = "stream_categories", value = "Stream categories") @JsonProperty("stream_categories") Set<String> streamCategories,
@ApiParam(name = "timerange", value = "Timerange for the search", required = true) @JsonProperty("timerange") TimeRange timerange,
@ApiParam(name = "sort", value = "Field to sort on") @JsonProperty("sort") String sort,
@ApiParam(name = "sort_order", value = "Sort order (ASC or DESC)") @JsonProperty("sort_order") SortSpec.Direction sortOrder,
@ApiParam(name = "from", value = "Number of elements to skip") @JsonProperty("from") int from,
@ApiParam(name = "size", value = "Number of results to return") @JsonProperty("size") int size,
@ApiParam(name = "fields", value = "Fields to show from the messages in the result set") @JsonProperty("fields") List<String> fields) implements SearchRequestSpec {


public static final List<String> DEFAULT_FIELDS = List.of("source", "timestamp");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public LuceneQueryParser(@Named("allow_leading_wildcard_searches") final boolean

public ParsedQuery parse(final String query) throws ParseException {
final TokenCollectingQueryParser parser = new TokenCollectingQueryParser(ParsedTerm.DEFAULT_FIELD, ANALYZER);
parser.setSplitOnWhitespace(true);
parser.setAllowLeadingWildcard(allowLeadingWildcard);

final Query parsed = parser.parse(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public class Message implements Messages, Indexable, Acknowledgeable {
public static final String FIELD_GL2_SOURCE_RADIO_INPUT = "gl2_source_radio_input";

// Matches whole field names containing a-z, A-Z, 0-9, period char, -, or @.
private static final CharMatcher VALID_KEY_CHAR_MATCHER = inRange('a', 'z').or(inRange('A', 'Z')).or(inRange('0', '9')).or(anyOf(".@-_")).precomputed();
private static final CharMatcher VALID_KEY_CHAR_MATCHER = inRange('a', 'z').or(inRange('A', 'Z')).or(inRange('0', '9')).or(anyOf(".@-_ /")).precomputed();
private static final CharMatcher INVALID_KEY_CHAR_MATCHER = VALID_KEY_CHAR_MATCHER.negate().precomputed();

private static final char KEY_REPLACEMENT_CHAR = '_';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1657,33 +1657,42 @@ void setField() {

assertThat(message.getField("f1")).isEqualTo("v1");
assertThat(message.getField("f_2")).isEqualTo("v_2");
assertThat(message.getField("f 3")).isNull();
assertThat(message.getField("f_3")).isNull();
assertThat(message.getField("f_4")).isEqualTo("will be added with clean field param");
assertThat(message.getField("f 3")).isEqualTo("f 3");
assertThat(message.getField("f/4")).isEqualTo("f/4");
assertThat(message.getField("f%5")).isNull();
assertThat(message.getField("f_6")).isEqualTo("will be added with clean field param");
}

@Test
void setFields() {
final Rule rule = parser.parseRule(ruleForTest(), true);
final Message message = messageFactory.createMessage("test", "test", Tools.nowUTC());
message.addField("json_field_map", "{ " +
" \"k1\": \"v1\", " +
" \"k_2\": \"v_2\", " +
" \"k 3\": \"will be skipped\" " +
"}");
message.addField("json_clean_field_map", "{ " +
" \"k4\": \"v4\", " +
" \"k_5\": \"v_5\", " +
" \"k 6\": \"will be added with clean_fields param\" " +
"}");
message.addField("json_field_map", """
{
"k1": "v1",
"k_2": "v_2",
"k 3": "v 3",
"k%4": "will be skipped"
}
""");
message.addField("json_clean_field_map", """
{
"k4": "v4",
"k_5": "v_5",
"k 7": "v 7",
"k%6": "will be added with clean_fields param"
}
""");
evaluateRule(rule, message);

assertThat(message.getField("k1")).isEqualTo("v1");
assertThat(message.getField("k_2")).isEqualTo("v_2");
assertThat(message.getField("k 3")).isNull();
assertThat(message.getField("k_3")).isNull();
assertThat(message.getField("k 3")).isEqualTo("v 3");
assertThat(message.getField("k%4")).isNull();
assertThat(message.getField("k_4")).isNull();
assertThat(message.getField("k4")).isEqualTo("v4");
assertThat(message.getField("k_5")).isEqualTo("v_5");
assertThat(message.getField("k 7")).isEqualTo("v 7");
assertThat(message.getField("k_6")).isEqualTo("will be added with clean_fields param");
}

Expand Down
19 changes: 10 additions & 9 deletions graylog2-server/src/test/java/org/graylog2/plugin/MessageTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -349,20 +349,21 @@ public void testGetSource() throws Exception {
}

@Test
public void testValidKeys() throws Exception {
public void testValidKeys() {
assertTrue(Message.validKey("a"));
assertTrue(Message.validKey("foo123"));
assertTrue(Message.validKey("foo-bar123"));
assertTrue(Message.validKey("foo_bar123"));
assertTrue(Message.validKey("foo.bar123"));
assertTrue(Message.validKey("foo@bar"));
assertTrue(Message.validKey("123"));
assertTrue(Message.validKey("foo bar"));
assertTrue(Message.validKey("foo/bar"));
assertTrue(Message.validKey(""));
assertFalse(Message.validKey(" foo123"));
assertFalse(Message.validKey("foo123 "));
assertFalse(Message.validKey("foo bar"));
assertTrue(Message.validKey(" foo123"));
assertTrue(Message.validKey("foo123 "));
assertFalse(Message.validKey("foo+bar"));
assertFalse(Message.validKey("foo$bar"));
assertFalse(Message.validKey(" "));
}

@Test
Expand All @@ -372,18 +373,18 @@ public void testCleanKey() throws Exception {
assertEquals("foo-bar123", Message.cleanKey("foo-bar123"));
assertEquals("foo_bar123", Message.cleanKey("foo_bar123"));
assertEquals("foo.bar123", Message.cleanKey("foo.bar123"));
assertEquals("foo bar", Message.cleanKey("foo bar"));
assertEquals("foo/bar", Message.cleanKey("foo/bar"));
assertEquals("foo@bar", Message.cleanKey("foo@bar"));
assertEquals("123", Message.cleanKey("123"));
assertEquals("", Message.cleanKey(""));
assertEquals(" ", Message.cleanKey(" "));

assertEquals("foo_bar", Message.cleanKey("foo bar"));
assertEquals("foo_bar", Message.cleanKey("foo+bar"));
assertEquals("foo_bar", Message.cleanKey("foo$bar"));
assertEquals("foo_bar", Message.cleanKey("foo{bar"));
assertEquals("foo_bar", Message.cleanKey("foo,bar"));
assertEquals("foo_bar", Message.cleanKey("foo?bar"));
assertEquals("foo___bar", Message.cleanKey("foo +?bar"));
assertEquals("_", Message.cleanKey(" "));
assertEquals("foo __bar", Message.cleanKey("foo +?bar"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ when true
then
set_field(field: "f1", value: "v1");
set_field(field: "f_2", value: "v_2");
set_field(field: "f 3", value: "will be skipped");
set_field(field: "f 4", value: "will be added with clean field param", clean_field: true);
set_field(field: "f 3", value: "f 3");
set_field(field: "f/4", value: "f/4");
set_field(field: "f%5", value: "will be skipped");
set_field(field: "f%6", value: "will be added with clean field param", clean_field: true);
end

0 comments on commit 2b69867

Please sign in to comment.