fakeserver, conformance, nntp-fakeserver: fixes from the v0.14.0 review

README.md

@@ -554,7 +554,9 @@ the order of `Names()`), so one failure cannot poison the next. The checks that
 all the others, so no article arrives in `PostGroup` while a reader check counts its articles. Before the dial a check is skipped when it is
 in `Options.Skip` ("skipped by option"), needs credentials without `User` or posts without
 `PostGroup`; after the dial and CAPABILITIES it is skipped when its capability is not listed
-(unless the server lists `MODE-READER`: the check then sends MODE READER and tests again). A
+(unless the server lists `MODE-READER`, or `User` is set: the check then sends MODE READER
+or logs in, reads CAPABILITIES again and tests then, since a server may list its reader
+capabilities only after AUTHINFO). A
 broken greeting fails `greeting` once and skips the rest; a panic in a check is its Fail.
 The articles that checks post carry a Message-ID under `@conformance.invalid`.
 

cmd/nntp-fakeserver/main_test.go

@@ -435,6 +435,60 @@ func TestRawInflate(t *testing.T) {
 	}
 }
 
+// TestRawInflatePartialFlush: a client that flushes like zlib's Z_PARTIAL_FLUSH (a fixed
+// block, then an empty fixed block, no sync marker) is read at once. compress/flate held the
+// command back until more input came, so the script waited until its Deadline.
+func TestRawInflatePartialFlush(t *testing.T) {
+	p := startProc(t, `{"Mode": "raw", "Raw": [{"Send": [{"Text": "200 hi\r\n"}]},
+	  {"Expect": "COMPRESS DEFLATE", "Send": [{"Text": "206 ok\r\n"}], "Then": "inflate"},
+	  {"Expect": "DATE", "Send": [{"Text": "111 20200102030405\r\n"}]}]}`)
+	c := p.dial()
+	cmd(t, c, 206, "COMPRESS DEFLATE")
+	if err := c.Send(string(partialFlush("DATE\r\n"))); err != nil {
+		t.Fatal(err)
+	}
+	if code, line, err := c.ReadStatus(); err != nil || code != 111 {
+		t.Fatalf("DATE through a partial flush: %d %q %v", code, line, err)
+	}
+	s, _ := p.finish()
+	if want := []string{"COMPRESS DEFLATE", "DATE"}; !slices.Equal(s.Lines, want) || len(s.Mismatches) != 0 {
+		t.Errorf("lines %q mismatches %q", s.Lines, s.Mismatches)
+	}
+}
+
+// partialFlush encodes text (bytes below 144 only) as a non-final fixed-Huffman block of
+// literals and an empty fixed block, padded to a byte: what zlib writes for Z_PARTIAL_FLUSH.
+func partialFlush(text string) []byte {
+	var out []byte
+	var acc uint32
+	var n uint
+	bits := func(v uint32, k uint) { // LSB first
+		acc |= v << n
+		for n += k; n >= 8; n -= 8 {
+			out = append(out, byte(acc))
+			acc >>= 8
+		}
+	}
+	code := func(c uint32, k uint) { // a Huffman code, MSB first
+		var r uint32
+		for i := uint(0); i < k; i++ {
+			r = r<<1 | (c>>i)&1
+		}
+		bits(r, k)
+	}
+	bits(0b010, 3) // BFINAL 0, BTYPE 01
+	for _, b := range []byte(text) {
+		code(0x30+uint32(b), 8)
+	}
+	code(0, 7)     // end of block
+	bits(0b010, 3) // the empty fixed block
+	code(0, 7)
+	if n > 0 {
+		out = append(out, byte(acc))
+	}
+	return out
+}
+
 // TestRawHang: hang reads and never answers, and does not close.
 func TestRawHang(t *testing.T) {
 	p := startProc(t, `{"Mode": "raw", "Raw": [{"Send": [{"Text": "200 hi\r\n"}]}, {"Expect": "QUIT", "Then": "hang"}]}`)

cmd/nntp-fakeserver/raw.go

@@ -6,6 +6,7 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"os"
 	"regexp"
@@ -14,6 +15,7 @@ import (
 	"time"
 
 	"github.com/go-while/nntp-testkit/fakeserver"
+	"github.com/go-while/nntp-testkit/internal/inflate"
 )
 
 // recorder holds what raw mode saw (the fakeserver records the other modes; a raw step reads
@@ -270,7 +272,17 @@ func rawHandler(steps []rawStep, rec *recorder, tlsCfg *tls.Config, deadline tim
 				rc.conn = c.Raw()
 				rc.r = bufio.NewReader(rc.conn)
 			case "inflate":
-				rc.r = bufio.NewReader(flate.NewReader(rc.r))
+				// the kit's streaming inflater, not compress/flate: that one holds the bytes
+				// of a Z_PARTIAL_FLUSH back until more input comes (see internal/inflate)
+				pr, pw := io.Pipe()
+				go func(src *bufio.Reader) {
+					_ = pw.CloseWithError(inflate.Run(src, func(b []byte) error {
+						_, err := pw.Write(b)
+						return err
+					}))
+				}(rc.r)
+				defer pr.Close() // ends the inflater when the handler returns first
+				rc.r = bufio.NewReader(pr)
 			}
 		}
 		for { // the lines after the script: no read deadline either

conformance/reader.go

@@ -35,12 +35,19 @@ const unknownGroup = "conformance.invalid.no-such-group"
 // farAbove is added to a group's high water mark for an article number that cannot exist.
 const farAbove = 1000000
 
-// maxArticle is the largest article number farNumber gives (2^31-1: servers with 32-bit
-// numbers must still parse it).
+// maxArticle is the largest article number farNumber gives while the group's numbers fit in
+// 32 bits (2^31-1: servers with 32-bit numbers must still parse it).
 const maxArticle = 2147483647
 
-// farNumber returns an article number far above high, at most maxArticle.
-func farNumber(high int64) int64 { return min(high+farAbove, maxArticle) }
+// farNumber returns an article number above high that cannot exist: high+farAbove capped at
+// maxArticle, unless high is maxArticle or more (the server counts beyond 32 bits, and the
+// cap would name an existing article), when it is high+farAbove.
+func farNumber(high int64) int64 {
+	if high >= maxArticle {
+		return high + farAbove
+	}
+	return min(high+farAbove, maxArticle)
+}
 
 // readerChecks returns the RFC 3977 checks, in report order.
 func readerChecks() []check {
@@ -1042,7 +1049,7 @@ func checkOverLine(l string, fmtLines []string, want int64, wantID string) (int6
 			continue
 		}
 		if !strings.HasPrefix(strings.ToLower(f[i]), name+":") {
-			return n, fmt.Errorf("OVER line %q: field %d %q does not start with %q", l, i+1, f[i], fmtLines[i-1][:len(name)+1])
+			return n, fmt.Errorf("OVER line %q: field %d %q does not start with %q", l, i+1, f[i], name+":")
 		}
 	}
 	return n, nil

conformance/reader_test.go

@@ -555,4 +555,21 @@ func TestParseHelpers(t *testing.T) {
 	if n := farNumber(maxArticle - 5); n != maxArticle {
 		t.Errorf("farNumber near the 32-bit limit: %d", n)
 	}
+	// a group already at or beyond the 32-bit limit: the number stays above high
+	for _, high := range []int64{maxArticle, maxArticle + 1, 5_000_000_000} {
+		if n := farNumber(high); n <= high {
+			t.Errorf("farNumber(%d) = %d, not above high", high, n)
+		}
+	}
+}
+
+// TestCheckOverLineFoldingName: a LIST OVERVIEW.FMT name whose lower-case form is longer than
+// the original (U+023A, 2 bytes, lower-cases to U+2C65, 3 bytes) gives an error, not a slice
+// panic.
+func TestCheckOverLineFoldingName(t *testing.T) {
+	fmtLines := []string{"Subject:", "From:", "Date:", "Message-ID:", "References:", ":bytes", ":lines", "\u023a\u023a\u023a\u023a\u023a:full"}
+	l := "1\ts\tf\td\t<1@x>\t\t10\t1\tother: x"
+	if _, err := checkOverLine(l, fmtLines, 1, "<1@x>"); err == nil || !strings.Contains(err.Error(), "does not start with") {
+		t.Errorf("checkOverLine: %v", err)
+	}
 }

conformance/run.go

@@ -208,7 +208,8 @@ func runPhase(o Options, checks []check, idx []int, out []Result) {
 // (Options.Skip, Auth without Options.User, Posts without Options.PostGroup); then the session
 // is dialed (a dial error is turned into a Result by sessionFailed), CAPABILITIES is read, and
 // c is skipped when the server does not advertise c.Needs, unless it advertises MODE-READER
-// (the check's ready switches the mode and tests Needs again). A panic in the check is a Fail.
+// or Options.User is set (the check's ready switches the mode or logs in, reads the
+// capabilities again and tests Needs then). A panic in the check is a Fail.
 func runOne(o Options, c check) (r Result) {
 	defer func() { r = o.allowed(r) }()
 	switch {
@@ -231,7 +232,9 @@ func runOne(o Options, c check) (r Result) {
 		return sessionFailed(c, err)
 	}
 	_, _ = s.capabilities() // a failure stays in s.capsErr; the capabilities check reports it
-	if c.Needs != "" && !s.has(c.Needs) && !s.has("MODE-READER") {
+	// with Options.User the check's ready logs in and reads the capabilities again before it
+	// tests Needs: a server may list its reader capabilities only after AUTHINFO
+	if c.Needs != "" && !s.has(c.Needs) && !s.has("MODE-READER") && o.User == "" {
 		return c.notAdvertised(c.Needs)
 	}
 	r = c.Run(s, c)

conformance/run_test.go

@@ -485,6 +485,39 @@ func TestRunNeedsModeReader(t *testing.T) {
 	}
 }
 
+// TestRunNeedsAfterLogin: a server that lists READER only after AUTHINFO gets its reader
+// checks run when Options.User is set (they were skipped before the login could happen), and
+// skipped when the login does not bring the capability.
+func TestRunNeedsAfterLogin(t *testing.T) {
+	steps := func(after string) fakeserver.Options {
+		return fakeserver.Options{Script: []fakeserver.Step{
+			{Expect: "CAPABILITIES", Reply: []string{"101 caps", "VERSION 2", "AUTHINFO USER", "."}},
+			{Expect: "AUTHINFO USER u", Reply: []string{"381 more"}},
+			{Expect: "AUTHINFO PASS p", Reply: []string{"281 ok"}},
+			{Expect: "CAPABILITIES", Reply: []string{"101 caps", "VERSION 2", after, "."}},
+		}}
+	}
+	ran := false
+	c := check{Name: "needs-reader", RFC: "test", Needs: "READER", Run: func(s *session, c check) Result {
+		if r, ok := s.ready(c); !ok {
+			return r
+		}
+		ran = true
+		return c.pass()
+	}}
+	srv := fakeserver.New(t, steps("READER"))
+	r := runChecks(Options{Addr: srv.Addr(), User: "u", Pass: "p"}, []check{c})[0]
+	if r.Status != Pass || !ran {
+		t.Errorf("READER after login: %s %q (ran %v), want PASS", r.Status, r.Detail, ran)
+	}
+	ran = false
+	srv = fakeserver.New(t, steps("POST"))
+	r = runChecks(Options{Addr: srv.Addr(), User: "u", Pass: "p"}, []check{c})[0]
+	if r.Status != Skip || r.Missing != "READER" || ran {
+		t.Errorf("no READER after login: %s %q (ran %v), want SKIP", r.Status, r.Detail, ran)
+	}
+}
+
 // TestRunVerbose: RunVerbose logs every result that is not a Skip, one line each, then
 // reports the Fails as Run does; ByName finds each result.
 func TestRunVerbose(t *testing.T) {

conformance/types.go

@@ -270,7 +270,8 @@ func (c check) failf(f string, a ...any) Result { return c.fail(fmt.Sprintf(f, a
 
 // check is one conformance check. Run opens a session for it (the connection, the greeting and
 // the capability list) and calls Run; Needs names a capability keyword (e.g. "READER", "OVER",
-// "STREAMING") the server must advertise, else the check is skipped; Posts marks checks that
+// "STREAMING") the server must advertise, else the check is skipped (tested again after MODE
+// READER or the login of Options.User; every check with Needs calls ready); Posts marks checks that
 // modify the server and need Options.PostGroup; Auth marks checks that need Options.User.
 //
 // Each slice contributes its checks through a function returning []check (readerChecks,

fakeserver/conn.go

@@ -59,6 +59,18 @@ func command(line string) string {
 	return strings.ToUpper(f[0])
 }
 
+// cutWord splits s into its first word and the rest: leading spaces and tabs are skipped, the
+// word ends at the next space or tab, and exactly that one separator is dropped, so the rest
+// keeps its bytes (a password with repeated or trailing spaces). rest is "" without a separator.
+func cutWord(s string) (word, rest string) {
+	s = strings.TrimLeft(s, " \t")
+	i := strings.IndexAny(s, " \t")
+	if i < 0 {
+		return s, ""
+	}
+	return s[:i], s[i+1:]
+}
+
 // lineKey is the key of Faults.OverrideLine for a client line: the command upper-cased and the
 // arguments as sent, joined by single spaces.
 func lineKey(line string) string {

fakeserver/doc.go

@@ -189,7 +189,8 @@
 //	// Arrived is at or after the time; date and time as NEWGROUPS, always UTC; 503 when a
 //	// matching generated group spans more than its limit), DATE, POST (440 when
 //	// ReaderOnly; 340 then 240/441: Newsgroups, Subject and From required, Message-ID must be
-//	// <...> and unused, generated as <posted-N@fakeserver.invalid> when missing; stored with
+//	// <...> and unused, generated as <posted-N@fakeserver.invalid> when missing; 441 when
+//	// Newsgroups names a generated group, before anything is stored; stored with
 //	// AddPosted and AddArticle per group), AUTHINFO USER/PASS (381/281/481/482; USER alone
 //	// answers 281/481 when Pass is empty; 502 when not configured, done or compression is active), COMPRESS alg (with
 //	// Compress or CompressOutputOnly, else 500; 480 before required auth; 501 missing or

fakeserver/generated.go

@@ -118,6 +118,19 @@ func (g *Generator) lookup(group, id string) (int64, bool) {
 	return generatedNumber(group, id)
 }
 
+// firstGenerated returns the first of groups that is a generated group (AddArticle refuses
+// those), without creating any group.
+func (st *Store) firstGenerated(groups []string) (string, bool) {
+	st.mu.Lock()
+	defer st.mu.Unlock()
+	for _, name := range groups {
+		if g, ok := st.groups[name]; ok && g.gen != nil {
+			return name, true
+		}
+	}
+	return "", false
+}
+
 // AddGenerated creates group (or turns an existing group without articles into one) as a
 // generated group: its articles Low..High come from gen and are never stored. Count is
 // High-Low+1 (holes included, like the estimate of a real server). Store methods work on it:

fakeserver/peer.go

@@ -146,9 +146,9 @@ func PeerHandler(o *PeerOptions) func(*Conn) {
 			if err != nil {
 				return
 			}
-			verb, arg := line, ""
-			if i := strings.IndexByte(line, ' '); i >= 0 {
-				verb, arg = line[:i], line[i+1:]
+			verb, arg := cutWord(line)
+			if !strings.EqualFold(verb, "AUTHINFO") {
+				arg = strings.TrimSpace(arg) // a message-id or keyword: no padding ("CHECK  <id>")
 			}
 			verb = strings.ToUpper(verb)
 			if o.NoStream && (verb == "CHECK" || verb == "TAKETHIS") {
@@ -225,10 +225,7 @@ func PeerHandler(o *PeerOptions) func(*Conn) {
 					reply = []string{"502 Command unavailable"} // RFC 4643 §2.3: no AUTHINFO after a successful one
 					break
 				}
-				sub, val := arg, ""
-				if i := strings.IndexByte(arg, ' '); i >= 0 {
-					sub, val = arg[:i], arg[i+1:]
-				}
+				sub, val := cutWord(arg)
 				switch strings.ToUpper(sub) {
 				case "USER":
 					userSent = true

fakeserver/reviewfix_test.go

@@ -0,0 +1,90 @@
+package fakeserver
+
+import (
+	"testing"
+
+	"github.com/go-while/nntp-testkit/client"
+)
+
+// A POST to a generated group gets 441 before anything is filed (it used to panic in the
+// connection goroutine and end the test process), also in a crosspost with a normal group.
+func TestStorePostGeneratedGroup(t *testing.T) {
+	st := testStore()
+	st.AddGenerated("gen.big", "", Generator{Low: 1, High: 100})
+	_, c := storeServer(t, st, StoreOptions{}, Faults{})
+	for _, ng := range []string{"gen.big", "alt.test,gen.big"} {
+		wantCode(t, c, "POST", 340)
+		if err := c.Send("Newsgroups: " + ng + "\r\nSubject: s\r\nFrom: f\r\nMessage-ID: <g@x>\r\n\r\nb\r\n.\r\n"); err != nil {
+			t.Fatal(err)
+		}
+		if line := wantStatus(t, c, 441); line != "441 Posting failed: gen.big is a generated group" {
+			t.Errorf("Newsgroups %q: %q", ng, line)
+		}
+	}
+	if n := len(st.Posted()); n != 0 {
+		t.Errorf("Posted() = %d", n)
+	}
+	if _, _, ok := st.ByMessageID("<g@x>"); ok {
+		t.Error("the refused article was filed")
+	}
+	wantCode(t, c, "DATE", 111)
+}
+
+// An overridden final first reply to POST with OverrideTimes uses one allowance per POST, and
+// the server does not wait for an article after it.
+func TestStorePostOverrideTimes(t *testing.T) {
+	f := Faults{Override: map[string][]string{"POST": {"440 not now"}}, OverrideTimes: map[string]int{"POST": 2}}
+	s, c := storeServer(t, testStore(), StoreOptions{}, f)
+	wantCode(t, c, "POST", 440)
+	wantCode(t, c, "DATE", 111) // used to be read as article data
+	wantCode(t, c, "POST", 440) // the second allowance
+	wantCode(t, c, "POST", 340) // the override is used up
+	if err := c.Send("Newsgroups: alt.test\r\nSubject: s\r\nFrom: f\r\n\r\nb\r\n.\r\n"); err != nil {
+		t.Fatal(err)
+	}
+	wantStatus(t, c, 240)
+	if got := s.Count("POST"); got != 3 {
+		t.Errorf("Count(POST) = %d", got)
+	}
+}
+
+// The Store password is compared byte for byte: repeated spaces count, a tab is not a space.
+func TestStoreAuthPasswordVerbatim(t *testing.T) {
+	for _, tc := range []struct {
+		pass, sent string
+		want       int
+	}{
+		{"p  q", "p  q", 281},
+		{"p q", "p q", 281},
+		{"p q", "p  q", 481},
+		{"p q", "p\tq", 481},
+		{"p q", "p q ", 481},
+		{"p", "p", 281},
+	} {
+		_, c := storeServer(t, testStore(), StoreOptions{User: "u", Pass: tc.pass}, Faults{})
+		wantCode(t, c, "AUTHINFO USER u", 381)
+		if line := wantCode(t, c, "AUTHINFO PASS "+tc.sent, tc.want); line == "" {
+			t.Errorf("pass %q sent %q", tc.pass, tc.sent)
+		}
+	}
+	// the separators between the words before the value may be any run of spaces and tabs
+	_, c := storeServer(t, testStore(), StoreOptions{User: "u", Pass: "p"}, Faults{})
+	wantCode(t, c, "authinfo\tuser u", 381)
+	wantCode(t, c, "AUTHINFO  PASS p", 281)
+}
+
+// PeerHandler splits the verb at any space or tab and trims the message-id, like the Faults
+// keys and Counts do.
+func TestPeerVerbSplit(t *testing.T) {
+	s := New(t, Options{Handler: PeerHandler(&PeerOptions{Have: map[string]bool{"<h@x>": true}})})
+	c := client.Dial(t, s.Addr())
+	if line := wantCode(t, c, "check\t<a@b>", 238); line != "238 <a@b>" {
+		t.Errorf("tab: %q", line)
+	}
+	if line := wantCode(t, c, "CHECK  <h@x>", 438); line != "438 <h@x>" {
+		t.Errorf("two spaces: %q", line)
+	}
+	if got := s.Count("CHECK"); got != 2 {
+		t.Errorf("Count(CHECK) = %d", got)
+	}
+}