Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't delete temporary file before using it #426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justafish
Copy link

Pull Request (PR) description

I encountered an LDAP error 80 (LDAP_OTHER) whilst adding SSL certificates:

 class { 'openldap::server':
        ssl_cert   => '/etc/ssl/certs/foo.com.crt',
        ssl_key    => '/etc/ssl/private/foo.com.key',
        ssl_ca     => '/etc/ssl/certs/foo.com.ca-bundle',
    }

This Pull Request (PR) fixes the following issues

Doesn't delete temporary files before they're used

@smortex
Copy link
Member

smortex commented Jun 20, 2024

I am not sure to follow: closing a (Temp)File does not remove it.

According to the documentation:

When a Tempfile object is garbage collected, or when the Ruby interpreter exits, its associated temporary file is automatically deleted

In this case, the garbage collection of the object would not happen before the execution flow exited the function that created the temporary file, which is the scope of the local variable.

We explicitly #close the file to make sure data written to it was flushed to disk before running a command that depend on the file content. We can probably do this another way (there is probably a #flush method), but the neat result should be the same, and as we will not write more data it feels more natural to just close the file IMHO.

Maybe you experienced some race condition the module does not handle correctly? Are you able to reproduce your error? Can you provide a manifest that we can use to reproduce the error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants