From 28eda97ddd48fc23f6d21d3f5b8a68f977f9627f Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Tue, 3 Dec 2024 17:32:28 -0700 Subject: [PATCH] Fix reconstituting version string from components (#117213) * Fix reconstituting version string from components Co-authored-by: Joe Gallo --- docs/changelog/117213.yaml | 6 +++ .../ingest/useragent/UserAgentProcessor.java | 47 +++++++++---------- .../useragent/UserAgentProcessorTests.java | 17 +++++++ 3 files changed, 45 insertions(+), 25 deletions(-) create mode 100644 docs/changelog/117213.yaml diff --git a/docs/changelog/117213.yaml b/docs/changelog/117213.yaml new file mode 100644 index 000000000000..3b4cd0cee966 --- /dev/null +++ b/docs/changelog/117213.yaml @@ -0,0 +1,6 @@ +pr: 117213 +summary: Fix reconstituting version string from components +area: Ingest Node +type: bug +issues: + - 116950 diff --git a/modules/ingest-user-agent/src/main/java/org/elasticsearch/ingest/useragent/UserAgentProcessor.java b/modules/ingest-user-agent/src/main/java/org/elasticsearch/ingest/useragent/UserAgentProcessor.java index 08ec00e0f04c..ddd754c9f7d1 100644 --- a/modules/ingest-user-agent/src/main/java/org/elasticsearch/ingest/useragent/UserAgentProcessor.java +++ b/modules/ingest-user-agent/src/main/java/org/elasticsearch/ingest/useragent/UserAgentProcessor.java @@ -9,6 +9,7 @@ package org.elasticsearch.ingest.useragent; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.util.Maps; import org.elasticsearch.core.UpdateForV10; import org.elasticsearch.ingest.AbstractProcessor; @@ -95,19 +96,8 @@ public class UserAgentProcessor extends AbstractProcessor { } break; case VERSION: - StringBuilder version = new StringBuilder(); if (uaClient.userAgent() != null && uaClient.userAgent().major() != null) { - version.append(uaClient.userAgent().major()); - if (uaClient.userAgent().minor() != null) { - version.append(".").append(uaClient.userAgent().minor()); - if (uaClient.userAgent().patch() != null) { - version.append(".").append(uaClient.userAgent().patch()); - if (uaClient.userAgent().build() != null) { - version.append(".").append(uaClient.userAgent().build()); - } - } - } - uaDetails.put("version", version.toString()); + uaDetails.put("version", versionToString(uaClient.userAgent())); } break; case OS: @@ -115,20 +105,10 @@ public class UserAgentProcessor extends AbstractProcessor { Map osDetails = Maps.newMapWithExpectedSize(3); if (uaClient.operatingSystem().name() != null) { osDetails.put("name", uaClient.operatingSystem().name()); - StringBuilder sb = new StringBuilder(); if (uaClient.operatingSystem().major() != null) { - sb.append(uaClient.operatingSystem().major()); - if (uaClient.operatingSystem().minor() != null) { - sb.append(".").append(uaClient.operatingSystem().minor()); - if (uaClient.operatingSystem().patch() != null) { - sb.append(".").append(uaClient.operatingSystem().patch()); - if (uaClient.operatingSystem().build() != null) { - sb.append(".").append(uaClient.operatingSystem().build()); - } - } - } - osDetails.put("version", sb.toString()); - osDetails.put("full", uaClient.operatingSystem().name() + " " + sb.toString()); + String version = versionToString(uaClient.operatingSystem()); + osDetails.put("version", version); + osDetails.put("full", uaClient.operatingSystem().name() + " " + version); } uaDetails.put("os", osDetails); } @@ -160,6 +140,23 @@ public class UserAgentProcessor extends AbstractProcessor { return ingestDocument; } + private static String versionToString(final UserAgentParser.VersionedName version) { + final StringBuilder versionString = new StringBuilder(); + if (Strings.hasLength(version.major())) { + versionString.append(version.major()); + if (Strings.hasLength(version.minor())) { + versionString.append(".").append(version.minor()); + if (Strings.hasLength(version.patch())) { + versionString.append(".").append(version.patch()); + if (Strings.hasLength(version.build())) { + versionString.append(".").append(version.build()); + } + } + } + } + return versionString.toString(); + } + @Override public String getType() { return TYPE; diff --git a/modules/ingest-user-agent/src/test/java/org/elasticsearch/ingest/useragent/UserAgentProcessorTests.java b/modules/ingest-user-agent/src/test/java/org/elasticsearch/ingest/useragent/UserAgentProcessorTests.java index 471015d57901..55900c6faf5c 100644 --- a/modules/ingest-user-agent/src/test/java/org/elasticsearch/ingest/useragent/UserAgentProcessorTests.java +++ b/modules/ingest-user-agent/src/test/java/org/elasticsearch/ingest/useragent/UserAgentProcessorTests.java @@ -345,4 +345,21 @@ public class UserAgentProcessorTests extends ESTestCase { assertThat(changed, is(false)); assertThat(config, is(Map.of("field", "user-agent"))); } + + // From https://github.com/elastic/elasticsearch/issues/116950 + @SuppressWarnings("unchecked") + public void testFirefoxVersion() { + Map document = new HashMap<>(); + document.put("source_field", "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:128.0) Gecko/20100101 Firefox/128.0"); + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + + processor.execute(ingestDocument); + Map data = ingestDocument.getSourceAndMetadata(); + + assertThat(data, hasKey("target_field")); + Map target = (Map) data.get("target_field"); + + assertThat(target.get("name"), is("Firefox")); + assertThat(target.get("version"), is("128.0")); + } }