Fix reconstituting version string from components (elastic#117213)

* Fix reconstituting version string from components Co-authored-by: Joe Gallo <[email protected]>
CloudGeometry · Dec 4, 2024 · 28eda97 · 28eda97
1 parent 4a90903
commit 28eda97
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 25 deletions.
diff --git a/docs/changelog/117213.yaml 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/...ngest-user-agent/src/main/java/org/elasticsearch/ingest/useragent/UserAgentProcessor.java b/...ngest-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,40 +96,19 @@ public IngestDocument execute(IngestDocument ingestDocument) {
                     }
                     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:
                     if (uaClient.operatingSystem() != null) {
                         Map<String, String> 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 IngestDocument execute(IngestDocument ingestDocument) {
         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/...-user-agent/src/test/java/org/elasticsearch/ingest/useragent/UserAgentProcessorTests.java b/...-user-agent/src/test/java/org/elasticsearch/ingest/useragent/UserAgentProcessorTests.java
@@ -345,4 +345,21 @@ public void testMaybeUpgradeConfig_doesNothingIfEcsAbsent() {
         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<String, Object> 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<String, Object> data = ingestDocument.getSourceAndMetadata();
+
+        assertThat(data, hasKey("target_field"));
+        Map<String, Object> target = (Map<String, Object>) data.get("target_field");
+
+        assertThat(target.get("name"), is("Firefox"));
+        assertThat(target.get("version"), is("128.0"));
+    }
 }