ingydotnet / io-all-pm

All in One Perl IO
http://search.cpan.org/dist/IO-All/
38 stars 20 forks source link

catfile() bug when protocol in dirname (url) #50

Closed hginzel closed 9 years ago

hginzel commented 10 years ago

Hello,

try please

use IO::All;
my(%opt) = (
        url             => 'ftp://my_host.local/',
        remote_dir      => 'directory',
        file_name       => 'file.txt',
);
warn "IO::All::VERSION = $IO::All::VERSION";
warn io->catfile(@opt{qw/url remote_dir file_name/});

It returns

IO::All::VERSION = 0.67 at io_all_catfilebug.pl line 9.
ftp:/my_host.local/directory/file.txt at io_all_catfilebug.pl line 10.

instaed of ftp://my_host.local/directory/file.txt – the slash after protocol is missing.

Regards HG

357r4bd commented 9 years ago

This is an artifact of how File:Spec works:

use File::Spec;
my(%opt) = (
    url             => 'ftp://my_host.local/',
    remote_dir      => 'directory',
    file_name       => 'file.txt',
);
warn File::Spec->catfile(@opt{qw/url remote_dir file_name/});

Returns:

ftp:/my_host.local/directory/file.txt at test.pl line 7.
wchristian commented 9 years ago

@ginzel I think @estrabd is correct here. You're trying to build a URL, but you're using path manipulating tools for that. This is especially incorrect, as you'll get wildly different fresults from catfile, depending on what OS you're on. That catfile can sometimes be used successfully to build URLs on unix is merely an accident.

@frew, @ingydotnet : I think this can be closed, unless maybe you want to add some code that recognizes such accidental misuses and warns/dies upon them?

frew commented 9 years ago

I know nothing about this project. Wrong frew?

wchristian commented 9 years ago

@frew: Oops, yes, sorry. I meant @frioux. :)

frioux commented 9 years ago

lol, arg it's my nemesis @frew who got my username after me because when I signed up it was 5 chars or more!

@wchristian I agree; closing unless @ingydotnet thinks it should work.

hginzel commented 9 years ago

In that case, add please a "safe" function for concatenating url parts, let say io->concat[enate]. Add a warning into documentation about catfile, please. @frioux, @ingydotnet please reopen the issue.

frioux commented 9 years ago

reopened, though I don't think concat is nearly obvious enough. Maybe url_concat though.

357r4bd commented 9 years ago

Why not just create a new issue. This issue was closed because it was a non-issue on IO::All's part. What I am reading above from @ginzel sounds like a feature request.

frioux commented 9 years ago

Agreed. @ginzel, create an issue that describes what you want please.

sent from a rotary phone, pardon my brevity On Apr 1, 2015 2:07 PM, "B. Estrade" notifications@github.com wrote:

Why not just create a new issue. This issue was closed because it was a non-issue on IO::All's part. What I am reading above from @ginzel https://github.com/ginzel sounds like a feature request.

— Reply to this email directly or view it on GitHub https://github.com/ingydotnet/io-all-pm/issues/50#issuecomment-88597102.