Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"match regular expression" stream rule type fails to properly validate message starting with a blank space #1936

Closed
StCyr opened this issue Mar 16, 2016 · 3 comments
Assignees
Milestone

Comments

@StCyr
Copy link

StCyr commented Mar 16, 2016

Problem description

Whe you want to match the beginning of a syslog message that starts with a blank space against a "match regular expression" stream rule, you must not include the leading blank space in your regular expression if you want the UI to tell you the message will match. Though, you must include it if you want the StreamMatcherFilter to effectively route your message to your stream.

Steps to reproduce the problem

  1. Have a system sending your graylog server a syslog message starting with an empty space (eg: " Login incorrect: [root] (from client BLABLABLA port 0 cli a.b.c.d)\n";
  2. Set up the following stream rule to match the beginning of the message: "^Login incorrect.*";
  3. Confirm that the UI tells you the message from step 1 will match;
  4. Confirm that no messages get routed to the stream;
  5. Set up the following stream rule to match the beginning of the message: "^.Login incorrect.*";
  6. Confirm that the UI tells you the message from step 1 wont't match;
  7. Confirm that messages get routed to the stream.

Sample data

Here are sample data to reproduce the problem:

Message

{
  "message": {
    "message": " Login incorrect: [root] (from client BLABLABLA port 0 cli a.b.c.d)\n",
    "timestamp": "2016-03-16T09:27:44.249Z",
    "process_id": "22562",
    "level": 5,
    "application_name": "freeradius",
    "source": "louise",
    "facility": "local5",
    "_id": "50b39030-eb59-11e5-a255-08002703c127",
    "gl2_source_input": "56e13cd1e4b0deefe9f54cf8",
    "gl2_source_node": "3aee6e01-9fdb-43d4-83b7-324a58e265d6",
    "streams": [],
    "gl2_remote_port": 41948,
    "gl2_remote_ip": "w.x.y.z"
  },
  "index": "graylog2_0"
}

Rules

 "stream_rules": [
    {
      "id": "56e91f0de4b0fcf9693e796a",
      "field": "application_name",
      "value": "freeradius",
      "stream_id": "56e91ebae4b0fcf9693e790f",
      "type": 1,
      "inverted": false
    },
    {
      "id": "56e91f2fe4b0fcf9693e798e",
      "field": "message",
      "value": "^Login incorrect.*",
      "stream_id": "56e91ebae4b0fcf9693e790f",
      "type": 2,
      "inverted": false
    }
  ]

Environment

  • Graylog Version: 1.3
  • Elasticsearch Version: 1.7.5
  • MongoDB Version: 3.2.4
  • Operating System: Ubuntu 14.04
  • Browser version: Firefox 44.0.2

IRC Logs

10:46:19 - StCyr: mmm, created a new stream, restarted the whole server, still not having my messages routed to the stream though the "Rules Editor UI" say they match the rules?!?
10:46:45 - StCyr: Here's a debug log after pausing-starting the stream => http://pastebin.com/40uCCexf
10:47:12 - StCyr: Message 50b39030-eb59-11e5-a255-08002703c127 matches the rule in the "Edit Rules" UI
10:54:37 - joschi: StCyr: could you post some example messages and the stream rules?
10:54:51 - joschi: StCyr: you don't need to restart Graylog when creating new streams or changing stream rules
10:54:58 - StCyr: pastebin?
10:58:35 - StCyr: message: http://pastebin.com/NsEwkzJG
10:59:01 - StCyr: rules: http://pastebin.com/UC0mGNg7
11:00:35 - StCyr: there's always this blank space at the start of all my messages, but "Edit Rules" UI doesn't mind
11:02:17 - StCyr: joschi: maybe I should try to add this blank space in my regular expression though "Edit Rules" UI doesn't mind?
11:05:53 - StCyr: the thing is: if I add this blank space in my regex the "Edit Rules" UI say message doesn't match O.o
12:09:27 - joschi: StCyr: the leading blank is definitely a problem. please create an issue for that problem (the web interface saying the rule matched while it didn't) at https://github.com/Graylog2/graylog2-server/issues
@kroepke kroepke added this to the 2.0.0 milestone Mar 18, 2016
@dennisoelkers dennisoelkers self-assigned this Mar 22, 2016
@dennisoelkers
Copy link
Member

dennisoelkers commented Mar 24, 2016

This is related to message fields being trimmed in the Message class, therefore the message which is retrieved by the web interface and passed to the server for stream rule matching is already trimmed of all leading whitespace. Will need to investigate if there is a good reason for trimming fields.

Seems to be introduced in 152e901 due to #186, unfortunately without any given context/reason.

@lennartkoopmann
Copy link
Contributor

This was introduced because people struggled with "invisible" whitespaces at the beginning or more importantly end of the string. We decided to trim them off and let people use the \s regex notation instead.

The UI should be consistent with this of course or as a workaround be able to alert that there are whitespaces.

I'd vote for not changing the existing backend behaviour because this can lead to hard to debug side effects when upgrading and stream rules are suddenly no longer matching.

@dennisoelkers
Copy link
Member

If we do not want to change the backend behavior, we have no way to fix this issue, as the frontend simply does not know that certain fields are trimmed.

We could look into moving the trimming to the stream rule matchers (or to be safe: match a stream rule both against the trimmed and non-trimmed field if it matches against the start of the string), although I think correcting a bug which causes an inconsistency between predicted and actual matching results justifies a breaking change, as long as it is documented properly.

@kroepke kroepke modified the milestones: 2.1.0, 2.0.0 May 2, 2016
@edmundoa edmundoa self-assigned this Jul 18, 2016
edmundoa pushed a commit that referenced this issue Jul 20, 2016
Before this change, one of the public constructors of `Message` was using
`addField()` to add fields in the `Message` object, while the other was
adding the fields by hand to the internal `Map` used to store message
fields.

That created an inconsistency, as the constructor using `addField()`
trims all `String` field values by default, unlike the other constructor.
You can read more about it in: #1936.

This commit ensures both constructors trim `String` values, while still
ensuring `Message(final String message, final String source, final
DateTime timestamp)` will create a `Message` object containing the
required fields given as arguments.

Making this change will break stream rules checking for one or more leading
or trailing whitespaces in message fields.

Fixes #1936
edmundoa pushed a commit that referenced this issue Jul 20, 2016
Before this change, one of the public constructors of `Message` was using
`addField()` to add fields in the `Message` object, while the other was
adding the fields by hand to the internal `Map` used to store message
fields.

That created an inconsistency, as the constructor using `addField()`
trims all `String` field values by default, unlike the other constructor.
You can read more about it in: #1936.

This commit ensures both constructors trim `String` values, while still
ensuring `Message(final String message, final String source, final
DateTime timestamp)` will create a `Message` object containing the
required fields given as arguments.

Making this change will break stream rules, extractors, and drool rules
checking for one or more leading or trailing whitespaces in message fields.

Fixes #1936
dennisoelkers pushed a commit that referenced this issue Jul 26, 2016
Before this change, one of the public constructors of `Message` was using
`addField()` to add fields in the `Message` object, while the other was
adding the fields by hand to the internal `Map` used to store message
fields.

That created an inconsistency, as the constructor using `addField()`
trims all `String` field values by default, unlike the other constructor.
You can read more about it in: #1936.

This commit ensures both constructors trim `String` values, while still
ensuring `Message(final String message, final String source, final
DateTime timestamp)` will create a `Message` object containing the
required fields given as arguments.

Making this change will break stream rules, extractors, and drool rules
checking for one or more leading or trailing whitespaces in message fields.

Fixes #1936
joschi pushed a commit that referenced this issue Jul 28, 2016
Before this change, one of the public constructors of `Message` was using
`addField()` to add fields in the `Message` object, while the other was
adding the fields by hand to the internal `Map` used to store message
fields.

That created an inconsistency, as the constructor using `addField()`
trims all `String` field values by default, unlike the other constructor.
You can read more about it in: #1936.

This commit ensures both constructors trim `String` values, while still
ensuring `Message(final String message, final String source, final
DateTime timestamp)` will create a `Message` object containing the
required fields given as arguments.

Making this change will break stream rules, extractors, and drool rules
checking for one or more leading or trailing whitespace in message fields.

Fixes #1936
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants