On Tue, Jul 19, 2011 at 14:59, Josh Cooper <j...@puppetlabs.com> wrote:
> When testing whether a file path is absolute, the regexp was only
> handling POSIX style file paths. This commit requires Windows
> style file paths to start with a drive letter. A future commit
> will refacter the various places we do path validation to
> support both Windows drive letters and UNC paths.
>
> Reviewed-by: Jacob Helwig <ja...@puppetlabs.com>
> Signed-off-by: Josh Cooper <j...@puppetlabs.com>
> - path = path.chomp(File::SEPARATOR) unless path == File::SEPARATOR
> - raise ArgumentError.new("Fileset paths must be fully qualified") unless
> File.expand_path(path) == path
> + if Puppet.features.microsoft_windows?
> + # REMIND: UNC path
> + path = path.chomp(File::SEPARATOR) unless path =~ /^[A-Za-z]:\/$/
Doesn't this fail in the face of platforms that have both
File::SEPARATOR and File::ALT_SEPARATOR? It wouldn't strip the right
character from the end of the path, at least, in one of the two valid
cases.
> + else
> + path = path.chomp(File::SEPARATOR) unless path == File::SEPARATOR
> + end
Wouldn't it be better to have unified these tests, so that we don't
have to open-code these Windows-specific tests in every line of code
that needs to handle paths? That sounds like an invitation to a
constant stream of bugs where we fail to handle these cases to me.
> def validate_dirs(dirs)
> + dir_regex = Puppet.features.microsoft_windows? ?
> /^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/
This will fail if File::SEPARATOR is '\', because you just shoved a
regexp escape character into your string. On the plus side, though,
because you are not actually getting it right WRT ALT_SEPARATOR, this
won't blow up. Two bugs make it work. ;)
> - unless file =~ /^#{File::SEPARATOR}/
> + regex = Puppet.features.microsoft_windows? ?
> /^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/
Hey, I think I recognise this code. Maybe we should have somewhere
central for it, rather than open-coding it every place that needs it?
Daniel
--
⎋ Puppet Labs Developer – http://puppetlabs.com
✉ Daniel Pittman <dan...@puppetlabs.com>
✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775
♲ Made with 100 percent post-consumer electrons
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
