Re: [JGIT PATCH] Test and fix handling of quotes in ~/.ssh/config

[Jonas Fonseca <fonseca@diku.dk>]

Shawn O. Pearce <spearce@spearce.org> wrote Mon, Sep 22, 2008:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > söndagen den 21 september 2008 13.25.19 skrev Jonas Fonseca:
> > > +		assertEquals("bad.tld\"", osc.lookup("bad").getHostName());
> > This one is really (as you noted) bad so we shouldn't allow it at all. A new 
> > subclass of TransportExcpeption should be thrown to indicate a serious
> > configuration problem when attempting to use the option.
>
> Probably so.
> 
> But then we need to mark that the Host is invalid, because we
> are serving requests from a cache, not from the file itself.
> And TransportException isn't something that the SshSessionFactory
> knows about.  Probably better to use a a subclass of IOException.

Sorry, for not following up on this. I was trying to cook up a patch for
this today. Now, it is somehow sad that testing "forces" us to waste
time on these stupid corner cases. ;-) On the other hand, this problem
might exist (.git/config) or turn up again, so it would be good to have
a design principle.

Using exceptions seems a bit harsh, since the quote is not really fatal
in anyway. Also, for badly formatted Port values the value is simply
ignored. So for bad quoting encountered during non-Host values, I think
it is fair to just ignore the value. For Host values it is a bit more
non-obvious to me. In terms of invalidating hosts, the API ensures that
a lookup will always return a host, so invalid hosts should not return
null. I propose to simply remove these hosts from the host map and clear
the current host list so that no values will be saved, effectively
causing invalid hosts to result in the same as unknown hosts.

-- 
Jonas Fonseca