Use temp properties file instead of providing agent arguments (#865)

closes #864

Author: felixbarny

CHANGELOG.md

@@ -18,6 +18,7 @@
  * Error in log when setting [server_urls](https://www.elastic.co/guide/en/apm/agent/java/current/config-reporter.html#config-server-urls) 
  to an empty string - `co.elastic.apm.agent.configuration.ApmServerConfigurationSource - Expected previousException not to be null`
 * Avoid terminating the TCP connection to APM Server when polling for configuration updates (#823)
+* Fixes potential segfault if attaching the agent with long arguments (#865)
 
 # 1.9.0
 

apm-agent-attach/src/main/java/co/elastic/apm/attach/ElasticApmAttacher.java

@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -46,6 +46,13 @@
  */
 public class ElasticApmAttacher {
 
+    /**
+     * This key is very short on purpose.
+     * The longer the agent argument ({@code -javaagent:<path>=<args>}), the greater the chance that the max length of the agent argument is reached.
+     * Because of a bug in the {@linkplain ByteBuddyAgent.AttachmentProvider.ForEmulatedAttachment emulated attachment},
+     * this can even lead to segfaults.
+     */
+    private static final String TEMP_PROPERTIES_FILE_KEY = "c";
     private static final ByteBuddyAgent.AttachmentProvider ATTACHMENT_PROVIDER = new ByteBuddyAgent.AttachmentProvider.Compound(
         ByteBuddyAgent.AttachmentProvider.ForEmulatedAttachment.INSTANCE,
         ByteBuddyAgent.AttachmentProvider.ForModularizedVm.INSTANCE,
@@ -97,7 +104,32 @@ public static void attach(Map<String, String> configuration) {
         if (Boolean.getBoolean("ElasticApm.attached")) {
             return;
         }
-        ByteBuddyAgent.attach(AgentJarFileHolder.INSTANCE.agentJarFile, ByteBuddyAgent.ProcessProvider.ForCurrentVm.INSTANCE, toAgentArgs(configuration), ATTACHMENT_PROVIDER);
+        File tempFile = createTempProperties(configuration);
+        String agentArgs = tempFile == null ? null : TEMP_PROPERTIES_FILE_KEY + "=" + tempFile.getAbsolutePath();
+
+        ByteBuddyAgent.attach(AgentJarFileHolder.INSTANCE.agentJarFile, ByteBuddyAgent.ProcessProvider.ForCurrentVm.INSTANCE, agentArgs, ATTACHMENT_PROVIDER);
+        if (tempFile != null) {
+            if (!tempFile.delete()) {
+                tempFile.deleteOnExit();
+            }
+        }
+    }
+
+    static File createTempProperties(Map<String, String> configuration) {
+        File tempFile = null;
+        if (!configuration.isEmpty()) {
+            Properties properties = new Properties();
+            properties.putAll(configuration);
+            try {
+                tempFile = File.createTempFile("elstcapm", ".tmp");
+                try (FileOutputStream outputStream = new FileOutputStream(tempFile)) {
+                    properties.store(outputStream, null);
+                }
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+        }
+        return tempFile;
     }
 
     static String toAgentArgs(Map<String, String> configuration) {

apm-agent-attach/src/test/java/co/elastic/apm/attach/ElasticApmAttacherTest.java

@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -27,6 +27,11 @@
 import org.junit.jupiter.api.Test;
 import wiremock.org.apache.commons.codec.digest.DigestUtils;
 
+import java.io.File;
+import java.io.FileReader;
+import java.util.Map;
+import java.util.Properties;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 class ElasticApmAttacherTest {
@@ -36,4 +41,15 @@ void testHash() throws Exception {
         assertThat(ElasticApmAttacher.md5Hash(getClass().getResourceAsStream(ElasticApmAttacher.class.getSimpleName() + ".class")))
             .isEqualTo(DigestUtils.md5Hex(getClass().getResourceAsStream(ElasticApmAttacher.class.getSimpleName() + ".class")));
     }
+
+    @Test
+    void testCreateTempProperties() throws Exception {
+        File tempProperties = ElasticApmAttacher.createTempProperties(Map.of("foo", "bär"));
+        assertThat(tempProperties).isNotNull();
+        tempProperties.deleteOnExit();
+        Properties properties = new Properties();
+        properties.load(new FileReader(tempProperties));
+        assertThat(properties.get("foo")).isEqualTo("bär");
+
+    }
 }

apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/AgentArgumentsConfigurationSource.java

@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -27,6 +27,7 @@
 import org.stagemonitor.configuration.source.AbstractConfigurationSource;
 import org.stagemonitor.util.StringUtils;
 
+import javax.annotation.Nullable;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -51,6 +52,7 @@ public static AgentArgumentsConfigurationSource parse(String agentAgruments) {
     }
 
     @Override
+    @Nullable
     public String getValue(String key) {
         return config.get(key);
     }

apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/source/PropertyFileConfigurationSource.java

@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -78,7 +78,7 @@ public static Properties getFromClasspath(String classpathLocation, ClassLoader
         return null;
     }
 
-    private static Properties getFromFileSystem(String location) {
+    public static Properties getFromFileSystem(String location) {
         Properties props = new Properties();
         try (InputStream input = new FileInputStream(location)) {
             props.load(input);

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilder.java

@@ -53,10 +53,15 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
 public class ElasticApmTracerBuilder {
 
+    /**
+     * See {@link co.elastic.apm.attach.ElasticApmAttacher#TEMP_PROPERTIES_FILE_KEY}
+     */
+    private static final String TEMP_PROPERTIES_FILE_KEY = "c";
     private final Logger logger;
     @Nullable
     private ConfigurationRegistry configurationRegistry;
@@ -153,7 +158,12 @@ private ConfigurationRegistry getDefaultConfigurationRegistry(List<Configuration
     private List<ConfigurationSource> getConfigSources(@Nullable String agentArguments) {
         List<ConfigurationSource> result = new ArrayList<>();
         if (agentArguments != null && !agentArguments.isEmpty()) {
-            result.add(AgentArgumentsConfigurationSource.parse(agentArguments));
+            AgentArgumentsConfigurationSource agentArgs = AgentArgumentsConfigurationSource.parse(agentArguments);
+            result.add(agentArgs);
+            ConfigurationSource attachmentConfig = getAttachmentArguments(agentArgs.getValue(TEMP_PROPERTIES_FILE_KEY));
+            if (attachmentConfig != null) {
+                result.add(attachmentConfig);
+            }
         }
         result.add(new PrefixingConfigurationSourceWrapper(new SystemPropertyConfigurationSource(), "elastic.apm."));
         result.add(new PrefixingConfigurationSourceWrapper(new EnvironmentVariableConfigurationSource(), "ELASTIC_APM_"));
@@ -180,4 +190,22 @@ public String getName() {
         return result;
     }
 
+    /**
+     * Loads the configuration from the temporary properties file created by ElasticApmAttacher
+     */
+    @Nullable
+    private ConfigurationSource getAttachmentArguments(@Nullable String configFileLocation) {
+        if (configFileLocation != null) {
+            Properties fromFileSystem = PropertyFileConfigurationSource.getFromFileSystem(configFileLocation);
+            if (fromFileSystem != null) {
+                SimpleSource attachmentConfig = new SimpleSource("Attachment configuration");
+                for (String key : fromFileSystem.stringPropertyNames()) {
+                    attachmentConfig.add(key, fromFileSystem.getProperty(key));
+                }
+                return attachmentConfig;
+            }
+        }
+        return null;
+    }
+
 }

apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerBuilderTest.java

@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -39,7 +39,6 @@
 
 class ElasticApmTracerBuilderTest {
 
-
     @AfterEach
     void tearDown() {
         System.clearProperty("elastic.apm." + CoreConfiguration.CONFIG_FILE);
@@ -59,4 +58,14 @@ void testConfigFileLocation(@TempDir Path tempDir) throws IOException {
         configurationRegistry.getString(CoreConfiguration.CONFIG_FILE);
         assertThat(configurationRegistry.getString(CoreConfiguration.CONFIG_FILE)).isEqualTo(file.toString());
     }
+
+    @Test
+    void testTempAttacherPropertiesFile(@TempDir Path tempDir) throws Exception {
+        Path file = Files.createFile(tempDir.resolve("elstcapm.tmp"));
+        Files.write(file, List.of("instrument=false"));
+
+        ConfigurationRegistry configurationRegistry = new ElasticApmTracerBuilder("c=" + file.toAbsolutePath()).build().getConfigurationRegistry();
+        CoreConfiguration config = configurationRegistry.getConfig(CoreConfiguration.class);
+        assertThat(config.isInstrument()).isFalse();
+    }
 }

docs/configuration.asciidoc

@@ -358,7 +358,7 @@ you should add an additional entry to this list (make sure to also include the d
 ==== `disable_instrumentations`
 
 A list of instrumentations which should be disabled.
-Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`.
+Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `mule`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`.
 If you want to try out incubating features,
 set the value to an empty string.
 
@@ -1551,7 +1551,7 @@ The default unit for this option is `ms`
 # sanitize_field_names=password,passwd,pwd,secret,*key,*token*,*session*,*credit*,*card*,authorization,set-cookie
 
 # A list of instrumentations which should be disabled.
-# Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`.
+# Valid options are `annotations`, `apache-httpclient`, `asynchttpclient`, `concurrent`, `elasticsearch-restclient`, `exception-handler`, `executor`, `hibernate-search`, `http-client`, `incubating`, `jax-rs`, `jax-ws`, `jdbc`, `jedis`, `jms`, `jsf`, `logging`, `mule`, `okhttp`, `opentracing`, `public-api`, `quartz`, `redis`, `render`, `scheduled`, `servlet-api`, `servlet-api-async`, `servlet-input-stream`, `slf4j`, `spring-mvc`, `spring-resttemplate`, `spring-service-name`, `spring-view-render`, `urlconnection`.
 # If you want to try out incubating features,
 # set the value to an empty string.
 #