secure/bidirule: fix bug/different semantics

This also fixes issues in secure/precis (the tests make a lot more
sense now).

- Rules only apply to Bidi Domain names. Currently the user had
to test this. This test is now embedded in the bidi test.

- Direction* now reports the direction as defined in the RFC,
instead of using the Unicode definition.

- Added Valid and ValidString functions. This is much more useful
and intuitive if the user does not require a transformer.

Fixes golang/go#17383

Change-Id: Id7be15e48bbf36581ce23347575eb88226e12841
Reviewed-on: https://go-review.googlesource.com/30553
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
diff --git a/secure/bidirule/bidirule.go b/secure/bidirule/bidirule.go
index 277257f..a7161bd 100644
--- a/secure/bidirule/bidirule.go
+++ b/secure/bidirule/bidirule.go
@@ -123,34 +123,64 @@
 // vice versa.
 const exclusiveRTL = uint16(1<<bidi.EN | 1<<bidi.AN)
 
-// Direction reports the direction of the given label as defined by RFC 5893 or
-// an error if b is not a valid label according to the Bidi Rule.
-func Direction(b []byte) (bidi.Direction, error) {
-	t := Transformer{}
-	if n, ok := t.advance(b); ok && n == len(b) {
-		switch t.state {
-		case ruleLTRFinal, ruleInitial:
-			return bidi.LeftToRight, nil
-		case ruleRTLFinal:
-			return bidi.RightToLeft, nil
+// From RFC 5893
+// An RTL label is a label that contains at least one character of type
+// R, AL, or AN.
+//
+// An LTR label is any label that is not an RTL label.
+
+// Direction reports the direction of the given label as defined by RFC 5893.
+// The Bidi Rule does not have to be applied to labels of the category
+// LeftToRight.
+func Direction(b []byte) bidi.Direction {
+	for i := 0; i < len(b); {
+		e, sz := bidi.Lookup(b[i:])
+		if sz == 0 {
+			i++
 		}
+		c := e.Class()
+		if c == bidi.R || c == bidi.AL || c == bidi.AN {
+			return bidi.RightToLeft
+		}
+		i += sz
 	}
-	return bidi.Neutral, ErrInvalid
+	return bidi.LeftToRight
 }
 
 // DirectionString reports the direction of the given label as defined by RFC
-// 5893 or an error if s is not a valid label according to the Bidi Rule.
-func DirectionString(s string) (bidi.Direction, error) {
-	t := Transformer{}
-	if n, ok := t.advanceString(s); ok && n == len(s) {
-		switch t.state {
-		case ruleLTRFinal, ruleInitial:
-			return bidi.LeftToRight, nil
-		case ruleRTLFinal:
-			return bidi.RightToLeft, nil
+// 5893. The Bidi Rule does not have to be applied to labels of the category
+// LeftToRight.
+func DirectionString(s string) bidi.Direction {
+	for i := 0; i < len(s); {
+		e, sz := bidi.LookupString(s[i:])
+		if sz == 0 {
+			i++
 		}
+		c := e.Class()
+		if c == bidi.R || c == bidi.AL || c == bidi.AN {
+			return bidi.RightToLeft
+		}
+		i += sz
 	}
-	return bidi.Neutral, ErrInvalid
+	return bidi.LeftToRight
+}
+
+// Valid reports whether b conforms to the BiDi rule.
+func Valid(b []byte) bool {
+	var t Transformer
+	if n, ok := t.advance(b); !ok || n < len(b) {
+		return false
+	}
+	return t.isFinal()
+}
+
+// ValidString reports whether s conforms to the BiDi rule.
+func ValidString(s string) bool {
+	var t Transformer
+	if n, ok := t.advanceString(s); !ok || n < len(s) {
+		return false
+	}
+	return t.isFinal()
 }
 
 // New returns a Transformer that verifies that input adheres to the Bidi Rule.
@@ -160,8 +190,23 @@
 
 // Transformer implements transform.Transform.
 type Transformer struct {
-	state ruleState
-	seen  uint16
+	state  ruleState
+	hasRTL bool
+	seen   uint16
+}
+
+// A rule can only be violated for "Bidi Domain names", meaning if one of the
+// following categories has been observed.
+func (t *Transformer) isRTL() bool {
+	const isRTL = 1<<bidi.R | 1<<bidi.AL | 1<<bidi.AN
+	return t.seen&isRTL != 0
+}
+
+func (t *Transformer) isFinal() bool {
+	if !t.isRTL() {
+		return true
+	}
+	return t.state == ruleLTRFinal || t.state == ruleRTLFinal || t.state == ruleInitial
 }
 
 // Reset implements transform.Transformer.
@@ -185,7 +230,7 @@
 
 // Span returns the first n bytes of src that conform to the Bidi rule.
 func (t *Transformer) Span(src []byte, atEOF bool) (n int, err error) {
-	if t.state == ruleInvalid {
+	if t.state == ruleInvalid && t.isRTL() {
 		return 0, ErrInvalid
 	}
 	n, ok := t.advance(src)
@@ -198,7 +243,7 @@
 			break
 		}
 		err = ErrInvalid
-	case t.state != ruleLTRFinal && t.state != ruleRTLFinal && t.state != ruleInitial:
+	case !t.isFinal():
 		err = ErrInvalid
 	}
 	return n, err
@@ -225,12 +270,15 @@
 			e, sz = bidi.Lookup(s[n:])
 			if sz <= 1 {
 				if sz == 1 {
-					return n, false // invalid UTF-8
+					// We always consider invalid UTF-8 to be invalid, even if
+					// the string has not yet been determined to be RTL.
+					// TODO: is this correct?
+					return n, false
 				}
 				return n, true // incomplete UTF-8 encoding
 			}
 		}
-		// TODO: using CompactClass results in noticeable speedup.
+		// TODO: using CompactClass would result in noticeable speedup.
 		// See unicode/bidi/prop.go:Properties.CompactClass.
 		c := uint16(1 << e.Class())
 		t.seen |= c
@@ -245,7 +293,9 @@
 			t.state = tr[1].next
 		default:
 			t.state = ruleInvalid
-			return n, false
+			if t.isRTL() {
+				return n, false
+			}
 		}
 		n += sz
 	}
@@ -282,7 +332,9 @@
 			t.state = tr[1].next
 		default:
 			t.state = ruleInvalid
-			return n, false
+			if t.isRTL() {
+				return n, false
+			}
 		}
 		n += sz
 	}
diff --git a/secure/bidirule/bidirule_test.go b/secure/bidirule/bidirule_test.go
index f3cc251..0794b3d 100644
--- a/secure/bidirule/bidirule_test.go
+++ b/secure/bidirule/bidirule_test.go
@@ -51,52 +51,52 @@
 		dir: bidi.LeftToRight,
 	}, {
 		in:  "\x80",
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
 		err: ErrInvalid,
 		n:   0,
 	}, {
 		in:  "\xcc",
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
 		err: ErrInvalid,
 		n:   0,
 	}, {
 		in:  "abc\x80",
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
 		err: ErrInvalid,
 		n:   3,
 	}, {
 		in:  "abc\xcc",
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
 		err: ErrInvalid,
 		n:   3,
 	}, {
 		in:  "abc\xccdef",
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
 		err: ErrInvalid,
 		n:   3,
 	}, {
 		in:  "\xccdef",
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
 		err: ErrInvalid,
 		n:   0,
 	}, {
 		in:  strR + "\x80",
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		err: ErrInvalid,
 		n:   len(strR),
 	}, {
 		in:  strR + "\xcc",
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		err: ErrInvalid,
 		n:   len(strR),
 	}, {
 		in:  strAL + "\xcc" + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		err: ErrInvalid,
 		n:   len(strAL),
 	}, {
 		in:  "\xcc" + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		err: ErrInvalid,
 		n:   0,
 	}},
@@ -114,49 +114,99 @@
 		in:  strAL,
 		dir: bidi.RightToLeft,
 	}, {
-		in:  strEN,
-		dir: bidi.Neutral,
+		in:  strAN,
+		dir: bidi.RightToLeft,
 		err: ErrInvalid,
 	}, {
+		in:  strEN,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
+	}, {
 		in:  strES,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strET,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
-	}, {
-		in:  strAN,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strCS,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strNSM,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strBN,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strB,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strS,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strWS,
-		dir: bidi.Neutral,
-		err: ErrInvalid,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
 	}, {
 		in:  strON,
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
+		err: nil, // not an RTL string
+	}, {
+		in:  strEN + strR,
+		dir: bidi.RightToLeft,
 		err: ErrInvalid,
+		n:   3,
+	}, {
+		in:  strES + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   2,
+	}, {
+		in:  strET + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   1,
+	}, {
+		in:  strCS + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   1,
+	}, {
+		in:  strNSM + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   2,
+	}, {
+		in:  strBN + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   3,
+	}, {
+		in:  strB + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   3,
+	}, {
+		in:  strS + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   1,
+	}, {
+		in:  strWS + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   1,
+	}, {
+		in:  strON + strR,
+		dir: bidi.RightToLeft,
+		err: ErrInvalid,
+		n:   1,
 	}},
 
 	// Rule 2.2: In an RTL label, only characters with the Bidi properties R,
@@ -193,22 +243,22 @@
 		dir: bidi.RightToLeft,
 	}, {
 		in:  strR + strL + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strB + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strS + strAL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strWS + strAL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
@@ -243,22 +293,22 @@
 		dir: bidi.RightToLeft,
 	}, {
 		in:  strAL + strL + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strB + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strS + strAL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strWS + strAL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}},
@@ -283,47 +333,47 @@
 		dir: bidi.RightToLeft,
 	}, {
 		in:  strR + strES + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strES + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strCS + strNSM + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strCS + strNSM + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strET,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strET),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strON + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strON + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strBN + strNSM + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strBN + strNSM + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strL + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strB + strNSM + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strS,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strWS,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 	}, {
@@ -343,47 +393,47 @@
 		dir: bidi.RightToLeft,
 	}, {
 		in:  strAL + strES + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strES + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strCS + strNSM + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strCS + strNSM + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strET,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strET),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strON + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strON + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strBN + strNSM + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strBN + strNSM + strNSM),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strL + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strB + strNSM + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strS,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strWS,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL),
 		err: ErrInvalid,
 	}},
@@ -392,22 +442,22 @@
 	// and vice versa.
 	4: []ruleTest{{
 		in:  strR + strEN + strAN,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strEN),
 		err: ErrInvalid,
 	}, {
 		in:  strR + strAN + strEN + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR + strAN),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strEN + strAN,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strEN),
 		err: ErrInvalid,
 	}, {
 		in:  strAL + strAN + strEN + strNSM,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strAL + strAN),
 		err: ErrInvalid,
 	}},
@@ -440,33 +490,48 @@
 		dir: bidi.LeftToRight,
 	}, {
 		in:  strL + strR + strL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strAL + strL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strAN + strL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strB + strL,
-		dir: bidi.Neutral,
-		n:   len(strL),
+		dir: bidi.LeftToRight,
+		n:   len(strL + strAN + strL),
+		err: nil,
+	}, {
+		in:  strL + strB + strL + strR,
+		dir: bidi.RightToLeft,
+		n:   len(strL + strB + strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strS + strL,
-		dir: bidi.Neutral,
-		n:   len(strL),
+		dir: bidi.LeftToRight,
+		n:   len(strL + strS + strL),
+		err: nil,
+	}, {
+		in:  strL + strS + strL + strR,
+		dir: bidi.RightToLeft,
+		n:   len(strL + strS + strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strWS + strL,
-		dir: bidi.Neutral,
-		n:   len(strL),
+		dir: bidi.LeftToRight,
+		n:   len(strL + strWS + strL),
+		err: nil,
+	}, {
+		in:  strL + strWS + strL + strR,
+		dir: bidi.RightToLeft,
+		n:   len(strL + strWS + strL),
 		err: ErrInvalid,
 	}},
 
@@ -493,58 +558,98 @@
 		dir: bidi.LeftToRight,
 	}, {
 		in:  strL + strES,
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strES),
+		err: nil,
+	}, {
+		in:  strL + strES + strR,
+		dir: bidi.RightToLeft,
 		n:   len(strL + strES),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strCS,
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strCS),
+		err: nil,
+	}, {
+		in:  strL + strCS + strR,
+		dir: bidi.RightToLeft,
 		n:   len(strL + strCS),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strET,
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strET),
+		err: nil,
+	}, {
+		in:  strL + strET + strR,
+		dir: bidi.RightToLeft,
 		n:   len(strL + strET),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strON,
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strON),
+		err: nil,
+	}, {
+		in:  strL + strON + strR,
+		dir: bidi.RightToLeft,
 		n:   len(strL + strON),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strBN,
-		dir: bidi.Neutral,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strBN),
+		err: nil,
+	}, {
+		in:  strL + strBN + strR,
+		dir: bidi.RightToLeft,
 		n:   len(strL + strBN),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strR,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strAL,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strAN,
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strL),
 		err: ErrInvalid,
 	}, {
 		in:  strL + strB,
-		dir: bidi.Neutral,
-		n:   len(strL),
+		dir: bidi.LeftToRight,
+		n:   len(strL + strB),
+		err: nil,
+	}, {
+		in:  strL + strB + strR,
+		dir: bidi.RightToLeft,
+		n:   len(strL + strB),
 		err: ErrInvalid,
 	}, {
-		in:  strL + strS,
-		dir: bidi.Neutral,
-		n:   len(strL),
+		in:  strL + strB,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strB),
+		err: nil,
+	}, {
+		in:  strL + strB + strR,
+		dir: bidi.RightToLeft,
+		n:   len(strL + strB),
 		err: ErrInvalid,
 	}, {
-		in:  strL + strWS,
-		dir: bidi.Neutral,
-		n:   len(strL),
+		in:  strL + strB,
+		dir: bidi.LeftToRight,
+		n:   len(strL + strB),
+		err: nil,
+	}, {
+		in:  strL + strB + strR,
+		dir: bidi.RightToLeft,
+		n:   len(strL + strB),
 		err: ErrInvalid,
 	}},
 
@@ -566,7 +671,7 @@
 	}, {
 		// Remain invalid once invalid.
 		in:  strR + "ab",
-		dir: bidi.Neutral,
+		dir: bidi.RightToLeft,
 		n:   len(strR),
 		err: ErrInvalid,
 
@@ -617,10 +722,7 @@
 
 func TestDirection(t *testing.T) {
 	doTests(t, func(t *testing.T, tc ruleTest) {
-		dir, err := Direction([]byte(tc.in))
-		if err != tc.err {
-			t.Errorf("error was %v; want %v", err, tc.err)
-		}
+		dir := Direction([]byte(tc.in))
 		if dir != tc.dir {
 			t.Errorf("dir was %v; want %v", dir, tc.dir)
 		}
@@ -629,16 +731,29 @@
 
 func TestDirectionString(t *testing.T) {
 	doTests(t, func(t *testing.T, tc ruleTest) {
-		dir, err := DirectionString(tc.in)
-		if err != tc.err {
-			t.Errorf("error was %v; want %v", err, tc.err)
-		}
+		dir := DirectionString(tc.in)
 		if dir != tc.dir {
 			t.Errorf("dir was %v; want %v", dir, tc.dir)
 		}
 	})
 }
 
+func TestValid(t *testing.T) {
+	doTests(t, func(t *testing.T, tc ruleTest) {
+		got := Valid([]byte(tc.in))
+		want := tc.err == nil
+		if got != want {
+			t.Fatalf("Valid: got %v; want %v", got, want)
+		}
+
+		got = ValidString(tc.in)
+		want = tc.err == nil
+		if got != want {
+			t.Fatalf("Valid: got %v; want %v", got, want)
+		}
+	})
+}
+
 func TestSpan(t *testing.T) {
 	doTests(t, func(t *testing.T, tc ruleTest) {
 		// Skip tests that test for limited destination buffer size.
diff --git a/secure/precis/enforce_test.go b/secure/precis/enforce_test.go
index a2f2328..31ef68c 100644
--- a/secure/precis/enforce_test.go
+++ b/secure/precis/enforce_test.go
@@ -205,28 +205,28 @@
 		{"\u0049", "\u0069", nil},
 		{"\u03D2", "", errDisallowedRune},
 		{"\u03B0", "\u03B0", nil},
-		{"foo bar", "", bidirule.ErrInvalid},
-		{"♚", "", bidirule.ErrInvalid},
-		{"\u007E", "", bidirule.ErrInvalid}, // disallowed by bidi rule
+		{"foo bar", "", errDisallowedRune},
+		{"♚", "", errDisallowedRune},
+		{"\u007E", "~", nil},
 		{"a", "a", nil},
-		{"!", "", bidirule.ErrInvalid}, // disallowed by bidi rule
-		{"²", "", bidirule.ErrInvalid},
-		{"\t", "", bidirule.ErrInvalid},
-		{"\n", "", bidirule.ErrInvalid},
-		{"\u26D6", "", bidirule.ErrInvalid},
-		{"\u26FF", "", bidirule.ErrInvalid},
+		{"!", "!", nil},
+		{"²", "", errDisallowedRune},
+		{"\t", "", errDisallowedRune},
+		{"\n", "", errDisallowedRune},
+		{"\u26D6", "", errDisallowedRune},
+		{"\u26FF", "", errDisallowedRune},
 		{"\uFB00", "", errDisallowedRune},
-		{"\u1680", "", bidirule.ErrInvalid},
-		{" ", "", bidirule.ErrInvalid},
-		{"  ", "", bidirule.ErrInvalid},
+		{"\u1680", "", errDisallowedRune},
+		{" ", "", errDisallowedRune},
+		{"  ", "", errDisallowedRune},
 		{"\u01C5", "", errDisallowedRune},
-		{"\u16EE", "", errDisallowedRune},   // Nl RUNIC ARLAUG SYMBOL
-		{"\u0488", "", bidirule.ErrInvalid}, // Me COMBINING CYRILLIC HUNDRED THOUSANDS SIGN
-		{"\u212B", "\u00e5", nil},           // Angstrom sign, NFC -> U+00E5
-		{"A\u030A", "å", nil},               // A + ring
-		{"\u00C5", "å", nil},                // A with ring
-		{"\u00E7", "ç", nil},                // c cedille
-		{"\u0063\u0327", "ç", nil},          // c + cedille
+		{"\u16EE", "", errDisallowedRune}, // Nl RUNIC ARLAUG SYMBOL
+		{"\u0488", "", errDisallowedRune}, // Me COMBINING CYRILLIC HUNDRED THOUSANDS SIGN
+		{"\u212B", "\u00e5", nil},         // Angstrom sign, NFC -> U+00E5
+		{"A\u030A", "å", nil},             // A + ring
+		{"\u00C5", "å", nil},              // A with ring
+		{"\u00E7", "ç", nil},              // c cedille
+		{"\u0063\u0327", "ç", nil},        // c + cedille
 		{"\u0158", "ř", nil},
 		{"\u0052\u030C", "ř", nil},