From: Robert Klemme <bob.news@...>
Date: 2005-09-23T01:46:40+09:00
Subject: Re: Net::Netrc request for comments

Bob Showalter <Bob_Showalter@taylorwhite.com> wrote:
> Hi,
>
> I'm a Perl-er learning ruby. I'm creating a ruby port of Perl's
> Net::Netrc module for accessing ftp(1)'s .netrc file. I couldn't find
> an existing ruby version anywhere.
>
> My current code is below. I've essentially ported the Perl module's
> usage semantics, although my internal implementation is quite
> different. I'd sure appreciate any feedback, especially related to
> doing things the "ruby way", with a view toward submitting this code
> to the appropriate archive site(s) at some point.
>
> Example usage:
>
>  require 'net/netrc'
>
>  n = Net::Netrc.locate('example.com')
>  if (n.nil?)
>    puts "No entry found"
>  else
>    puts "login = #{n.login}, password = #{n.password}"
>  end
>
> Questions:
>
> 1. Should I use locate() instead of new()? locate() is how the Perl
> module works.

I'd use locate only and probably make new private.  Reason is that locate 
will not always return a valid object.

> 2. Should I return nil if no entry is found, or should I return an
> object with the accessors all nil?

Return nil

> 3. Should I be returning a Net::Netrc object at all, or just a simple
> Hash? Or should Net::Netrc sublcass Hash?

Definitely not the latter.  I'd probably use an instance of Net::Netrc and 
add some methods (who says the Ruby version must not be better than the Perl 
version)?  For example, I'd add a method that opens a connection with the 
info you have:

class Netrc
  def open(do_init = true)
    conn = Ftp.open machine
    begin
      conn.login login, password
      # add code that executes all default actions if defined, maybe 
argument flag controlled
    rescue Net::FTPPermError
       conn.close
       raise
    end

    if block_given?
      begin
        yield conn
      ensure
        conn.close
      end

      nil
    else
      conn
    end
  end
end

Then you can do

n = Net::Netrc.locate('example.com')

if n
  n.open do |ftp|
    files = ftp.chdir('pub/lang/ruby/contrib')
    files = ftp.list('n*')
    ftp.getbinaryfile('nif.rb-0.91.gz', 'nif.gz', 1024)
  end
else
  puts "not found"
end

You could even make this more convenient by adding a class method open that 
does this

class Netrc
  def self.open(machine, do_init = true, &b)
    n = locate machine
    raise "error" unless n
    n.open(do_init, &b)
  end
end

Then you can do

Net::Netrc.open('ftp.foo.bar') do |ftp|
  ...
end

Uh, just detected a name clash.  I'd rename your open as this seems to be a 
rather internal method.

> 4. Is rasing a SecurityError appropriate? Should I create a specific
> exception class instead?

I think it's appropriate.  However, it seems to me that if you want to mimic 
FTP's behavior then you should raise the exception only if you find a 
password:
http://www.die.net/doc/linux/man/man5/netrc.5.html

> ------- BEGIN CODE --------
>
> module Net
>
>  class Netrc
>
>    attr_accessor :machine, :login, :password, :account
>
>    # returns name of .netrc file
>    def Netrc.rcname
>      # TODO: cross platform? getpwuid() for home dir?
>      home = ENV['HOME']
>      home ||= ENV['HOMEDRIVE'] + (ENV['HOMEPATH'] || '') if
> ENV['HOMEDRIVE']
>      File.join(home, '.netrc')
>    end
>
>    # opens .netrc file, returning File object if successful.
>    # returns nil if .netrc not found.
>    # raises SecurityError if .netrc is not owned by the current.
>    # user or if it is readable or writable by other than the
>    # current user.
>    def Netrc.open
>      name = rcname
>      return nil unless File.exist?(name)
>      # TODO: this stat code not applicable to Win32 (and others?)
>      s = File.stat(name)
>      raise SecurityError, "Not owner: #{name}" unless s.owned?
>      raise SecurityError, "Bad permissions: #{name}" if s.mode & 077
>      != 0 File.open(name, 'r')
>    end
>
>    # given a machine name, returns a Net::Netrc object containing
>    # the matching entry for that name, or the default entry. If
>    # no match is found an no default entry exists, nil is returned.
>    def Netrc.locate(mach)
>      f = open or return nil
>      entry = nil
>      key = nil
>      inmacdef = false
>      while line = f.gets
>        if inmacdef
>          inmacdef = false if line.strip.empty?
>          next
>        end
>        toks =
>        line.scan(/"((?:\\.|[^"])*)"|((?:\\.|\S)+)/).flatten.compact
>        toks.each { |t| t.gsub!(/\\(.)/, '\1') } while toks.length > 0

# what do you need that "while toks.length > 0" for?  Seems completely 
superfluous to me.  Or is this an indentation problem and the "while" should 
have been on the next line?  Hmm, probably...

I'd probably do the parsing a bit different: I would have to think about 
this a bit more but I'd keep a set of settings for default and a set for the 
machine and directly return if I found the machine.  Just a rough idea...

>          tok = toks.shift
>          if key
>            entry = new if key == 'machine' && tok == mach
>            entry.send "#{key}=", tok if entry
>            key = nil
>          end
>          case tok
>          when 'default'
>            return entry if entry
>            entry = new
>          when 'machine'
>            return entry if entry
>            key = 'machine'
>          when 'login', 'password', 'account'
>            key = tok
>          end
>        end
>      end
>      entry
>    end
>
>  end
>
> end
>
> ------- END CODE --------
>
> TIA for any feedback,
>
> Bob

You're welcome!

Kind regards

    robert