diff --git a/examples/route-proxy/main.go b/examples/route-proxy/main.go index 10d7217..9b59643 100644 --- a/examples/route-proxy/main.go +++ b/examples/route-proxy/main.go @@ -416,8 +416,17 @@ func handler(held *table) http.Handler { // **Named, not a bare 404.** A route that was withdrawn and a name that never existed // are different things, and a proxy that says only "not found" makes an operator go // and read the mesh to tell them apart. What it is serving is the answer to both. + // + // And since a host may now be routed only on some paths, those are a third thing: + // saying "no route for this name" while listing that very name as served is a + // contradiction an operator would have to disbelieve the proxy to get past. w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusNotFound) + if held.routed(r.Host) { + fmt.Fprintf(w, "%s is served here, but no route covers %q.\n", + bareHost(r.Host), r.URL.Path) + return + } fmt.Fprintf(w, "no route for %q in this mesh.\nserving: %s\n", r.Host, strings.Join(held.names(), ", ")) return @@ -524,7 +533,7 @@ func routesFrom(path string) (map[string][]rule, error) { host := strings.ToLower(name) made := rule{path: asPath(c.Values["path"])} - if p, ok := asPort(c.Values["priority"]); ok { + if p, ok := asWhole(c.Values["priority"]); ok { made.priority = p } made.policy.deny, _ = c.Values["deny"].(bool) @@ -574,6 +583,26 @@ func routesFrom(path string) (map[string][]rule, error) { return out, nil } +// asWhole is any whole number the mesh wrote, whatever its magnitude. +// +// **Not asPort.** Priority was read with the port reader first, which caps at 65535 — so a rule +// declared at a priority above that silently became priority 0 and stopped shadowing the route it +// exists to shadow. The one real rule this has to reproduce is declared at 100000, so the bug was +// exactly load-bearing. A priority is an ordering, not a port: it has no range. +func asWhole(v any) (int, bool) { + switch n := v.(type) { + case float64: + // JSON makes a float of every number, so a non-integral one was not meant as a priority. + if n != float64(int(n)) { + return 0, false + } + return int(n), true + case int: + return n, true + } + return 0, false +} + // asPath is the path prefix a rule is scoped to, or "" for every path. func asPath(v any) string { p, _ := v.(string) diff --git a/examples/route-proxy/policy_test.go b/examples/route-proxy/policy_test.go index 7f98ec4..db1dc0b 100644 --- a/examples/route-proxy/policy_test.go +++ b/examples/route-proxy/policy_test.go @@ -231,3 +231,43 @@ func TestPriorityOutranksPathLength(t *testing.T) { t.Fatalf("a higher priority did not win: %+v", rules[0]) } } + +// A priority above a port number survives, because a priority is an ordering and not a port. +// +// **Found by review, and it was load-bearing.** Priority was first read with the port reader, which +// caps at 65535 — so a rule declared above that silently became priority 0 and stopped shadowing the +// route it exists to shadow. The one real rule this has to reproduce is declared at 100000, so the +// capability would have shipped looking complete and doing nothing. +func TestAPriorityAboveAPortNumberSurvives(t *testing.T) { + at, port := served(t, "the workload") + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","at":"`+at+`","values":{"name":"forge.example","port":`+strconv.Itoa(port)+`}}, + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","priority":100000,"deny":true}} + ]}`) + + if got := ask(t, proxy, "forge.example", "/api/internal/hook", [2]string{}).StatusCode; got != http.StatusForbidden { + t.Fatalf("a rule declared at priority 100000 answered %d instead of refusing", got) + } +} + +// A host routed only on some paths says so, rather than claiming the name is not served here. +// +// Saying "no route for this name" while listing that very name as served is a contradiction an +// operator has to disbelieve the proxy to get past — and path scoping makes it reachable, because a +// host can now have rules that none of this request's paths match. +func TestAHostRoutedOnlyOnSomePathsSaysSo(t *testing.T) { + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","deny":true}} + ]}`) + + answer := ask(t, proxy, "forge.example", "/elsewhere", [2]string{}) + if answer.StatusCode != http.StatusNotFound { + t.Fatalf("an uncovered path answered %d", answer.StatusCode) + } + body := make([]byte, 256) + n, _ := answer.Body.Read(body) + said := string(body[:n]) + if !strings.Contains(said, "is served here") || !strings.Contains(said, "/elsewhere") { + t.Fatalf("the refusal does not distinguish an uncovered path from an unserved name: %q", said) + } +}