mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
the upgrade sibling names the limit it broke, not the errno
An app listening on a unix socket also listens on a DERIVED one — the same address plus ".http" — because a websocket upgrade cannot cross ZAP framing. The five bytes that suffix adds are not something the caller chose and not something it can see, so a path that fits can still put its sibling over Darwin's 103-byte sun_path limit. It failed inside the transport as "bind: invalid argument", which names neither the limit nor the length and reads like a broken socket rather than a long path. It cost a real afternoon to read once already, in a test whose own NAME was the 49 bytes that pushed it over. load.go has asked exactly this question of exactly this address since the plugin loader learned it. This is the other half of that one rule, asked in listenOn where an error can still be returned rather than in plainSibling, which has no way to report one and would otherwise have to disable upgrades silently. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
@@ -0,0 +1,39 @@
|
||||
package zip
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestALongSocketNamesTheLimitRatherThanTheErrno is the failure this cost an
|
||||
// afternoon to read once already: the sibling's address is DERIVED, so a caller
|
||||
// that picked a fine socket path can still be over the limit by the five bytes
|
||||
// the suffix adds — and the bind reported "invalid argument", which names neither
|
||||
// the limit nor the length.
|
||||
func TestALongSocketNamesTheLimitRatherThanTheErrno(t *testing.T) {
|
||||
dir, err := os.MkdirTemp("", "z")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = os.RemoveAll(dir) })
|
||||
|
||||
// Long enough that the derived ".http" sibling cannot fit.
|
||||
long := filepath.Join(dir, strings.Repeat("n", 120)+".sock")
|
||||
|
||||
app := New(Config{DisableStartupMessage: true})
|
||||
got := app.listenOn([]string{long})
|
||||
if got == nil {
|
||||
t.Fatal("a socket past the limit must be refused")
|
||||
}
|
||||
msg := got.Error()
|
||||
for _, want := range []string{"limit", "sibling"} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Errorf("error = %q, want it to mention %q", msg, want)
|
||||
}
|
||||
}
|
||||
if strings.Contains(msg, "invalid argument") {
|
||||
t.Errorf("error = %q — that is the errno this replaces", msg)
|
||||
}
|
||||
}
|
||||
@@ -232,6 +232,18 @@ func (a *App) listenOn(addrs []string) error {
|
||||
// answers. Appended to servers so it shuts down with the rest, and NOT
|
||||
// bound: it is the same app at a derived address, not a second place to
|
||||
// find it.
|
||||
// The sibling's address is DERIVED, so its length is not something the
|
||||
// caller chose and not something it can see. Asked here, where an error can
|
||||
// still be returned, because the alternative is what it used to be: the bind
|
||||
// fails inside the transport as "invalid argument", which names neither the
|
||||
// limit nor the length, and reads like a broken socket rather than a long
|
||||
// path. load.go asks the same question of the same address before it execs a
|
||||
// child; this is the other half of one rule.
|
||||
if to := plain(addr); to != "" {
|
||||
if err := socketFits(to); err != nil {
|
||||
return fmt.Errorf("zip: the upgrade sibling cannot be bound: %w", err)
|
||||
}
|
||||
}
|
||||
if p := a.plainSibling(addr, h); p != nil {
|
||||
servers = append(servers, p)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user