chirp: handle multiline responses from BIRD

Also add tests to verify the parsing logic.

Signed-off-by: Maisem Ali <maisem@tailscale.com>

chirp/chirp.go

@@ -10,6 +10,7 @@ import (
 	"bufio"
 	"fmt"
 	"net"
+	"regexp"
 	"strings"
 )
 
@@ -19,9 +20,9 @@ func New(socket string) (*BIRDClient, error) {
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to BIRD: %w", err)
 	}
-	b := &BIRDClient{socket: socket, conn: conn, bs: bufio.NewScanner(conn)}
+	b := &BIRDClient{socket: socket, conn: conn, scanner: bufio.NewScanner(conn)}
 	// Read and discard the first line as that is the welcome message.
-	if _, err := b.readLine(); err != nil {
+	if _, err := b.readResponse(); err != nil {
 		return nil, err
 	}
 	return b, nil
@@ -29,9 +30,9 @@ func New(socket string) (*BIRDClient, error) {
 
 // BIRDClient handles communication with the BIRD Internet Routing Daemon.
 type BIRDClient struct {
-	socket string
-	conn   net.Conn
-	bs     *bufio.Scanner
+	socket  string
+	conn    net.Conn
+	scanner *bufio.Scanner
 }
 
 // Close closes the underlying connection to BIRD.
@@ -39,7 +40,7 @@ func (b *BIRDClient) Close() error { return b.conn.Close() }
 
 // DisableProtocol disables the provided protocol.
 func (b *BIRDClient) DisableProtocol(protocol string) error {
-	out, err := b.exec("disable %s\n", protocol)
+	out, err := b.exec("disable %s", protocol)
 	if err != nil {
 		return err
 	}
@@ -53,7 +54,7 @@ func (b *BIRDClient) DisableProtocol(protocol string) error {
 
 // EnableProtocol enables the provided protocol.
 func (b *BIRDClient) EnableProtocol(protocol string) error {
-	out, err := b.exec("enable %s\n", protocol)
+	out, err := b.exec("enable %s", protocol)
 	if err != nil {
 		return err
 	}
@@ -65,19 +66,51 @@ func (b *BIRDClient) EnableProtocol(protocol string) error {
 	return fmt.Errorf("failed to enable %s: %v", protocol, out)
 }
 
+// BIRD CLI docs from https://bird.network.cz/?get_doc&v=20&f=prog-2.html#ss2.9
+
+// Each session of the CLI consists of a sequence of request and replies,
+// slightly resembling the FTP and SMTP protocols.
+// Requests are commands encoded as a single line of text,
+// replies are sequences of lines starting with a four-digit code
+// followed by either a space (if it's the last line of the reply) or
+// a minus sign (when the reply is going to continue with the next line),
+// the rest of the line contains a textual message semantics of which depends on the numeric code.
+// If a reply line has the same code as the previous one and it's a continuation line,
+// the whole prefix can be replaced by a single white space character.
+//
+// Reply codes starting with 0 stand for ‘action successfully completed’ messages,
+// 1 means ‘table entry’, 8 ‘runtime error’ and 9 ‘syntax error’.
+
 func (b *BIRDClient) exec(cmd string, args ...interface{}) (string, error) {
 	if _, err := fmt.Fprintf(b.conn, cmd, args...); err != nil {
 		return "", err
 	}
-	return b.readLine()
+	fmt.Fprintln(b.conn)
+	return b.readResponse()
 }
 
-func (b *BIRDClient) readLine() (string, error) {
-	if !b.bs.Scan() {
-		return "", fmt.Errorf("reading response from bird failed")
-	}
-	if err := b.bs.Err(); err != nil {
-		return "", err
+var respCodeRegex = regexp.MustCompile(`^\d{4}[ -]`)
+
+func (b *BIRDClient) readResponse() (string, error) {
+	var resp strings.Builder
+	var done bool
+	for !done {
+		if !b.scanner.Scan() {
+			return "", fmt.Errorf("reading response from bird failed: %q", resp.String())
+		}
+		if err := b.scanner.Err(); err != nil {
+			return "", err
+		}
+		out := b.scanner.Bytes()
+		if _, err := resp.Write(out); err != nil {
+			return "", err
+		}
+		if respCodeRegex.Match(out) {
+			done = out[4] == ' '
+		}
+		if !done {
+			resp.WriteRune('\n')
+		}
 	}
-	return b.bs.Text(), nil
+	return resp.String(), nil
 }

chirp/chirp_test.go

@@ -0,0 +1,111 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+package chirp
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"net"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+type fakeBIRD struct {
+	net.Listener
+	protocolsEnabled map[string]bool
+	sock             string
+}
+
+func newFakeBIRD(t *testing.T, protocols ...string) *fakeBIRD {
+	sock := filepath.Join(t.TempDir(), "sock")
+	l, err := net.Listen("unix", sock)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pe := make(map[string]bool)
+	for _, p := range protocols {
+		pe[p] = false
+	}
+	return &fakeBIRD{
+		Listener:         l,
+		protocolsEnabled: pe,
+		sock:             sock,
+	}
+}
+
+func (fb *fakeBIRD) listen() error {
+	for {
+		c, err := fb.Accept()
+		if err != nil {
+			if errors.Is(err, net.ErrClosed) {
+				return nil
+			}
+			return err
+		}
+		go fb.handle(c)
+	}
+}
+
+func (fb *fakeBIRD) handle(c net.Conn) {
+	fmt.Fprintln(c, "0001 BIRD 2.0.8 ready.")
+	sc := bufio.NewScanner(c)
+	for sc.Scan() {
+		cmd := sc.Text()
+		args := strings.Split(cmd, " ")
+		switch args[0] {
+		case "enable":
+			en, ok := fb.protocolsEnabled[args[1]]
+			if !ok {
+				fmt.Fprintln(c, "9001 syntax error, unexpected CF_SYM_UNDEFINED, expecting CF_SYM_KNOWN or TEXT or ALL")
+			} else if en {
+				fmt.Fprintf(c, "0010-%s: already enabled\n", args[1])
+			} else {
+				fmt.Fprintf(c, "0011-%s: enabled\n", args[1])
+			}
+			fmt.Fprintln(c, "0000 ")
+			fb.protocolsEnabled[args[1]] = true
+		case "disable":
+			en, ok := fb.protocolsEnabled[args[1]]
+			if !ok {
+				fmt.Fprintln(c, "9001 syntax error, unexpected CF_SYM_UNDEFINED, expecting CF_SYM_KNOWN or TEXT or ALL")
+			} else if !en {
+				fmt.Fprintf(c, "0008-%s: already disabled\n", args[1])
+			} else {
+				fmt.Fprintf(c, "0009-%s: disabled\n", args[1])
+			}
+			fmt.Fprintln(c, "0000 ")
+			fb.protocolsEnabled[args[1]] = false
+		}
+	}
+}
+
+func TestChirp(t *testing.T) {
+	fb := newFakeBIRD(t, "tailscale")
+	defer fb.Close()
+	go fb.listen()
+	c, err := New(fb.sock)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := c.EnableProtocol("tailscale"); err != nil {
+		t.Fatal(err)
+	}
+	if err := c.EnableProtocol("tailscale"); err != nil {
+		t.Fatal(err)
+	}
+	if err := c.DisableProtocol("tailscale"); err != nil {
+		t.Fatal(err)
+	}
+	if err := c.DisableProtocol("tailscale"); err != nil {
+		t.Fatal(err)
+	}
+	if err := c.EnableProtocol("rando"); err == nil {
+		t.Fatalf("enabling %q succeded", "rando")
+	}
+	if err := c.DisableProtocol("rando"); err == nil {
+		t.Fatalf("disabling %q succeded", "rando")
+	}
+}