Adjust the way arguments are being escaped.

config/app_dynamics_agent.yml

@@ -18,6 +18,6 @@
 version: +
 repository_root: https://packages.appdynamics.com/java
 default_application_name: $(jq -r -n "$VCAP_APPLICATION | .space_name + \":\" + .application_name | @sh")
-default_node_name: $(jq -r -n "\"$APPD_CF_NODE_PREFIX\" + ($VCAP_APPLICATION | .application_name) + \":$CF_INSTANCE_INDEX\"")
+default_node_name: $(jq -r -n "\"$APPD_CF_NODE_PREFIX\" + ($VCAP_APPLICATION | .application_name) + \":$CF_INSTANCE_INDEX\" | @sh")
 default_tier_name:
-default_unique_host_name: $(jq -r -n "$VCAP_APPLICATION | .application_id + \":$CF_INSTANCE_INDEX\"")
+default_unique_host_name: $(jq -r -n "$VCAP_APPLICATION | .application_id + \":$CF_INSTANCE_INDEX\" | @sh")

docs/framework-app_dynamics_agent.md

@@ -13,21 +13,23 @@ The AppDynamics Agent Framework causes an application to be automatically config
 Tags are printed to standard output by the buildpack detect script
 
 ## User-Provided Service
-When binding AppDynamics using a user-provided service, it must have name or tag with `app-dynamics` or `appdynamics` in it. The credential payload can contain the following entries.  **Note:** Credentials marked as "(Optional)" may be required for some versions of the AppDynamics agent.  Please see the [AppDynamics Java Agent Configuration Properties][] for the version of the agent used by your application for more details.
+When binding AppDynamics using a user-provided service, it must have name or tag with `app-dynamics` or `appdynamics` in it. The credential payload can contain the following entries.
 
 | Name | Description
 | ---- | -----------
-| `account-access-key` | (Optional) The account access key to use when authenticating with the controller
-| `account-name` | (Optional) The account name to use when authenticating with the controller
-| `application-name` | (Optional) the application's name
+| `account-access-key` | The account access key to use when authenticating with the controller
+| `account-name` | The account name to use when authenticating with the controller
 | `host-name` | The controller host name
+| `port` | The controller port
+| `ssl-enabled` | Whether or not to use an SSL connection to the controller
+| `application-name` | (Optional) the application's name
 | `node-name` | (Optional) the application's node name
-| `port` | (Optional) The controller port
-| `ssl-enabled` | (Optional) Whether or not to use an SSL connection to the controller
 | `tier-name` | (Optional) the application's tier name
 
 To provide more complex values such as the `tier-name`, using the interactive mode when creating a user-provided service will manage the character escaping automatically. For example, the default `tier-name` could be set with a value of `Tier-$(expr "$VCAP_APPLICATION" : '.*instance_index[": ]*\([[:digit:]]*\).*')` to calculate a value from the Cloud Foundry instance index.
 
+**Note:** Some credentials were previously marked as "(Optional)" as requirements have changed across versions of the AppDynamics agent.  Please see the [AppDynamics Java Agent Configuration Properties][] for the version of the agent used by your application for more details.
+
 ## Configuration
 For general information on configuring the buildpack, including how to specify configuration values through environment variables, refer to [Configuration and Extension][].
 

lib/java_buildpack/framework/app_dynamics_agent.rb

@@ -16,6 +16,7 @@
 # limitations under the License.
 
 require 'fileutils'
+require 'shellwords'
 require 'java_buildpack/component/versioned_dependency_component'
 require 'java_buildpack/framework'
 
@@ -79,30 +80,36 @@ def supports?
       private_constant :CONFIG_FILES, :FILTER
 
       def application_name(java_opts, credentials)
-        name = credentials['application-name'] || @configuration['default_application_name'] ||
-          @application.details['application_name']
-        java_opts.add_system_property('appdynamics.agent.applicationName', "\\\"#{name}\\\"")
+        name = Shellwords.escape(@application.details['application_name'])
+        name = @configuration['default_application_name'] if @configuration['default_application_name']
+        name = Shellwords.escape(credentials['application-name']) if credentials['application-name']
+
+        java_opts.add_system_property('appdynamics.agent.applicationName', name.to_s)
       end
 
       def account_access_key(java_opts, credentials)
         account_access_key = credentials['account-access-key'] || credentials.dig('account-access-secret', 'secret')
+        account_access_key = Shellwords.escape(account_access_key)
+
         java_opts.add_system_property 'appdynamics.agent.accountAccessKey', account_access_key if account_access_key
       end
 
       def account_name(java_opts, credentials)
         account_name = credentials['account-name']
-        java_opts.add_system_property 'appdynamics.agent.accountName', account_name if account_name
+        java_opts.add_system_property 'appdynamics.agent.accountName', Shellwords.escape(account_name) if account_name
       end
 
       def host_name(java_opts, credentials)
         host_name = credentials['host-name']
         raise "'host-name' credential must be set" unless host_name
 
-        java_opts.add_system_property 'appdynamics.controller.hostName', host_name
+        java_opts.add_system_property 'appdynamics.controller.hostName', Shellwords.escape(host_name)
       end
 
       def node_name(java_opts, credentials)
-        name = credentials['node-name'] || @configuration['default_node_name']
+        name = @configuration['default_node_name']
+        name = Shellwords.escape(credentials['node-name']) if credentials['node-name']
+
         java_opts.add_system_property('appdynamics.agent.nodeName', name.to_s)
       end
 
@@ -117,13 +124,17 @@ def ssl_enabled(java_opts, credentials)
       end
 
       def tier_name(java_opts, credentials)
-        name = credentials['tier-name'] || @configuration['default_tier_name'] ||
-          @application.details['application_name']
+        name = Shellwords.escape(@application.details['application_name'])
+        name = @configuration['default_tier_name'] if @configuration['default_tier_name']
+        name = Shellwords.escape(credentials['tier-name']) if credentials['tier-name']
+
         java_opts.add_system_property('appdynamics.agent.tierName', name.to_s)
       end
 
       def unique_host_name(java_opts)
-        name = @configuration['default_unique_host_name'] || @application.details['application_name']
+        name = @configuration['default_unique_host_name']
+        name = Shellwords.escape(@application.details['application_name']) if @application.details['application_name']
+
         java_opts.add_system_property('appdynamics.agent.uniqueHostId', name.to_s)
       end
 

spec/java_buildpack/framework/app_dynamics_agent_spec.rb

@@ -66,7 +66,7 @@
 
         expect(java_opts).to include('-javaagent:$PWD/.java-buildpack/app_dynamics_agent/javaagent.jar')
         expect(java_opts).to include('-Dappdynamics.controller.hostName=test-host-name')
-        expect(java_opts).to include('-Dappdynamics.agent.applicationName=\"test-application-name\"')
+        expect(java_opts).to include('-Dappdynamics.agent.applicationName=test-application-name')
         expect(java_opts).to include('-Dappdynamics.agent.tierName=test-application-name')
         expect(java_opts).to include('-Dappdynamics.agent.nodeName=$(expr "$VCAP_APPLICATION" : ' \
                                      '\'.*instance_index[": ]*\\([[:digit:]]*\\).*\')')
@@ -83,12 +83,38 @@
       end
 
       context do
-        let(:credentials) { super().merge 'application-name' => 'another-test-application-name' }
+        let(:credentials) { super().merge 'tier-name' => 'another-test tier-name' }
 
-        it 'adds application_name from credentials to JAVA_OPTS if specified' do
+        it 'adds tier_name from credentials with space in name to JAVA_OPTS if specified' do
           component.release
 
-          expect(java_opts).to include('-Dappdynamics.agent.applicationName=\"another-test-application-name\"')
+          expect(java_opts).to include('-Dappdynamics.agent.tierName=another-test\ tier-name')
+        end
+      end
+
+      context do
+        let(:credentials) { super().merge 'application-name' => 'another-test application-name' }
+
+        it 'adds application_name from credentials with space in name to JAVA_OPTS if specified' do
+          component.release
+
+          expect(java_opts).to include('-Dappdynamics.agent.applicationName=another-test\ application-name')
+        end
+      end
+
+      context do
+        let(:configuration) do
+          { 'default_tier_name' => nil,
+            'default_node_name' => nil,
+            'default_application_name' => 'default application-name' }
+        end
+
+        it 'adds application_name from default config to JAVA_OPTS if specified' do
+          component.release
+
+          # should not be escaped, escaping happens at runtime because default value is a sub-command
+          #   executed in the runtime container
+          expect(java_opts).to include('-Dappdynamics.agent.applicationName=default application-name')
         end
       end