From 308d072f9433d18bdfc01cec703552b166f420e0 Mon Sep 17 00:00:00 2001 From: Alexander Fisher Date: Wed, 23 Oct 2024 10:52:02 +0100 Subject: [PATCH] Fix race condition by memoizing `Tempfile` objects Previously, the `certificate` and `private_key` methods created `Tempfile` objects without maintaining references after the methods exited. This allowed the Ruby garbage collector to collect and delete these temporary files at any time. As a result, there was a race condition where the `keytool` binary could fail with a `java.io.FileNotFoundException` if the temporary files were deleted before `keytool` accessed them. This commit resolves the issue by memoizing the `Tempfile` objects using instance variables (`@temp_certificate_file` and `@temp_private_key_file`). By caching these objects, we prevent them from being garbage collected prematurely, ensuring that the temporary files remain available for the duration of the provider instance. Fixes #425 --- lib/puppet/provider/java_ks/keytool.rb | 35 +++++++++++++++----------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/puppet/provider/java_ks/keytool.rb b/lib/puppet/provider/java_ks/keytool.rb index 3bbef320..b80a80e8 100644 --- a/lib/puppet/provider/java_ks/keytool.rb +++ b/lib/puppet/provider/java_ks/keytool.rb @@ -284,13 +284,17 @@ def certificate # When no certificate file is specified, we infer the usage of # certificate content and create a tempfile containing this value. # we leave it to to the tempfile to clean it up after the pupet run exists. - file = Tempfile.new('certificate') - # Check if the specified value is a Sensitive data type. If so, unwrap it and use - # the value. - content = @resource[:certificate_content].respond_to?(:unwrap) ? @resource[:certificate_content].unwrap : @resource[:certificate_content] - file.write(content) - file.close - file.path + @temp_certificate_file ||= begin + file = Tempfile.new('certificate') + # Check if the specified value is a Sensitive data type. If so, unwrap it and use + # the value. + content = @resource[:certificate_content].respond_to?(:unwrap) ? @resource[:certificate_content].unwrap : @resource[:certificate_content] + file.write(content) + file.close + file + end + + @temp_certificate_file.path end def private_key @@ -300,13 +304,16 @@ def private_key # When no private key file is specified, we infer the usage of # private key content and create a tempfile containing this value. # we leave it to to the tempfile to clean it up after the pupet run exists. - file = Tempfile.new('private_key') - # Check if the specified value is a Sensitive data type. If so, unwrap it and use - # the value. - content = @resource[:private_key_content].respond_to?(:unwrap) ? @resource[:private_key_content].unwrap : @resource[:private_key_content] - file.write(content) - file.close - file.path + @temp_private_key_file ||= begin + file = Tempfile.new('private_key') + # Check if the specified value is a Sensitive data type. If so, unwrap it and use + # the value. + content = @resource[:private_key_content].respond_to?(:unwrap) ? @resource[:private_key_content].unwrap : @resource[:private_key_content] + file.write(content) + file.close + file + end + @temp_private_key_file.path end def private_key_type