diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index f500cb8f020..19941583713 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -67,6 +67,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Convert increments of 100 nanoseconds/ticks to milliseconds for WriteTime and ReadTime in diskio metricset (Windows) for consistency. {issue}14233[14233] - Fix diskio issue for windows 32 bit on disk_performance struct alignment. {issue}16680[16680] - Reduce memory usage in `elasticsearch/index` metricset. {issue}16503[16503] {pull}16538[16538] +- Fix issue in Jolokia module when mbean contains multiple quoted properties. {issue}17375[17375] {pull}17374[17374] *Packetbeat* diff --git a/metricbeat/module/jolokia/jmx/config.go b/metricbeat/module/jolokia/jmx/config.go index 60b9a60ff63..3cd61201c4c 100644 --- a/metricbeat/module/jolokia/jmx/config.go +++ b/metricbeat/module/jolokia/jmx/config.go @@ -117,7 +117,14 @@ type MBeanName struct { Properties map[string]string } -var mbeanRegexp = regexp.MustCompile("([^,=:*?]+)=([^,=:\"]+|\".*\")") +// Parse strings with properties with the format key=value, being: +// - Key a nonempty string of characters which may not contain any of the characters, +// comma (,), equals (=), colon, asterisk, or question mark. +// - Value a string that can be quoted or unquoted, if unquoted it cannot be empty and +// cannot contain any of the characters comma, equals, colon, or quote. +// If quoted, it can contain any character, including newlines, but quote needs to be +// escaped with a backslash. +var mbeanRegexp = regexp.MustCompile(`([^,=:*?]+)=([^,=:"]+|"([^\\"]|\\.)*?")`) // This replacer is responsible for adding a "!" before special characters in GET request URIs // For more information refer: https://jolokia.org/reference/html/protocol.html @@ -158,7 +165,6 @@ func (m *MBeanName) Canonicalize(escape bool) string { // The Mbean string has to abide by the rules which are imposed by Java. // For more info: https://docs.oracle.com/javase/8/docs/api/javax/management/ObjectName.html#getCanonicalName-- func ParseMBeanName(mBeanName string) (*MBeanName, error) { - // Split mbean string in two parts: the bean domain and the properties parts := strings.SplitN(mBeanName, ":", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { @@ -170,15 +176,6 @@ func ParseMBeanName(mBeanName string) (*MBeanName, error) { Domain: parts[0], } - // First of all verify that all bean properties are - // in the form key=value - tmpProps := propertyRegexp.FindAllString(parts[1], -1) - propertyList := strings.Join(tmpProps, ",") - if len(propertyList) != len(parts[1]) { - // Some property didn't match - return nil, fmt.Errorf("mbean properties must be in the form key=value: %s", mBeanName) - } - // Using this regexp we will split the properties in a 2 dimensional array // instead of just splitting by commas because values can be quoted // and contain commas, what complicates the parsing. @@ -195,7 +192,6 @@ func ParseMBeanName(mBeanName string) (*MBeanName, error) { // } properties := mbeanRegexp.FindAllStringSubmatch(parts[1], -1) - // If we could not parse MBean properties if properties == nil { return nil, fmt.Errorf("mbean properties must be in the form key=value: %s", mBeanName) } @@ -203,6 +199,8 @@ func ParseMBeanName(mBeanName string) (*MBeanName, error) { // Initialise properties map mybean.Properties = make(map[string]string) + // Get the parsed string to check that everything has been parsed + var parsed []string for _, prop := range properties { // If every row does not have 3 columns, then @@ -212,9 +210,16 @@ func ParseMBeanName(mBeanName string) (*MBeanName, error) { return nil, fmt.Errorf("mbean properties must be in the form key=value: %s", mBeanName) } + parsed = append(parsed, prop[0]) + mybean.Properties[prop[1]] = prop[2] } + // Not all the properties definition has been parsed + if parsed := strings.Join(parsed, ","); len(parts[1]) != len(parsed) { + return nil, fmt.Errorf("not all properties could be parsed: %s, parsed: %s", mBeanName, parsed) + } + return mybean, nil } @@ -418,13 +423,6 @@ func (pc *JolokiaHTTPPostFetcher) BuildRequestsAndMappings(configMappings []JMXM return httpRequests, mapping, nil } -// Parse strings with properties with the format key=value, being: -// - key a nonempty string of characters which may not contain any of the characters, -// comma (,), equals (=), colon, asterisk, or question mark. -// - value a string that can be quoted or unquoted, if unquoted it cannot be empty and -// cannot contain any of the characters comma, equals, colon, or quote. -var propertyRegexp = regexp.MustCompile("[^,=:*?]+=([^,=:\"]+|\".*\")") - func (pc *JolokiaHTTPPostFetcher) buildRequestBodyAndMapping(mappings []JMXMapping) ([]byte, AttributeMapping, error) { responseMapping := make(AttributeMapping) var blocks []RequestBlock diff --git a/metricbeat/module/jolokia/jmx/config_test.go b/metricbeat/module/jolokia/jmx/config_test.go index b6ccc1b7ccb..e873ccec866 100644 --- a/metricbeat/module/jolokia/jmx/config_test.go +++ b/metricbeat/module/jolokia/jmx/config_test.go @@ -75,32 +75,32 @@ func TestBuildJolokiaGETUri(t *testing.T) { func TestParseMBean(t *testing.T) { - cases := []struct { + cases := map[string]struct { mbean string expected *MBeanName ok bool }{ - { + "empty": { mbean: ``, ok: false, }, - { + "no domain": { mbean: `type=Runtime`, ok: false, }, - { + "no properties": { mbean: `java.lang`, ok: false, }, - { + "no properties, with colon": { mbean: `java.lang:`, ok: false, }, - { + "property without value": { mbean: `java.lang:type=Runtime,name`, ok: false, }, - { + "single property": { mbean: `java.lang:type=Runtime`, expected: &MBeanName{ Domain: `java.lang`, @@ -110,18 +110,17 @@ func TestParseMBean(t *testing.T) { }, ok: true, }, - { - mbean: `java.lang:name=Foo,type=Runtime`, + "other single property": { + mbean: `java.lang:type=Memory`, expected: &MBeanName{ Domain: `java.lang`, Properties: map[string]string{ - "name": "Foo", - "type": "Runtime", + "type": "Memory", }, }, ok: true, }, - { + "multiple properties": { mbean: `java.lang:name=Foo,type=Runtime`, expected: &MBeanName{ Domain: `java.lang`, @@ -132,7 +131,7 @@ func TestParseMBean(t *testing.T) { }, ok: true, }, - { + "property with wildcard": { mbean: `java.lang:type=Runtime,name=Foo*`, expected: &MBeanName{ Domain: `java.lang`, @@ -143,7 +142,7 @@ func TestParseMBean(t *testing.T) { }, ok: true, }, - { + "property with wildcard as value": { mbean: `java.lang:type=Runtime,name=*`, expected: &MBeanName{ Domain: `java.lang`, @@ -154,7 +153,7 @@ func TestParseMBean(t *testing.T) { }, ok: true, }, - { + "quoted property": { mbean: `java.lang:name="foo,bar",type=Runtime`, expected: &MBeanName{ Domain: `java.lang`, @@ -165,17 +164,41 @@ func TestParseMBean(t *testing.T) { }, ok: true, }, - { - mbean: `java.lang:type=Memory`, + "multiple quoted properties": { + mbean: `java.lang:name="foo",othername="bar",type=Runtime`, expected: &MBeanName{ Domain: `java.lang`, Properties: map[string]string{ - "type": "Memory", + "name": `"foo"`, + "othername": `"bar"`, + "type": "Runtime", }, }, ok: true, }, - { + "escaped quote in quoted property": { + mbean: `java.lang:name="foo,\"bar\"",type=Runtime`, + expected: &MBeanName{ + Domain: `java.lang`, + Properties: map[string]string{ + "name": `"foo,\"bar\""`, + "type": "Runtime", + }, + }, + ok: true, + }, + "newline in quoted property": { + mbean: `java.lang:name="foo\nbar",type=Runtime`, + expected: &MBeanName{ + Domain: `java.lang`, + Properties: map[string]string{ + "name": `"foo\nbar"`, + "type": "Runtime", + }, + }, + ok: true, + }, + "real catalina mbean": { mbean: `Catalina:name=HttpRequest1,type=RequestProcessor,worker="http-nio-8080"`, expected: &MBeanName{ Domain: `Catalina`, @@ -187,17 +210,36 @@ func TestParseMBean(t *testing.T) { }, ok: true, }, + "real activemq artemis mbean": { + mbean: `org.apache.activemq.artemis:broker="0.0.0.0",component=addresses,address="helloworld",subcomponent=queues,routing-type="anycast",queue="helloworld"`, + expected: &MBeanName{ + Domain: `org.apache.activemq.artemis`, + Properties: map[string]string{ + "broker": `"0.0.0.0"`, + "component": `addresses`, + "address": `"helloworld"`, + "subcomponent": `queues`, + "routing-type": `"anycast"`, + "queue": `"helloworld"`, + }, + }, + ok: true, + }, } - for _, c := range cases { - beanObj, err := ParseMBeanName(c.mbean) - - if c.ok { - assert.NoError(t, err, "failed parsing for: "+c.mbean) - assert.Equal(t, c.expected, beanObj, "mbean: "+c.mbean) - } else { - assert.Error(t, err, "should have failed for: "+c.mbean) - } + for title, c := range cases { + t.Run(title, func(t *testing.T) { + beanObj, err := ParseMBeanName(c.mbean) + + if c.ok { + if assert.NoError(t, err, "failed parsing for: "+c.mbean) { + t.Log("Canonicalized mbean: ", beanObj.Canonicalize(true)) + } + assert.Equal(t, c.expected, beanObj, "mbean: "+c.mbean) + } else { + assert.Error(t, err, "should have failed for: "+c.mbean) + } + }) } }