This commit changes underlying file handling and known hosts parsing.
A known hosts file opened through Load() never closed the underlying
file. During known hosts parsing most errors were unchecked, or just
led to the line being skipped.
I removed the KnownHosts type, which didn't really have a role after
the refactor. The embedding of KnownHosts in KnownHosts file has been
removed as it also leaked the map unprotected by the mutex.
The Fingerprint type is now KnownHost and has taken over the
responsibility of marshalling and unmarshalling.
SetOutput now takes a WriteCloser so that we can close the underlying
writer when it's replaced, or when it's explicitly closed through the
new Close() function.
KnownHostsFile.Add() now also writes the known host to the output if
set. I think that makes sense expectation-wise for the type.
Turned WriteAll() into WriteTo() to conform with the io.WriterTo
interface.
Load() is now Open() to better reflect the fact that a file is opened,
and kept open. It can now also return errors from the parsing process.
The parser does a lot more error checking, and this might be an area
where I've changed a desired behaviour as invalid entries no longer
are ignored, but aborts the parsing process. That could be changed to
a warning, or some kind of parsing feedback.
I added KnownHostsFile.TOFU() to fill the developer experience gap
that was left after the client no longer knows about
KnownHostsFile. It implements a basic non-interactive TOFU flow.
Client.Timout isn't respected for the dial. Requests will hang on dial
until OS-level timouts kick in unless there is a Request.Context with
a deadline. We also fail to close the connection on errors.
This change sets the client timeout as the dialer timeout so that it
will be respected. It also ensures that we close the connection if we
fail to make the request.
Error handling is currently missing is a couple of places. Most of
them are i/o related.
This change adds checks, an therefore sometimes also has to change
function signatures by adding an error return value. In the case of
the response writer the status and meta handling is changed and this
also breaks the API.
In some places where we don't have any reasonable I've added
assignment to a blank identifier to make it clear that we're ignoring
an error.
text: read the Err() that can be set by the scanner.
client: check if conn.SetDeadline() returns an error.
client: check if req.Write() returns an error.
fs: panic if mime type registration fails.
server: stop performing i/o in Header/Status functions
By deferring the actual header write to the first Write() or Flush()
call we don't have to do any error handling in Header() or Status().
As Server.respond() now defers a ResponseWriter.Flush() instead of
directly flushing the underlying bufio.Writer this has the added
benefit of ensuring that we always write a header
to the client, even if the responder is a complete NOOP.
tofu: return an error if we fail to write to the known hosts writer.
A request to a hostname that hasn't been registered with the server
currently results in a nil pointer deref panic in server.go:215 as
request handling continues even if ReadRequest() returns an error.
This change changes all if-else error handling in Server.respond() to
a WriteStatus-call and early return. This makes it clear when request
handling is aborted (and actually aborts when ReadRequest() fails).