noxxi / p5-io-socket-ssl

IO::Socket::SSL Perl Module
36 stars 60 forks source link

Passing objects in for filenames fails in a bad way #23

Closed frioux closed 9 years ago

frioux commented 9 years ago

Sometimes I use IO::All or Path::Class file objects. I accidentally passed one in to IO::Socket::SSL as an SSL_ca_file (indirectly, via Net::Async::HTTP) and end up getting really strange errors.

Here's some code:

  use Net::Async::HTTP;
  use IO::Async::Loop;
  use IO::All;
  $loop = IO::Async::Loop->new;
  $loop->add(
     my $ua = Net::Async::HTTP->new(
        SSL_ca_file => io->file('/home/frew/code/root.crt')
     )
  );
  my $res = $ua->GET('https://google.com')->get;
  warn $res->status_line . ' ' . $res->decoded_content;

Here's output:

  google.com:443 - Operation "eq": no method found,
          left argument in overloaded package IO::All::File,
          right argument has no overloaded magic at /home/frew/.plenv/versions/5.20.1/lib/perl5/site_perl/5.20.1/IO/Socket/SSL.pm line 1992.
   failed [Operation "eq": no method found,
          left argument in overloaded package IO::All::File,
          right argument has no overloaded magic at /home/frew/.plenv/versions/5.20.1/lib/perl5/site_perl/5.20.1/IO/Socket/SSL.pm line 1992.
  ] at foo.pl line 10.

Similarly:

  use Net::Async::HTTP;
  use IO::Async::Loop;
  use Path::Class 'file';
  $loop = IO::Async::Loop->new;
  $loop->add(
     my $ua = Net::Async::HTTP->new(
        SSL_ca_file => file('/home/frew/code/root.crt')
     )
  );
  my $res = $ua->GET('https://google.com')->get;
  warn $res->status_line . ' ' . $res->decoded_content;

results in

  google.com:443 - Not a SCALAR reference at /home/frew/.plenv/versions/5.20.1/lib/perl5/site_perl/5.20.1/IO/Socket/SSL.pm line 2006.
   failed [Not a SCALAR reference at /home/frew/.plenv/versions/5.20.1/lib/perl5/site_perl/5.20.1/IO/Socket/SSL.pm line 2006.
  ] at foo.pl line 10.

I'm not saying that I think that I think you should support these objects at all, I just think it would be nice to get a more sensible error message.

For what it's worth, this applies to all of the _file type args, not just the ca one.

So if you can comment on how you'd like it to work, I'd gladly make a patch, but I don't want to work on a patch that does too much etc.

noxxi commented 9 years ago

Right now I think that users should use the module as documented. If they don't they are responsible for the problems they get. If more users run into such problems I might change my mind to check if the user input makes any sense. Alternatively you could contact the author of File::Path so that he provides the path as string by overloading the "" operator.