Fix long delay replies answering later command

This commit is contained in:
Jonathon Leight committed 2026-06-21 21:43:22 -04:00
1 parent 0c453df563
commit dc7b858f78
3 files changed
+87 -4

No files matched your search

+18 -4
View File
@@ -182,8 +182,8 @@ func (s *Handlers) wsConfirm(w http.ResponseWriter, r *http.Request) {
// Optionally fetch and store the repeater's location (owner consented). Each
// coordinate is a separate query so progress (and retries) are visible.
if rep.StoreLocation {
fetchCoord := func(label, cmd string) (float64, bool) {
reply, err := ex.Command(ctx, cmd, func(attempt, max int) {
fetchCoord := func(label, cmd string, accept func(text string) bool) (float64, bool) {
reply, err := ex.CommandAccept(ctx, cmd, accept, func(attempt, max int) {
if attempt == 1 {
_ = bridge.Status("info", "Fetching "+label+"…")
} else {
@@ -195,8 +195,22 @@ func (s *Handlers) wsConfirm(w http.ResponseWriter, r *http.Request) {
}
return parseLocationFloat(reply)
}
lat, okLat := fetchCoord("latitude", "get lat")
lon, okLon := fetchCoord("longitude", "get lon")
lat, okLat := fetchCoord("latitude", "get lat", nil)
// A slow latitude fetch is retried, which makes the repeater re-run "get
// lat" and emit duplicate replies; one can straggle in during the "get
// lon" wait and be misread as the longitude (storing lat,lat). Since the
// two coordinates differ, reject a longitude reply whose value equals the
// latitude we just read and keep waiting for the genuine reply.
lon, okLon := fetchCoord("longitude", "get lon", func(text string) bool {
f, ok := parseLocationFloat(text)
if ok && okLat && f == lat {
if debug {
_ = bridge.Status("debug", "ignored a stale 'get lat' reply while awaiting longitude")
}
return false
}
return true
})
if okLat && okLon {
if err := s.Store.SetRepeaterLocation(ctx, id, lat, lon); err != nil {
_ = bridge.Status("error", "could not store location: "+err.Error())
+17
View File
@@ -170,6 +170,20 @@ func (e *Exchanger) directThreshold() int { return (e.tries + 1) / 2 }
// path has been learned (via Login), early attempts use direct routing and
// later attempts fall back to flood.
func (e *Exchanger) Command(ctx context.Context, command string, onAttempt func(attempt, max int)) (string, error) {
return e.CommandAccept(ctx, command, nil, onAttempt)
}
// CommandAccept is Command with a caller-supplied acceptance test. A decoded
// reply is only taken as the answer when accept(text) returns true; otherwise
// the reply is ignored and the exchange keeps waiting (and retrying).
//
// This lets a caller reject a stale reply that decrypts fine but belongs to an
// earlier command. Because a retried request makes the repeater re-run the
// command, a slow exchange can leave duplicate replies in flight; without a
// request identifier in the reply, the only way to tell such a straggler from
// the genuine reply is a caller filter that knows what a valid answer looks
// like. accept may be nil to take the first decodable reply (like Command).
func (e *Exchanger) CommandAccept(ctx context.Context, command string, accept func(text string) bool, onAttempt func(attempt, max int)) (string, error) {
var reply string
err := e.exchange(ctx,
func(ts time.Time, attempt int) ([]byte, error) {
@@ -187,6 +201,9 @@ func (e *Exchanger) Command(ctx context.Context, command string, onAttempt func(
if err != nil {
return false
}
if accept != nil && !accept(text) {
return false
}
reply = text
return true
},
+52
View File
@@ -146,6 +146,58 @@ func TestExchangerUsesDirectAfterLogin(t *testing.T) {
}
}
// seqModem replies to each send with the next text in replies (repeating the
// last once exhausted), simulating a repeater that emits a stale duplicate
// before the genuine reply arrives.
type seqModem struct {
ex *Exchanger
repeater meshcore.LocalIdentity
server meshcore.Identity
replies []string
sends int
}
func (f *seqModem) SendData(_ []byte) error {
text := f.replies[len(f.replies)-1]
if f.sends < len(f.replies) {
text = f.replies[f.sends]
}
f.sends++
shared, _ := f.repeater.SharedSecret(f.server)
plain := meshcore.BuildTextPlaintext(time.Unix(1_700_000_000, 0), 1<<2, []byte(text))
tm, _ := meshcore.NewTextMessage(f.repeater, f.server, plain, shared)
payload, _ := tm.ToBytes()
pkt := &meshcore.Packet{Header: meshcore.MakeHeader(meshcore.RouteTypeFlood, meshcore.PayloadTypeTxtMsg, 0), Payload: payload}
raw, _ := pkt.ToBytes()
f.ex.HandleData(raw)
return nil
}
// TestCommandAcceptRejectsStaleReply verifies the acceptance filter skips a
// reply it rejects and keeps waiting for one it accepts — the guard that keeps
// a straggling "get lat" duplicate from being misread as the longitude.
func TestCommandAcceptRejectsStaleReply(t *testing.T) {
server := mustIdentity(t)
repeater := mustIdentity(t)
// First two sends echo the stale latitude; the third gives the longitude.
fm := &seqModem{repeater: repeater, server: server.Identity, replies: []string{"> 37.7749", "> 37.7749", "> -122.4194"}}
ex := NewExchanger(fm, server, repeater.Identity, 0, time.Millisecond, 4)
fm.ex = ex
reply, err := ex.CommandAccept(context.Background(), "get lon", func(text string) bool {
return text != "> 37.7749" // reject the stale latitude duplicate
}, nil)
if err != nil {
t.Fatalf("CommandAccept: %v", err)
}
if reply != "> -122.4194" {
t.Errorf("reply = %q, want %q", reply, "> -122.4194")
}
if fm.sends != 3 {
t.Errorf("sends = %d, want 3 (two rejected, one accepted)", fm.sends)
}
}
func TestExchangerCancelled(t *testing.T) {
server := mustIdentity(t)
repeater := mustIdentity(t)