[PUP-2687] File Names with "?" cause failures with recursive copies Created: 2014/05/28  Updated: 2019/06/14

Status: Accepted
Project: Puppet
Component/s: Types and Providers
Affects Version/s: PUP 3.8.1, PUP 4.2.1
Fix Version/s: None

Type: Bug Priority: Normal
Reporter: redmine.exporter Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: customer, redmine
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PUP-2700 Puppet 3.6.1 File recurse improperly ... Closed
relates to PUP-3135 Square brackets in file names not han... Accepted
relates to PUP-7581 Special Character File Names Fail whe... Accepted
Template:
Epic Link: File Type/Provider Improvements
Team: Coremunity
QA Contact: Narmadha Perumal

 Description   

File names that contain "?" cause runs to fail and reports that it can't find a file name by section before the ?, and seemingly drops the ? and anything after.

Example code:
<pre>
file

{ '/tmp/foo': ensure => directory, recurse => true, purge => true, force => true, owner => 'root', group => 'root', mode => '0644', source => "puppet:///modules/foobar/foo", }

</pre>

Example directory/file structure:
<pre>
├── foobar
│   └── files
│   └── foo
│   ├── a
│   ├── b
│   ├── c
│   └── test?moretest

</pre>

Error:
<pre>
Error: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/foobar/foo/test
Error: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/foobar/foo/test
Wrapped exception:
Error 404 on SERVER: Not Found: Could not find file_content modules/foobar/foo/test
Error: /File[/tmp/foo/test?moretest]/ensure: change from absent to file failed: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/foobar/foo/test
</pre>



 Comments   
Comment by Henrik Lindberg [ 2014/05/28 ]

This reproduces the error

file { '/tmp/source':
  ensure => directory,
}
 
file { 'problem_file':
  ensure => file,
  path   => '/tmp/source/?',
}
 
file { '/tmp/dest':
  ensure  => directory,
  recurse => true, 
  purge   => true, 
  force   => true, 
  source  => '/tmp/source',
  require => File['problem_file'],
}

Comment by Henrik Lindberg [ 2014/05/28 ]

And here is a resulting stack trace

Wrapped exception:
Is a directory - /tmp/source/
/Users/henrik/git/puppet/lib/puppet/file_system/file19.rb:3:in `binread'
/Users/henrik/git/puppet/lib/puppet/file_system/file19.rb:3:in `binread'
/Users/henrik/git/puppet/lib/puppet/file_system/file19.rb:3:in `binread'
/Users/henrik/git/puppet/lib/puppet/file_system.rb:137:in `binread'
/Users/henrik/git/puppet/lib/puppet/file_serving/content.rb:37:in `content'
/Users/henrik/git/puppet/lib/puppet/type/file/source.rb:106:in `content'
/Users/henrik/git/puppet/lib/puppet/type/file/content.rb:186:in `each_chunk_from'
/Users/henrik/git/puppet/lib/puppet/type/file/content.rb:169:in `block in write'
/Users/henrik/git/puppet/lib/puppet/util/checksums.rb:99:in `md5_stream'
/Users/henrik/git/puppet/lib/puppet/type/file/checksum.rb:32:in `sum_stream'
/Users/henrik/git/puppet/lib/puppet/type/file/content.rb:168:in `write'
/Users/henrik/git/puppet/lib/puppet/type/file.rb:897:in `write_content'
/Users/henrik/git/puppet/lib/puppet/type/file.rb:801:in `block in write'
/Users/henrik/git/puppet/lib/puppet/util.rb:429:in `replace_file'
/Users/henrik/git/puppet/lib/puppet/type/file.rb:799:in `write'
/Users/henrik/git/puppet/lib/puppet/type/file/content.rb:149:in `sync'
/Users/henrik/git/puppet/lib/puppet/type/file/ensure.rb:65:in `block (2 levels) in <module:Puppet>'
/Users/henrik/git/puppet/lib/puppet/property.rb:197:in `call_valuemethod'
/Users/henrik/git/puppet/lib/puppet/property.rb:498:in `set'
/Users/henrik/git/puppet/lib/puppet/property.rb:581:in `sync'
/Users/henrik/git/puppet/lib/puppet/type/file/ensure.rb:183:in `sync'
/Users/henrik/git/puppet/lib/puppet/transaction/resource_harness.rb:191:in `sync'
/Users/henrik/git/puppet/lib/puppet/transaction/resource_harness.rb:128:in `sync_if_needed'
/Users/henrik/git/puppet/lib/puppet/transaction/resource_harness.rb:81:in `perform_changes'
/Users/henrik/git/puppet/lib/puppet/transaction/resource_harness.rb:20:in `evaluate'
/Users/henrik/git/puppet/lib/puppet/transaction.rb:174:in `apply'
/Users/henrik/git/puppet/lib/puppet/transaction.rb:187:in `eval_resource'
/Users/henrik/git/puppet/lib/puppet/transaction.rb:117:in `call'
/Users/henrik/git/puppet/lib/puppet/transaction.rb:117:in `block (2 levels) in evaluate'
/Users/henrik/git/puppet/lib/puppet/util.rb:327:in `block in thinmark'
/Users/henrik/.rvm/rubies/ruby-1.9.3-p448/lib/ruby/1.9.1/benchmark.rb:295:in `realtime'
/Users/henrik/git/puppet/lib/puppet/util.rb:326:in `thinmark'
/Users/henrik/git/puppet/lib/puppet/transaction.rb:117:in `block in evaluate'
/Users/henrik/git/puppet/lib/puppet/graph/relationship_graph.rb:118:in `traverse'
/Users/henrik/git/puppet/lib/puppet/transaction.rb:108:in `evaluate'
/Users/henrik/git/puppet/lib/puppet/resource/catalog.rb:169:in `block in apply'
/Users/henrik/git/puppet/lib/puppet/util/log.rb:149:in `with_destination'
/Users/henrik/git/puppet/lib/puppet/transaction/report.rb:112:in `as_logging_destination'
/Users/henrik/git/puppet/lib/puppet/resource/catalog.rb:168:in `apply'
/Users/henrik/git/puppet/lib/puppet/configurer.rb:117:in `block in apply_catalog'
/Users/henrik/git/puppet/lib/puppet/util.rb:161:in `block in benchmark'
/Users/henrik/.rvm/rubies/ruby-1.9.3-p448/lib/ruby/1.9.1/benchmark.rb:295:in `realtime'
/Users/henrik/git/puppet/lib/puppet/util.rb:160:in `benchmark'
/Users/henrik/git/puppet/lib/puppet/configurer.rb:116:in `apply_catalog'
/Users/henrik/git/puppet/lib/puppet/configurer.rb:198:in `run'
/Users/henrik/git/puppet/lib/puppet/application/apply.rb:288:in `apply_catalog'
/Users/henrik/git/puppet/lib/puppet/application/apply.rb:228:in `block in main'
/Users/henrik/git/puppet/lib/puppet/context.rb:64:in `override'
/Users/henrik/git/puppet/lib/puppet.rb:237:in `override'
/Users/henrik/git/puppet/lib/puppet/application/apply.rb:190:in `main'
/Users/henrik/git/puppet/lib/puppet/application/apply.rb:151:in `run_command'
/Users/henrik/git/puppet/lib/puppet/application.rb:384:in `block (2 levels) in run'
/Users/henrik/git/puppet/lib/puppet/application.rb:510:in `plugin_hook'
/Users/henrik/git/puppet/lib/puppet/application.rb:384:in `block in run'
/Users/henrik/git/puppet/lib/puppet/util.rb:479:in `exit_on_fail'
/Users/henrik/git/puppet/lib/puppet/application.rb:384:in `run'
/Users/henrik/git/puppet/lib/puppet/util/command_line.rb:145:in `run'
/Users/henrik/git/puppet/lib/puppet/util/command_line.rb:91:in `execute'
/Users/henrik/git/puppet/bin/puppet:4:in `<top (required)>'
/Users/henrik/.rvm/gems/ruby-1.9.3-p448/bin/puppet:23:in `load'
/Users/henrik/.rvm/gems/ruby-1.9.3-p448/bin/puppet:23:in `<main>'

Comment by Rémi [ 2014/05/28 ]

Hello,

I've got a similar problem since my update in 3.6.1. With this manifest :

file { "${mcollective_plugin_path}/agent":
    ensure  => directory,
    recurse => true,
    purge   => true,
    mode    => '0644',
    tag     => ['mcolplugin', 'mcolagents'],
    source  => 'puppet:///modules/mcollective/agent',
    notify  => Class['mcollective::service'],
  }

With a 3.6.1 agent :

# rm /usr/share/mcollective/plugins/mcollective/agent/__ATTENTION\ REP\ PUPPET__
# puppet agent -t
[...]
Error: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/__ATTENTION%20REP%20PUPPET__
Error: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/__ATTENTION%20REP%20PUPPET__
Wrapped exception:
Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/__ATTENTION%20REP%20PUPPET__
Error: /Stage[main]/Mcollective::Plugin/File[/usr/share/mcollective/plugins/mcollective/agent/__ATTENTION REP PUPPET__]/ensure: change from absent to file failed: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/__ATTENTION%20REP%20PUPPET__
Notice: /Stage[main]/Mcollective::Service/Service[mcollective]: Dependency File[/usr/share/mcollective/plugins/mcollective/agent/__ATTENTION REP PUPPET__] has failures: true
Warning: /Stage[main]/Mcollective::Service/Service[mcollective]: Skipping because of failed dependencies

And with a 3.5.1 agent :

Info: Applying configuration version '1401282140'
Notice: /Stage[main]/Mcollective::Plugin/File[/usr/share/mcollective/plugins/mcollective/agent/__ATTENTION REP PUPPET__]/ensure: defined content as '{md5}d41d8cd98f00b204e9800998ecf8427e'
Info: /usr/share/mcollective/plugins/mcollective/agent: Scheduling refresh of Class[Mcollective::Service]
Info: Class[Mcollective::Service]: Scheduling refresh of Service[mcollective]
Notice: /Stage[main]/Mcollective::Service/Service[mcollective]: Triggered 'refresh' from 1 events

Thanks for your help

Comment by Kylo Ginsberg [ 2014/05/29 ]

Rémi this issue (with "?") seems to be an older issue, i.e. it pre-dates 3.5.1. The example you mentioned is apparently a regression between 3.5.1 and 3.6.1 and involves a space rather than a "?". If so, it may actually be PUP-2700.

Comment by Rémi [ 2014/05/30 ]

Hello,

Yes PUP-2700 seems to be the same bug... In fact, several characters seem problematic :
If I've got a file "TEST#TEST" in my directory :

Error: /Stage[main]/Mcollective::Plugin/File[/usr/share/mcollective/plugins/mcollective/agent/TEST#TEST]/ensure: change from absent to file failed: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/TEST%23TEST

With a file "TEST?TEST" :

Error: /Stage[main]/Mcollective::Plugin/File[/usr/share/mcollective/plugins/mcollective/agent/TEST?TEST]/ensure: change from absent to file failed: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/TEST

With a space "TEST TEST" :

Error: /Stage[main]/Mcollective::Plugin/File[/usr/share/mcollective/plugins/mcollective/agent/TEST TEST]/ensure: change from absent to file failed: Could not set 'file' on ensure: Error 404 on SERVER: Not Found: Could not find file_content modules/mcollective/agent/TEST%20TEST

No problems with all these characters with my client in 3.5.1.

Thanks for your help

Comment by Daniel Patrick Johnson [ 2014/06/02 ]

There was some suspicion that the specific characters that break have to do with puppet internally converting file paths into URL's such that they can be copied over the network the same as locally. There are standard ways for converting URL's, but the ruby language standard URI library has an escape method that misses '?'. It is also deprecated, and used by puppet. I tried to replace the escaping with the CGI.escape function. It also escapes the '/' character though which breaks creating a URL so it wasn't a super simple replacement. It didn't work as a fix, but it might be part of the solution so I'm including it here so that no one else has to figure out how to get all the URL escaping right.

diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index a8b78da..308c0b5 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -4,6 +4,7 @@ require 'English'
 require 'puppet/error'
 require 'puppet/util/execution_stub'
 require 'uri'
+require 'cgi'
 require 'tempfile'
 require 'pathname'
 require 'ostruct'
@@ -258,7 +259,11 @@ module Util
       end
     end
 
-    params[:path] = URI.escape(path)
+    path_array = Pathname(path).each_filename.
+      to_a.map{ | part | Pathname(CGI.escape(part)) }
+    path_array.unshift(Pathname('/'))
+    full_pathname = path_array.inject{ |path, part| path + part}
+    params[:path] = full_pathname.to_s
 
     begin
       URI::Generic.build(params)

Comment by Branan Riley [ 2017/05/15 ]

It is totally embarrassing that we haven't fixed this yet. AFAICT we still do not CGI escape filenames in path_to_uri. Unless it was fixed at a different layer in a recent puppet, I'm assuming this is still valid

Generated at Sat Dec 07 20:52:48 PST 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.