From ac96fa56425f936ee3458b4c61cf361a96157473 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 19:55:39 +0200 Subject: [PATCH 01/12] Go version --- .github/workflows/govulncheck.yml | 17 ++++++++++++++--- go.mod | 2 +- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml index c6445b1..762ca5a 100644 --- a/.github/workflows/govulncheck.yml +++ b/.github/workflows/govulncheck.yml @@ -1,8 +1,9 @@ name: Vulnerabilities # Known vulnerabilities in the code dbgp calls, in its dependencies and in -# the standard library of the newest Go, which release builds use. Weekly -# too, since new ones are found in code that has not changed. +# the standard library: of the oldest Go that go.mod allows, which go install +# and local builds may use, and of the newest, which release builds use. +# Weekly too, since new ones are found in code that has not changed. on: pull_request: push: @@ -16,11 +17,21 @@ permissions: jobs: govulncheck: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + go: [minimum, stable] + name: govulncheck (${{ matrix.go }} Go) steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + - if: matrix.go == 'minimum' + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + - if: matrix.go == 'stable' + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: go-version: stable check-latest: true diff --git a/go.mod b/go.mod index eb0bdb7..d7398ec 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/php-debugger/dbgp-client -go 1.26.0 +go 1.26.9 require ( github.com/ergochat/readline v0.1.3 From 8a825def9f6474531e484cc003f203724dda9c72 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 20:55:14 +0200 Subject: [PATCH 02/12] Memory limits --- cmd/dbgp/mcp_inspect.go | 17 +++-- cmd/dbgp/mcp_inspect_test.go | 33 +++++++++ go.mod | 4 +- protocol.go | 36 ++-------- server.go | 24 +++++++ session.go | 135 +++++++++++++++++++++++++++++------ session_test.go | 75 +++++++++++++++++++ 7 files changed, 265 insertions(+), 59 deletions(-) diff --git a/cmd/dbgp/mcp_inspect.go b/cmd/dbgp/mcp_inspect.go index 40996da..66a47d8 100644 --- a/cmd/dbgp/mcp_inspect.go +++ b/cmd/dbgp/mcp_inspect.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "math" "unicode/utf8" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -37,7 +36,8 @@ func (t *tools) addInspectTools(server *mcp.Server) { mcp.AddTool(server, &mcp.Tool{ Name: "variable_value", Description: "Show the whole value of a variable whose value variables or variable returned truncated, " + - "such as a long string. A value longer than 32 KB is returned in parts: call again with offset set to next.", + "such as a long string. A value longer than 32 KB is returned in parts: call again with offset set to next. " + + "Only the first 20 MB of a value can be read: next is not set past that.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}, }, t.variableValue) } @@ -220,6 +220,11 @@ type variableValueOutput struct { Next int `json:"next,omitempty" jsonschema:"where the rest of the value starts: call variable_value again with this offset; absent when the value is complete"` } +// maxValueOffset is the furthest variable_value can start a part: PHP sends +// a value's first bytes up to the end of the part, and at most +// dbgp.MaxValueBytes of them. +const maxValueOffset = dbgp.MaxValueBytes - maxTextBytes - utf8.UTFMax + func (t *tools) variableValue(_ context.Context, req *mcp.CallToolRequest, in variableValueInput) (*mcp.CallToolResult, variableValueOutput, error) { s, err := t.session(req, in.Session) if err != nil { @@ -228,9 +233,13 @@ func (t *tools) variableValue(_ context.Context, req *mcp.CallToolRequest, in va if in.Name == "" { return nil, variableValueOutput{}, errors.New("name is required, e.g. $html") } - if in.Offset < 0 || in.Offset > math.MaxInt32 { + if in.Offset < 0 { return nil, variableValueOutput{}, fmt.Errorf("offset must be from 0 to the size of %s", in.Name) } + if in.Offset > maxValueOffset { + return nil, variableValueOutput{}, fmt.Errorf("only the first %d MB of a value can be read: offset must be at most %d", + dbgp.MaxValueBytes>>20, maxValueOffset) + } // The engine sends a value's first bytes, not a range: ask for those up // to the end of this part, not the whole value each time, and a few more // so that the part can end on a whole character. @@ -243,7 +252,7 @@ func (t *tools) variableValue(_ context.Context, req *mcp.CallToolRequest, in va return nil, variableValueOutput{}, fmt.Errorf("offset must be from 0 to %d, the size of %s", size, in.Name) } out := variableValueOutput{Name: in.Name, Value: cutText(value[in.Offset:], maxTextBytes), Size: size} - if end := in.Offset + len(out.Value); end < size { + if end := in.Offset + len(out.Value); end < size && end <= maxValueOffset { out.Next = end } return nil, out, nil diff --git a/cmd/dbgp/mcp_inspect_test.go b/cmd/dbgp/mcp_inspect_test.go index ccfa499..33deb7f 100644 --- a/cmd/dbgp/mcp_inspect_test.go +++ b/cmd/dbgp/mcp_inspect_test.go @@ -1,6 +1,7 @@ package main import ( + "os" "path/filepath" "strings" "testing" @@ -210,3 +211,35 @@ func TestMCPInspectRunningSession(t *testing.T) { } } } + +// variable_value reads up to dbgp.MaxValueBytes of a value, which fits in a +// packet; past that it says so, rather than ask for more than fits. +func TestMCPVariableValueLimit(t *testing.T) { + requireEngine(t) + script := filepath.Join(t.TempDir(), "big.php") + if err := os.WriteFile(script, []byte(" maxNotificationBytes/16 { + return // only a malicious engine sends one this large + } s.mu.Lock() defer s.mu.Unlock() s.notes = append(s.notes, n) - if over := len(s.notes) - maxNotifications; over > 0 { + s.notesBytes += size + over := 0 + for len(s.notes)-over > maxNotifications || s.notesBytes > maxNotificationBytes { + s.notesBytes -= s.notes[over].size() + over++ + } + if over > 0 { s.notes = append([]Notification(nil), s.notes[over:]...) s.notesBase += over } } +// size approximates the memory a notification takes: its strings, and a +// little for the rest. +func (n Notification) size() int { + size := 64 + len(n.Name) + if m := n.Message; m != nil { + size += len(m.Filename) + len(m.Exception) + len(m.Type) + len(m.Code) + len(m.Text) + } + if b := n.Breakpoint; b != nil { + size += len(b.Type) + len(b.Filename) + len(b.Function) + len(b.State) + len(b.Resolved) + + len(b.Exception) + len(b.HitCondition) + len(b.Expression) + } + return size +} + +// cutUTF8 returns at most the first n bytes of s, ending on a whole +// character. +func cutUTF8(s string, n int) string { + if len(s) <= n { + return s + } + for n > 0 && !utf8.RuneStart(s[n]) { + n-- + } + return s[:n] +} + // formatCommand builds a DBGp command line: // // name -a value ... -i id [-- base64(data)] @@ -921,7 +1010,7 @@ func (o PropertyOptions) args(fullname string) []string { args = append(args, "-p", strconv.Itoa(o.Page)) } if o.MaxData > 0 { - args = append(args, "-m", strconv.Itoa(o.MaxData)) + args = append(args, "-m", strconv.Itoa(min(o.MaxData, MaxValueBytes))) } return args } diff --git a/session_test.go b/session_test.go index 857436e..cbcbf46 100644 --- a/session_test.go +++ b/session_test.go @@ -12,6 +12,7 @@ import ( "sync" "testing" "time" + "unicode/utf8" ) func TestSessionInit(t *testing.T) { @@ -1096,3 +1097,77 @@ func TestSetPropertyFailure(t *testing.T) { t.Error("SetProperty succeeded although the engine reported failure") } } + +// Sessions take turns reading packets when together they would read more +// than the server allows at once, and a session waiting for its turn still +// ends when it is closed. +func TestPacketBudget(t *testing.T) { + sess, e := startFakeSession(t, standardHandlers()) + srv := sess.server + stream := `` + + base64.StdEncoding.EncodeToString([]byte("hi")) + `` + + // With the whole budget taken, the packet waits. + if err := srv.packetBudget.Acquire(context.Background(), maxPacketsInFlight); err != nil { + t.Fatal(err) + } + e.Send(stream) + time.Sleep(100 * time.Millisecond) + if text, _, _ := sess.Output(0); text != "" { + t.Fatalf("output %q read without budget", text) + } + srv.packetBudget.Release(maxPacketsInFlight) + waitFor(t, func() bool { text, _, _ := sess.Output(0); return text == "hi" }) + + // Closing the session ends the wait. + if err := srv.packetBudget.Acquire(context.Background(), maxPacketsInFlight); err != nil { + t.Fatal(err) + } + defer srv.packetBudget.Release(maxPacketsInFlight) + e.Send(stream) + time.Sleep(100 * time.Millisecond) + _ = sess.Close() + select { + case <-sess.Done(): + case <-time.After(3 * time.Second): + t.Fatal("session waiting for budget did not end when closed") + } +} + +// Notifications are bounded in bytes too, and each message in length. +func TestNotificationBytesAreBounded(t *testing.T) { + s := &Session{} + long := strings.Repeat("é", maxMessageBytes) // two bytes each + for i := 0; i < maxNotifications; i++ { + s.addNotification(Notification{Name: strconv.Itoa(i), Message: &Message{Text: long}}) + } + notes, next := s.Notifications(0) + if next != maxNotifications { + t.Errorf("next = %d, want %d", next, maxNotifications) + } + text := notes[len(notes)-1].Message.Text + if len(text) > maxMessageBytes || len(text) < maxMessageBytes-1 || !utf8.ValidString(text) { + t.Errorf("message cut to %d bytes, valid UTF-8 %v; want about %d", len(text), utf8.ValidString(text), maxMessageBytes) + } + total := 0 + for _, n := range notes { + total += n.size() + } + if total > maxNotificationBytes || s.notesBytes != total || len(notes) == maxNotifications { + t.Errorf("%d notes of %d bytes (counted %d), want at most %d bytes, oldest dropped", + len(notes), total, s.notesBytes, maxNotificationBytes) + } + + // One far too large to be real is dropped. + s.addNotification(Notification{Name: "huge", Message: &Message{Filename: strings.Repeat("x", maxNotificationBytes)}}) + if _, after := s.Notifications(0); after != next { + t.Errorf("a huge notification was kept") + } +} + +func TestPropertyMaxDataIsCapped(t *testing.T) { + args := PropertyOptions{MaxData: 1 << 30}.args("$big") + if got := args[len(args)-1]; got != strconv.Itoa(MaxValueBytes) { + t.Errorf("args = %v, want -m %d", args, MaxValueBytes) + } +} From 96494454458ad0b2a3a0d460d0d4d7489f37e4ff Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 21:28:46 +0200 Subject: [PATCH 03/12] Handshake limits --- server.go | 21 ++++++++++++++++-- server_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ session.go | 8 ++++++- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/server.go b/server.go index 9a3fd2c..c7ac49d 100644 --- a/server.go +++ b/server.go @@ -73,6 +73,11 @@ const ( // maxLiveSessions bounds sessions that have not ended. Beyond it, new // scripts are detached, so they run on without the debugger. maxLiveSessions = 16 + // setupTimeout bounds setting up a session after its init packet: + // features, output capture and breakpoints, a command each. A + // connection that takes longer is dropped, so one that stops answering + // frees its handshake slot. + setupTimeout = 30 * time.Second // maxPacketsInFlight bounds the bytes of the packets a server's sessions // are reading and parsing at once, which takes a few times as much // memory. At least maxPacketBytes, so every packet fits. @@ -115,6 +120,7 @@ type Server struct { warnings []string // addresses the last StartListening could not use closed bool handshakes map[net.Conn]struct{} + reserved int // live session slots taken by handshakes setting up nextID int sessions []*Session queue []*Session // not yet returned by WaitForSession @@ -124,6 +130,7 @@ type Server struct { // The connection limits; tests lower them. maxHandshakes, maxLiveSessions int + setupTimeout time.Duration // packetBudget holds maxPacketsInFlight bytes, which sessions take // while they read and parse a packet. @@ -162,6 +169,7 @@ func NewServer(cfg Config) (*Server, error) { arrived: make(chan struct{}), maxHandshakes: maxHandshakes, maxLiveSessions: maxLiveSessions, + setupTimeout: setupTimeout, packetBudget: semaphore.NewWeighted(maxPacketsInFlight), }, nil } @@ -363,25 +371,34 @@ func (s *Server) handshake(conn net.Conn) { return } + // The slot is reserved until the session is added, so handshakes + // setting up at the same time cannot go past the limit together. s.mu.Lock() - if s.liveSessionsLocked() >= s.maxLiveSessions { + if s.liveSessionsLocked()+s.reserved >= s.maxLiveSessions { s.mu.Unlock() detachConn(conn) return } + s.reserved++ s.nextID++ id := s.nextID + setupTimeout := s.setupTimeout s.mu.Unlock() + // One deadline for the whole setup: once it passes, the session ends + // and its remaining commands fail at once. + _ = conn.SetDeadline(time.Now().Add(setupTimeout)) sess := newSession(id, s, conn, reader, init) sess.setup(s.cfg, s.pathMap()) // Nothing reports errors here: the first Continue reports rejected // breakpoints, and retries the others. _ = sess.syncBreakpoints(false) + _ = conn.SetDeadline(time.Time{}) s.mu.Lock() defer s.mu.Unlock() - if s.closed { + s.reserved-- + if s.closed || sess.ended() { _ = sess.Close() return } diff --git a/server_test.go b/server_test.go index 85f78a0..8ecb52d 100644 --- a/server_test.go +++ b/server_test.go @@ -912,3 +912,61 @@ func TestSessionRemoteAddr(t *testing.T) { t.Errorf("RemoteAddr() = %s, want 127.0.0.1", sess.RemoteAddr()) } } + +// A connection that sends its init packet and then stops answering is +// dropped once setup takes too long, which frees its handshake slot. +func TestServerDropsSilentSetup(t *testing.T) { + srv := newTestServer(t, Config{}) + setLimits(srv, 1, maxLiveSessions) + srv.mu.Lock() + srv.setupTimeout = 300 * time.Millisecond + srv.mu.Unlock() + + silent := dialFakeEngine(t, srv, fixture(t, "init"), map[string]fakeHandler{ + "feature_set": func(*fakeEngine, fakeCommand) []string { return nil }, + }) + if !silent.WaitClosed(3 * time.Second) { + t.Fatal("a connection that stopped answering during setup was kept") + } + dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) + sess := waitSession(t, srv) + if got := srv.Sessions(); len(got) != 1 || got[0] != sess { + t.Errorf("sessions = %v, want only the one that answered", got) + } +} + +// Handshakes setting up at the same time cannot go past the live session +// limit together. +func TestServerLimitsConcurrentHandshakes(t *testing.T) { + srv := newTestServer(t, Config{}) + setLimits(srv, maxHandshakes, 1) + slow := func() map[string]fakeHandler { + handlers := standardHandlers() + handlers["feature_set"] = func(e *fakeEngine, cmd fakeCommand) []string { + time.Sleep(100 * time.Millisecond) + return reply("feature_set")(e, cmd) + } + return handlers + } + var engines []*fakeEngine + for range 3 { + engines = append(engines, dialFakeEngine(t, srv, fixture(t, "init"), slow())) + } + detached := func() int { + n := 0 + for _, e := range engines { + select { + case <-e.done: + n++ + default: + } + } + return n + } + waitFor(t, func() bool { return detached() == 2 }) + waitFor(t, func() bool { return len(srv.Sessions()) == 1 }) + time.Sleep(200 * time.Millisecond) + if n := len(srv.Sessions()); n != 1 || detached() != 2 { + t.Errorf("%d sessions and %d detached, want 1 and 2", n, detached()) + } +} diff --git a/session.go b/session.go index a3e1893..946d55a 100644 --- a/session.go +++ b/session.go @@ -645,8 +645,14 @@ func (s *Session) sendCommand(name string, args []string, data []byte) (*Respons s.pending = p s.mu.Unlock() - if _, err := s.conn.Write([]byte(line + "\x00")); err != nil { + // A peer that stops reading cannot block a write for long. A command + // cut short would garble the ones after it, so the connection ends. + _ = s.conn.SetWriteDeadline(time.Now().Add(commandTimeout)) + _, err = s.conn.Write([]byte(line + "\x00")) + _ = s.conn.SetWriteDeadline(time.Time{}) + if err != nil { s.clearPending(p) + _ = s.conn.Close() return nil, fmt.Errorf("send command: %w", err) } From a7d5509f453834a37630a0879b29340d706c1891 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 21:36:40 +0200 Subject: [PATCH 04/12] Agent cleanup --- cmd/dbgp/mcp.go | 55 +++++++++++++----- cmd/dbgp/mcp_daemon.go | 59 +++++++++++-------- cmd/dbgp/mcp_daemon_test.go | 48 ++++++++++++++- cmd/dbgp/mcp_sessions.go | 7 ++- cmd/dbgp/mcp_sessions_test.go | 106 ++++++++++++++++++++++++++-------- server.go | 32 +++++++--- server_test.go | 40 +++++++++++-- session.go | 3 + 8 files changed, 269 insertions(+), 81 deletions(-) diff --git a/cmd/dbgp/mcp.go b/cmd/dbgp/mcp.go index 4f928d7..b5b2b71 100644 --- a/cmd/dbgp/mcp.go +++ b/cmd/dbgp/mcp.go @@ -352,9 +352,12 @@ type tools struct { // allowCode allows what runs the agent's PHP code in the debugged script. allowCode bool - // mu guards agents, by agent id: see agentID. + // mu guards agents, by agent id (see agentID), and detaching. mu sync.Mutex agents map[string]*agentState + // detaching holds the ids of the sessions being detached because the + // agents that took them left: no agent can take them meanwhile. + detaching map[int]bool } // agentState is what the tools keep for one agent. Agents sharing the @@ -402,33 +405,53 @@ func (a *agentState) take(id int) { // scripts run on instead of holding their PHP workers, unless another agent // has taken them too. func (t *tools) leave(id string) error { + orphans, err := t.release(id) + t.detachSessions(orphans) + return err +} + +// release forgets the agent with the given id, stops listening if it was +// the last agent listening, and returns the live sessions only it took, +// marked as detaching, so no other agent takes one before it is detached. +func (t *tools) release(id string) ([]*dbgp.Session, error) { t.mu.Lock() + defer t.mu.Unlock() a := t.agents[id] if a == nil { - t.mu.Unlock() - return nil + return nil, nil } delete(t.agents, id) - var orphans []int + var orphans []*dbgp.Session for sid := range a.taken { - if !t.taken(sid) { - orphans = append(orphans, sid) + if t.taken(sid) { + continue + } + if s := t.srv.Session(sid); s != nil && !ended(s.State()) { + if t.detaching == nil { + t.detaching = map[int]bool{} + } + t.detaching[sid] = true + orphans = append(orphans, s) } } - var err error if a.listening && t.listeners() == 0 { - err = t.srv.StopListening() + return orphans, t.srv.StopListening() } - t.mu.Unlock() + return orphans, nil +} - // Outside t.mu: detaching a stopped session waits for PHP's answer. - for _, sid := range orphans { - if s := t.srv.Session(sid); s != nil && !ended(s.State()) { - _ = s.Detach() - _ = s.Close() - } +// detachSessions detaches the sessions release returned, outside t.mu: +// detaching a stopped session waits for PHP's answer. Once they have ended, +// agents can name them again, to read their output. +func (t *tools) detachSessions(sessions []*dbgp.Session) { + for _, s := range sessions { + _ = s.Detach() + _ = s.Close() + <-s.Done() + t.mu.Lock() + delete(t.detaching, s.ID()) + t.mu.Unlock() } - return err } // taken reports whether any agent has taken session id. t.mu must be held. diff --git a/cmd/dbgp/mcp_daemon.go b/cmd/dbgp/mcp_daemon.go index 6dd137c..3c7e5fe 100644 --- a/cmd/dbgp/mcp_daemon.go +++ b/cmd/dbgp/mcp_daemon.go @@ -375,7 +375,11 @@ func bridge(ctx context.Context, st *daemonState) error { if err != nil { return err } - defer leaveServer(client, st.URL) + presence, err := attachServer(ctx, client, st.URL) + if err != nil { + return err + } + defer presence.Close() remote, err := (&mcp.StreamableClientTransport{Endpoint: st.URL, HTTPClient: client}).Connect(ctx) if err != nil { return err @@ -404,36 +408,39 @@ func bridge(ctx context.Context, st *daemonState) error { return nil } -// leavePath is where dbgp mcp tells the server its agent has gone, so it -// no longer keeps dbgp listening. -const leavePath = "/leave" +// attachPath is where dbgp mcp tells the server its agent is there, for as +// long as it runs: it keeps the request open, and the server takes the +// agent as gone when the request ends. The operating system ends it when +// dbgp mcp ends, even when it crashes or is killed. +const attachPath = "/attach" -// leaveTimeout is how long dbgp mcp waits for the server to hear it has -// gone. -const leaveTimeout = 2 * time.Second - -// leaveServer tells the server at rawURL, through client, that the agent -// client sends the id of has gone. It is best effort: the agent has -// already gone, and so has its dbgp mcp when the server cannot be reached. -func leaveServer(client *http.Client, rawURL string) { +// attachServer tells the server at rawURL, through client, that the agent +// client sends the id of is there, until the returned body is closed or +// ctx is done. +func attachServer(ctx context.Context, client *http.Client, rawURL string) (io.Closer, error) { u, err := url.Parse(rawURL) if err != nil { - return + return nil, err } - u.Path = leavePath - ctx, cancel := context.WithTimeout(context.Background(), leaveTimeout) - defer cancel() + u.Path = attachPath req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), nil) if err != nil { - return + return nil, err + } + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("attach to the background server: %w", err) } - if resp, err := client.Do(req); err == nil { + if resp.StatusCode != http.StatusOK { _ = resp.Body.Close() + return nil, fmt.Errorf("attach to the background server: %s", resp.Status) } + return resp.Body, nil } -// leave forgets the agent each request is from, by its agentHeader. -func leave(t *tools) http.Handler { +// attach holds each request open until the agent it is from, by its +// agentHeader, has gone, then forgets that agent. +func attach(t *tools) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if req.Method != http.MethodPost { w.Header().Set("Allow", http.MethodPost) @@ -445,11 +452,13 @@ func leave(t *tools) http.Handler { http.Error(w, "no "+agentHeader+" header", http.StatusBadRequest) return } + w.WriteHeader(http.StatusOK) + _ = http.NewResponseController(w).Flush() + // Done when dbgp mcp closes the connection or the server stops. + <-req.Context().Done() if err := t.leave(id); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return + fmt.Fprintf(os.Stderr, "dbgp: agent left: %v\n", err) } - w.WriteHeader(http.StatusNoContent) }) } @@ -526,14 +535,14 @@ func serveHTTP(ctx context.Context, server *mcp.Server, t *tools, port int) erro return nil } -// httpHandler serves MCP at /mcp and agents leaving, to requests with st's +// httpHandler serves MCP at /mcp and agents' presence, to requests with st's // token, and the identity check. t are server's tools. func httpHandler(server *mcp.Server, t *tools, st daemonState) http.Handler { handler := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return server }, &mcp.StreamableHTTPOptions{CrossOriginProtection: http.NewCrossOriginProtection()}) mux := http.NewServeMux() mux.Handle("/mcp", requireToken(st.Token, handler)) - mux.Handle(leavePath, requireToken(st.Token, leave(t))) + mux.Handle(attachPath, requireToken(st.Token, attach(t))) mux.Handle(identifyPath, identify(st)) return mux } diff --git a/cmd/dbgp/mcp_daemon_test.go b/cmd/dbgp/mcp_daemon_test.go index 680c714..c510266 100644 --- a/cmd/dbgp/mcp_daemon_test.go +++ b/cmd/dbgp/mcp_daemon_test.go @@ -212,9 +212,9 @@ func TestMCPBridge(t *testing.T) { if err := bridged.Close(); err != nil { t.Errorf("close: %v", err) } - // Its dbgp mcp told the server the agent left, which counts as unlisten. - callTool(t, direct, "status", nil, &status) - if status.Listening { + // Its dbgp mcp ending tells the server the agent left, which counts as + // unlisten. + if !stopsListening(t, direct) { t.Error("still listening after the only agent that called listen left") } if !strings.Contains(stderr.String(), "using the background server at "+url) || !strings.Contains(stderr.String(), "-listen ignored") { @@ -224,6 +224,48 @@ func TestMCPBridge(t *testing.T) { callTool(t, direct, "status", nil, &status) } +// stopsListening reports whether the server cs is connected to stops +// listening within 5s. +func stopsListening(t *testing.T, cs *mcp.ClientSession) bool { + t.Helper() + for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(20 * time.Millisecond) { + var status statusOutput + callTool(t, cs, "status", nil, &status) + if !status.Listening { + return true + } + } + return false +} + +// An agent's dbgp mcp that is killed, so it cannot say it is leaving, +// still leaves: the operating system closes its connection to the server. +func TestMCPBridgeKilled(t *testing.T) { + bin := buildBinary(t) + daemonEnv(t) + url := startTestDaemon(t, bin) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, bin, "mcp") + bridged, err := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "0"}, nil). + Connect(ctx, &mcp.CommandTransport{Command: cmd}, nil) + if err != nil { + t.Fatal(err) + } + defer bridged.Close() + direct := connectDirect(ctx, t, url) + defer direct.Close() + + callTool(t, bridged, "listen", nil, &listenOutput{}) + if err := cmd.Process.Kill(); err != nil { + t.Fatal(err) + } + if !stopsListening(t, direct) { + t.Error("still listening after the only agent that called listen was killed") + } +} + // connectDirect connects to the background server over HTTP, with the token // from the state file, as dbgp mcp does. func connectDirect(ctx context.Context, t *testing.T, url string) *mcp.ClientSession { diff --git a/cmd/dbgp/mcp_sessions.go b/cmd/dbgp/mcp_sessions.go index db53a8f..74f688e 100644 --- a/cmd/dbgp/mcp_sessions.go +++ b/cmd/dbgp/mcp_sessions.go @@ -112,9 +112,14 @@ func (t *tools) session(req *mcp.CallToolRequest, id int) (*dbgp.Session, error) if s == nil { return nil, fmt.Errorf("no session %d", id) } + if t.detaching[id] { + return nil, fmt.Errorf("session %d is being detached: the agent that took it has left", id) + } // Named by id, it is taken: wait_for_session will not return it, and it // is not detached for waiting too long. - t.srv.Claim(s) + if err := t.srv.Claim(s); errors.Is(err, dbgp.ErrUnclaimed) { + return nil, fmt.Errorf("session %d is being detached: no agent took it within a minute of connecting", id) + } a.take(id) return s, nil } diff --git a/cmd/dbgp/mcp_sessions_test.go b/cmd/dbgp/mcp_sessions_test.go index 0fbf24a..e098b86 100644 --- a/cmd/dbgp/mcp_sessions_test.go +++ b/cmd/dbgp/mcp_sessions_test.go @@ -305,7 +305,7 @@ func TestMCPSelectedSession(t *testing.T) { // newSharedMCP serves the tools over HTTP as the background server does, // with token, on a PHP port that is not listening yet. -func newSharedMCP(t *testing.T, token string) (string, *dbgp.Server) { +func newSharedMCP(t *testing.T, token string) (string, *dbgp.Server, *tools) { t.Helper() srv, err := dbgp.NewServer(dbgp.Config{Addr: phpAddr(0)}) if err != nil { @@ -315,7 +315,7 @@ func newSharedMCP(t *testing.T, token string) (string, *dbgp.Server) { server, tl := newMCPServer(srv, true, true) hs := httptest.NewServer(httpHandler(server, tl, daemonState{Token: token})) t.Cleanup(hs.Close) - return hs.URL + "/mcp", srv + return hs.URL + "/mcp", srv, tl } // connectAgent connects to the server at url as dbgp mcp does, as agent. @@ -337,7 +337,7 @@ func connectAgent(t *testing.T, url, token, agent string) *mcp.ClientSession { // Agents on the background server are told they share it; an agent on // stdin and stdout has dbgp to itself. func TestMCPSharedInstructions(t *testing.T) { - url, _ := newSharedMCP(t, "token") + url, _, _ := newSharedMCP(t, "token") shared := connectAgent(t, url, "token", "agent").InitializeResult().Instructions if !strings.HasSuffix(shared, sharedInstructions) { t.Errorf("background server instructions lack the shared ones:\n%s", shared) @@ -353,7 +353,7 @@ func TestMCPSharedInstructions(t *testing.T) { // other's. func TestMCPAgentsSharingServer(t *testing.T) { requireEngine(t) - url, srv := newSharedMCP(t, "token") + url, srv, _ := newSharedMCP(t, "token") a := connectAgent(t, url, "token", "agent-a") b := connectAgent(t, url, "token", "agent-b") @@ -400,25 +400,44 @@ func TestMCPAgentsSharingServer(t *testing.T) { } } -// On the background server, dbgp listens while any agent that called -// listen has not called unlisten, or left: one agent does not stop PHP -// connections that another is waiting for. -// leaveAs tells the server at url that agent has gone, as its dbgp mcp -// does when it ends. -func leaveAs(t *testing.T, url, agent string) { +// attachAs tells the server at url, whose tools are tl, that agent is +// there, as its dbgp mcp does when it starts. It returns the function that +// ends that, as dbgp mcp ending does, and waits until tl has forgotten the +// agent. +func attachAs(t *testing.T, url string, tl *tools, agent string) (leave func()) { t.Helper() client, err := tokenClient(url, "token", agent) if err != nil { t.Fatal(err) } - leaveServer(client, url) + presence, err := attachServer(context.Background(), client, url) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = presence.Close() }) + return func() { + t.Helper() + _ = presence.Close() + for deadline := time.Now().Add(5 * time.Second); ; time.Sleep(10 * time.Millisecond) { + tl.mu.Lock() + _, present := tl.agents[agent] + tl.mu.Unlock() + if !present { + return + } + if time.Now().After(deadline) { + t.Fatalf("%s still known 5s after leaving", agent) + } + } + } } // An agent that leaves without ending its sessions does not hold their PHP // processes: they are detached, unless another agent took them too. func TestMCPLeaveDetachesSessions(t *testing.T) { requireEngine(t) - url, srv := newSharedMCP(t, "token") + url, srv, tl := newSharedMCP(t, "token") + leaveA, leaveB := attachAs(t, url, tl, "agent-a"), attachAs(t, url, tl, "agent-b") a := connectAgent(t, url, "token", "agent-a") b := connectAgent(t, url, "token", "agent-b") callTool(t, a, "listen", nil, &listenOutput{}) @@ -456,7 +475,7 @@ func TestMCPLeaveDetachesSessions(t *testing.T) { shared := start(b, "shared") callTool(t, a, "output", map[string]any{"session": shared}, &outputOutput{}) - leaveAs(t, url, "agent-a") + leaveA() if !detached(own) || !finished("own") { t.Error("a's session was not detached when a left") } @@ -465,14 +484,54 @@ func TestMCPLeaveDetachesSessions(t *testing.T) { t.Error("the session b took too was detached when a left") case <-time.After(300 * time.Millisecond): } - leaveAs(t, url, "agent-b") + leaveB() if !detached(shared) || !finished("shared") { t.Error("the shared session was not detached when b left too") } } +// A session the leaving agent took is detached as one step: no other agent +// can take it after it was picked for detaching, and have it detached under +// them; once it has ended, they can name it again. +func TestMCPLeaveRace(t *testing.T) { + requireEngine(t) + srv, err := dbgp.Listen(dbgp.Config{Addr: "127.0.0.1:0"}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = srv.Close() }) + startPHP(t, srv.Port(), "-r", `echo "hi";`) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + s, err := srv.WaitForSession(ctx) + if err != nil { + t.Fatal(err) + } + as := func(agent string) *mcp.CallToolRequest { + return &mcp.CallToolRequest{Extra: &mcp.RequestExtra{Header: http.Header{agentHeader: {agent}}}} + } + tl := &tools{srv: srv} + tl.selectSession(as("a"), s) + + orphans, err := tl.release("a") + if err != nil || len(orphans) != 1 || orphans[0] != s { + t.Fatalf("release = %v, %v; want session %d", orphans, err, s.ID()) + } + if _, err := tl.session(as("b"), s.ID()); err == nil || !strings.Contains(err.Error(), "being detached") { + t.Errorf("taking a session picked for detaching: %v", err) + } + tl.detachSessions(orphans) + if got, err := tl.session(as("b"), s.ID()); err != nil || got != s || !ended(s.State()) { + t.Errorf("after detaching: session %v, %v; want the ended session", got, err) + } +} + +// On the background server, dbgp listens while any agent that called +// listen has not called unlisten, or left: one agent does not stop PHP +// connections that another is waiting for. func TestMCPListeningPerAgent(t *testing.T) { - url, srv := newSharedMCP(t, "token") + url, srv, tl := newSharedMCP(t, "token") + leaveA, leaveB := attachAs(t, url, tl, "agent-a"), attachAs(t, url, tl, "agent-b") a := connectAgent(t, url, "token", "agent-a") b := connectAgent(t, url, "token", "agent-b") unlisten := func(name string, cs *mcp.ClientSession, listening bool, others int) { @@ -495,25 +554,24 @@ func TestMCPListeningPerAgent(t *testing.T) { callTool(t, a, "listen", nil, &listenOutput{}) unlisten("b, not listening", b, true, 1) - // An agent leaving, as its dbgp mcp tells the server when it ends, - // counts as unlisten. - leaveAs(t, url, "agent-b") + // An agent leaving, as its dbgp mcp ending tells the server, counts as + // unlisten. + leaveB() if !srv.Listening() { t.Error("b leaving stopped a's listening") } - leaveAs(t, url, "agent-a") + leaveA() if srv.Listening() { t.Error("still listening after a, the only agent listening, left") } - // Only agents with the token can leave. - callTool(t, a, "listen", nil, &listenOutput{}) - resp, err := http.Post(strings.TrimSuffix(url, "/mcp")+leavePath, "", nil) + // Only agents with the token can attach, and so leave. + resp, err := http.Post(strings.TrimSuffix(url, "/mcp")+attachPath, "", nil) if err != nil { t.Fatal(err) } _ = resp.Body.Close() - if resp.StatusCode != http.StatusUnauthorized || !srv.Listening() { - t.Errorf("leave without the token: status %d, listening %v; want 401 and still listening", resp.StatusCode, srv.Listening()) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("attach without the token: status %d, want 401", resp.StatusCode) } } diff --git a/server.go b/server.go index c7ac49d..bd3d0c2 100644 --- a/server.go +++ b/server.go @@ -24,6 +24,10 @@ const DefaultAddr = "localhost:9003" // ErrServerClosed is returned by WaitForSession after Close. var ErrServerClosed = errors.New("server closed") +// ErrUnclaimed is returned by Claim for a session being detached because +// nobody took it within Config.ClaimTimeout. +var ErrUnclaimed = errors.New("session is being detached: nobody took it in time") + // StdoutMode selects what happens to a session's program output. type StdoutMode int @@ -413,15 +417,21 @@ func (s *Server) handshake(conn net.Conn) { } // detachUnclaimed detaches sess if nobody has taken it, so its script runs -// on instead of waiting. +// on instead of waiting. Meanwhile Claim refuses it. func (s *Server) detachUnclaimed(sess *Session) { s.mu.Lock() unclaimed := !s.closed && s.unqueueLocked(sess) + sess.unclaimed = unclaimed s.mu.Unlock() - if unclaimed { - _ = sess.Detach() - _ = sess.Close() + if !unclaimed { + return } + _ = sess.Detach() + _ = sess.Close() + <-sess.Done() + s.mu.Lock() + sess.unclaimed = false + s.mu.Unlock() } // reservePacket waits until the server's sessions are reading few enough @@ -599,12 +609,18 @@ func (s *Server) WaitForSession(ctx context.Context) (*Session, error) { } // Claim takes sess as if WaitForSession had returned it, so WaitForSession -// does not return it and Config.ClaimTimeout no longer applies to it. It -// reports whether sess was still waiting to be taken. -func (s *Server) Claim(sess *Session) bool { +// does not return it and Config.ClaimTimeout no longer applies to it. +// Claiming a session already taken does nothing. It returns ErrUnclaimed +// while sess is being detached because nobody took it in time; once that +// is done, the session has ended. +func (s *Server) Claim(sess *Session) error { s.mu.Lock() defer s.mu.Unlock() - return s.unqueueLocked(sess) + if sess.unclaimed { + return ErrUnclaimed + } + s.unqueueLocked(sess) + return nil } // unqueueLocked removes sess from the sessions WaitForSession has not diff --git a/server_test.go b/server_test.go index 8ecb52d..19d3949 100644 --- a/server_test.go +++ b/server_test.go @@ -6,6 +6,7 @@ import ( "errors" "io" "net" + "slices" "sort" "strconv" "strings" @@ -880,11 +881,11 @@ func TestServerDetachesUnclaimedSessions(t *testing.T) { waited := dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) waitSession(t, srv) claimed := dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) - if !srv.Claim(waitForSessionID(t, srv, 3)) { - t.Error("Claim of a new session = false") + if err := srv.Claim(waitForSessionID(t, srv, 3)); err != nil { + t.Errorf("Claim of a new session: %v", err) } - if srv.Claim(srv.Session(3)) || srv.Claim(srv.Session(2)) { - t.Error("Claim of a taken session = true") + if err := errors.Join(srv.Claim(srv.Session(3)), srv.Claim(srv.Session(2))); err != nil { + t.Errorf("Claim of a taken session: %v", err) } time.Sleep(3 * timeout) for _, e := range []*fakeEngine{waited, claimed} { @@ -970,3 +971,34 @@ func TestServerLimitsConcurrentHandshakes(t *testing.T) { t.Errorf("%d sessions and %d detached, want 1 and 2", n, detached()) } } + +// While a session nobody took is being detached, nobody can claim it, so it +// is not detached under them; once it has ended, claiming it is harmless. +func TestClaimWhileDetachingUnclaimed(t *testing.T) { + answer := make(chan struct{}) + var once sync.Once + answerDetach := func() { once.Do(func() { close(answer) }) } + defer answerDetach() + handlers := standardHandlers() + handlers["detach"] = func(e *fakeEngine, cmd fakeCommand) []string { + <-answer + return reply("detach")(e, cmd) + } + srv := newTestServer(t, Config{ClaimTimeout: 100 * time.Millisecond}) + e := dialFakeEngine(t, srv, fixture(t, "init"), handlers) + sess := waitForSessionID(t, srv, 1) + waitFor(t, func() bool { + return slices.ContainsFunc(e.Received(), func(c fakeCommand) bool { return c.Name == "detach" }) + }) + + if err := srv.Claim(sess); !errors.Is(err, ErrUnclaimed) { + t.Errorf("Claim while detaching = %v, want ErrUnclaimed", err) + } + answerDetach() + select { + case <-sess.Done(): + case <-time.After(3 * time.Second): + t.Fatal("the unclaimed session did not end") + } + waitFor(t, func() bool { return srv.Claim(sess) == nil }) +} diff --git a/session.go b/session.go index 946d55a..8ec1c4a 100644 --- a/session.go +++ b/session.go @@ -84,6 +84,9 @@ type Session struct { closed chan struct{} // closed when the read loop exits // connCtx is cancelled when conn is closed. connCtx context.Context + // unclaimed is set while the session is being detached because nobody + // took it in time. Guarded by server.mu. + unclaimed bool // cmdSlot serializes commands: at most one is in flight, so an error // response without a transaction_id belongs to it. From af7037617795d34b959651007e820d81b12a0652 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 21:43:22 +0200 Subject: [PATCH 05/12] skip the probe on no-eval --- cmd/dbgp/mcp.go | 2 ++ cmd/dbgp/mcp_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++ server.go | 9 ++++++- server_test.go | 5 ++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/cmd/dbgp/mcp.go b/cmd/dbgp/mcp.go index b5b2b71..ec7d9eb 100644 --- a/cmd/dbgp/mcp.go +++ b/cmd/dbgp/mcp.go @@ -191,6 +191,8 @@ func runMCP(args []string) int { srv, err := dbgp.NewServer(dbgp.Config{ Addr: phpAddr(*phpPort), IDEKey: *idekey, PathMap: pathMap, ClaimTimeout: claimTimeout, + // The tools never read the engine mapping that the probe reports. + NoEval: *noEval, }) if err != nil { fmt.Fprintln(os.Stderr, err) diff --git a/cmd/dbgp/mcp_test.go b/cmd/dbgp/mcp_test.go index 1238ac7..e4618fa 100644 --- a/cmd/dbgp/mcp_test.go +++ b/cmd/dbgp/mcp_test.go @@ -1,13 +1,16 @@ package main import ( + "bufio" "context" "encoding/json" "fmt" "net" + "os" "os/exec" "path/filepath" "reflect" + "slices" "strconv" "strings" "testing" @@ -545,3 +548,56 @@ func TestMCPListenWarnings(t *testing.T) { t.Errorf("listen = %+v, want port %s with a warning about [::1]", out, port) } } + +// dbgp mcp -no-eval sends no eval of its own to the engine: the setup +// probe for the engine's path mapping is skipped. +func TestMCPNoEvalSendsNoEval(t *testing.T) { + bin := buildBinary(t) + daemonEnv(t) + init, err := os.ReadFile("../../testdata/dbgp/init.xml") + if err != nil { + t.Fatal(err) + } + for _, noEval := range []bool{false, true} { + args := []string{"mcp", "-php-port", "0"} + if noEval { + args = append(args, "-no-eval") + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cs, err := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "0"}, nil). + Connect(ctx, &mcp.CommandTransport{Command: exec.CommandContext(ctx, bin, args...)}, nil) + if err != nil { + t.Fatal(err) + } + defer cs.Close() + var listening listenOutput + callTool(t, cs, "listen", nil, &listening) + + // An engine that answers every setup command, until output capture, + // the last one. + conn, err := net.Dial("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(listening.Port))) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(10 * time.Second)) + fmt.Fprintf(conn, "%d\x00%s\x00", len(init), init) + reader := bufio.NewReader(conn) + var commands []string + for !slices.Contains(commands, "stdout") { + line, err := reader.ReadString(0) + if err != nil { + t.Fatalf("%v: reading commands %v: %v", args, commands, err) + } + fields := strings.Fields(strings.TrimSuffix(line, "\x00")) + id := fields[slices.Index(fields, "-i")+1] + commands = append(commands, fields[0]) + resp := fmt.Sprintf(``, fields[0], id) + fmt.Fprintf(conn, "%d\x00%s\x00", len(resp), resp) + } + if sent := slices.Contains(commands, "eval"); sent == noEval { + t.Errorf("%v: setup commands %v, eval sent %v", args, commands, sent) + } + } +} diff --git a/server.go b/server.go index bd3d0c2..5eb86a3 100644 --- a/server.go +++ b/server.go @@ -60,6 +60,11 @@ type Config struct { // PathMap maps local directories to the engine's, for PHP running in a // container or on a remote server. Sessions take and return local paths. PathMap []PathMapping + // NoEval stops the server from running PHP code of its own in sessions: + // setup skips the eval that asks the engine whether it maps paths + // itself, so Session.EngineMapping reports nothing detected. Session + // methods that run code, such as EvalProperty, still do when called. + NoEval bool // ClaimTimeout, when set, detaches sessions that neither WaitForSession // nor Claim has taken within this time of connecting, so their scripts // run on without the debugger instead of waiting for someone to debug @@ -472,7 +477,9 @@ func (sess *Session) setup(cfg Config, paths PathMap) { _ = sess.FeatureSet("notify_ok", "1") _ = sess.FeatureSet("resolved_breakpoints", "1") // Before cfg.Features, which may lower max_data below the probe's reply. - sess.detectEngineMapping(paths) + if !cfg.NoEval { + sess.detectEngineMapping(paths) + } names := make([]string, 0, len(cfg.Features)) for name := range cfg.Features { diff --git a/server_test.go b/server_test.go index 19d3949..3643325 100644 --- a/server_test.go +++ b/server_test.go @@ -128,6 +128,11 @@ func TestServerSessionSetup(t *testing.T) { "feature_set -n resolved_breakpoints -v 1", probeCommand, }}, + {"no eval", Config{NoEval: true}, []string{ + "feature_set -n notify_ok -v 1", + "feature_set -n resolved_breakpoints -v 1", + "stdout -c 1", + }}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From fdb8f04f1949d13051db397f8794b53611b5421e Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 21:49:22 +0200 Subject: [PATCH 06/12] Windows path case --- pathmap.go | 30 ++++++++++++++++++++----- pathmap_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ server.go | 2 +- 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/pathmap.go b/pathmap.go index c54bf52..8afa316 100644 --- a/pathmap.go +++ b/pathmap.go @@ -79,18 +79,38 @@ func (m PathMap) ToLocal(remote string) string { // cutPathPrefix reports whether p is prefix or inside it, and returns the // remainder. Matching is by whole path components: /var/www does not -// contain /var/www2. +// contain /var/www2. For a Windows prefix it ignores case, as Windows does: +// C:\Project contains c:\project\a.php. func cutPathPrefix(p, prefix, sep string) (string, bool) { - if p == prefix { + if samePath(p, prefix) { return "", true } if !strings.HasSuffix(prefix, sep) { prefix += sep } - if rest, ok := strings.CutPrefix(p, prefix); ok { - return rest, true + if len(p) < len(prefix) || !samePath(p[:len(prefix)], prefix) { + return "", false } - return "", false + return p[len(prefix):], true +} + +// samePath reports whether paths a and b are the same: regardless of case +// for Windows paths, exactly for others. +func samePath(a, b string) bool { + if windowsPath(a) || windowsPath(b) { + return strings.EqualFold(a, b) + } + return a == b +} + +// windowsPath reports whether p is a Windows path, which Windows compares +// regardless of case: one that starts with a drive, such as C:\dir or +// C:/dir, or a UNC share, such as \\server\share or //server/share. +func windowsPath(p string) bool { + if len(p) >= 2 && p[1] == ':' && 'a' <= p[0]|0x20 && p[0]|0x20 <= 'z' { + return true + } + return strings.HasPrefix(p, `\\`) || strings.HasPrefix(p, "//") } func joinPath(dir, rest, sep string) string { diff --git a/pathmap_test.go b/pathmap_test.go index 66ef4f4..60ec530 100644 --- a/pathmap_test.go +++ b/pathmap_test.go @@ -358,3 +358,62 @@ func TestAddPathMappingWhileRunning(t *testing.T) { t.Errorf("commands = %q, want %q", strings.Join(names, " "), want) } } + +// Windows compares paths regardless of case, so mappings for Windows paths +// do too, on either side; other paths are compared exactly. +func TestPathMapWindowsCase(t *testing.T) { + m := mustPathMap(t, + PathMapping{Local: "/home/me/win", Remote: `C:\Inetpub\wwwroot`}, + PathMapping{Local: "/home/me/share", Remote: "//FileServer/php"}, + PathMapping{Local: "/home/me/project", Remote: "/var/www/html"}, + ) + tests := []struct{ remote, local string }{ + // PHP on Windows may send a path in another case: what follows the + // mapped directory keeps its own. + {"c:/inetpub/WWWROOT/Default.php", "/home/me/win/Default.php"}, + {`C:\INETPUB\wwwroot`, "/home/me/win"}, + {"//fileserver/PHP/app.php", "/home/me/share/app.php"}, + // Linux paths are case-sensitive. + {"/VAR/www/html/a.php", "/VAR/www/html/a.php"}, + // Whole components only, in any case. + {"C:/inetpub/wwwroot2/a.php", "C:/inetpub/wwwroot2/a.php"}, + } + for _, tt := range tests { + if got := m.ToLocal(tt.remote); got != filepath.FromSlash(tt.local) { + t.Errorf("ToLocal(%q) = %q, want %q", tt.remote, got, filepath.FromSlash(tt.local)) + } + } + if got := m.localPath("file:///c:/inetpub/wwwroot/x.php"); got != filepath.FromSlash("/home/me/win/x.php") { + t.Errorf("localPath(lower-case drive) = %q", got) + } + + // Local Windows paths, as dbgp on Windows sees them. + for _, tt := range []struct { + p, prefix, sep, rest string + ok bool + }{ + {`c:\project\src\a.php`, `C:\Project`, `\`, `src\a.php`, true}, + {`C:\PROJECT`, `C:\Project`, `\`, "", true}, + {`\\server\SHARE\a.php`, `\\Server\share`, `\`, "a.php", true}, + {`C:\Project2\a.php`, `C:\Project`, `\`, "", false}, + {"/home/Me/a.php", "/home/me", "/", "", false}, + } { + if rest, ok := cutPathPrefix(tt.p, tt.prefix, tt.sep); rest != tt.rest || ok != tt.ok { + t.Errorf("cutPathPrefix(%q, %q) = %q, %v; want %q, %v", tt.p, tt.prefix, rest, ok, tt.rest, tt.ok) + } + } +} + +// A mapping for a Windows directory replaces one for the same directory in +// another case. +func TestAddPathMappingWindowsCase(t *testing.T) { + srv := newTestServer(t, Config{}) + for _, local := range []string{`C:\Project`, `c:\project`} { + if err := srv.AddPathMapping(PathMapping{Local: local, Remote: "/app"}); err != nil { + t.Fatal(err) + } + } + if got := srv.PathMap(); len(got) != 1 || got[0].Local != `c:\project` { + t.Errorf("PathMap = %+v, want only the newer mapping", got) + } +} diff --git a/server.go b/server.go index 5eb86a3..27f5dc3 100644 --- a/server.go +++ b/server.go @@ -531,7 +531,7 @@ func (s *Server) AddPathMapping(m PathMapping) error { s.pathsMu.Lock() paths := make(PathMap, 0, len(s.paths)+1) for _, old := range s.paths { - if old.Local != m.Local { + if !samePath(old.Local, m.Local) { paths = append(paths, old) } } From 881ee09f4eb90e360ae730c7886eb0885a801eae Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 21:52:57 +0200 Subject: [PATCH 07/12] README security section --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index 6cd6c22..1ff2b87 100644 --- a/README.md +++ b/README.md @@ -81,3 +81,10 @@ php app.php Each script or request that connects becomes a session, stopped at its start. At the prompt, `run` runs to the next breakpoint, `next`, `step` and `out` step through the code, and `stack`, `vars` and `print $user["name"]` show where it stopped. On a terminal it has line editing, tab completion and history. Type `help` at the prompt for all the commands, and see `dbgp -h` for the options, such as `-map` for PHP in a container or on a server. + +## Security + +Like any debugger, dbgp trusts the PHP that connects to it and the agent that uses it: + +- **The PHP port.** DBGp has no authentication: anything that can reach the port, 9003 by default, can connect as PHP and show dbgp made-up code, variables and output, which agents then read. dbgp listens on every address of this machine so that PHP in a container or on another machine can connect, so make sure only your PHP hosts and container networks can reach that port, for example with a firewall. `-idekey` accepts only sessions with that IDE key, which keeps out other PHP, but it is sent in plain text and is not a password. To debug PHP across a network you do not trust, connect through an SSH tunnel or a VPN. `dbgp mcp` only listens while an agent is debugging, and every session shows the address it connected from. +- **The agent.** An agent using dbgp can see everything the debugged script can: its variables, including passwords and keys, and any file PHP can read. `-no-eval` stops the agent from running PHP code of its own, but does not hide anything from it. From fcf8a3d9e2ea63a33b3d409eab065fc186b6e6bd Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 19 Oct 2026 22:01:28 +0200 Subject: [PATCH 08/12] test write deadline --- server.go | 7 ++++++ session.go | 50 ++++++++++++++++++++++++++++----------- session_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 14 deletions(-) diff --git a/server.go b/server.go index 27f5dc3..099450a 100644 --- a/server.go +++ b/server.go @@ -57,6 +57,10 @@ type Config struct { // InitTimeout bounds how long a new connection may take to send its // init packet. Default: 10s. InitTimeout time.Duration + // CommandTimeout bounds how long a command may take to reach the engine + // and be answered; continuing the script is not bounded (see + // Session.Continue). Default: 30s. + CommandTimeout time.Duration // PathMap maps local directories to the engine's, for PHP running in a // container or on a remote server. Sessions take and return local paths. PathMap []PathMapping @@ -166,6 +170,9 @@ func NewServer(cfg Config) (*Server, error) { if cfg.InitTimeout <= 0 { cfg.InitTimeout = 10 * time.Second } + if cfg.CommandTimeout <= 0 { + cfg.CommandTimeout = defaultCommandTimeout + } paths, err := newPathMap(cfg.PathMap) if err != nil { return nil, err diff --git a/session.go b/session.go index 8ec1c4a..90b9b30 100644 --- a/session.go +++ b/session.go @@ -18,9 +18,8 @@ import ( "unicode/utf8" ) -// commandTimeout bounds how long a command waits for its response. -// Continuation commands are not bounded by it: see Session.Continue. -const commandTimeout = 30 * time.Second +// defaultCommandTimeout is Config.CommandTimeout's default. +const defaultCommandTimeout = 30 * time.Second // Continuation commands resume the script, which then runs until it stops // again: at a breakpoint, after the step, or at the end of the script. @@ -84,6 +83,8 @@ type Session struct { closed chan struct{} // closed when the read loop exits // connCtx is cancelled when conn is closed. connCtx context.Context + // cmdTimeout is Config.CommandTimeout; see commandTimeout. + cmdTimeout time.Duration // unclaimed is set while the session is being detached because nobody // took it in time. Guarded by server.mu. unclaimed bool @@ -216,6 +217,9 @@ func newSession(id int, server *Server, conn net.Conn, reader *bufio.Reader, ini state: State{Status: StatusStarting}, changed: make(chan struct{}), } + if server != nil { + s.cmdTimeout = server.cfg.CommandTimeout + } go s.readLoop(reader) return s } @@ -626,7 +630,7 @@ func (s *Session) checkCanSendLocked() error { // sendCommand sends a command and waits for its response. Commands are // serialized, so this also waits for any command already in flight. func (s *Session) sendCommand(name string, args []string, data []byte) (*Response, error) { - timeout := time.NewTimer(commandTimeout) + timeout := time.NewTimer(s.commandTimeout()) defer timeout.Stop() if err := s.acquire(name, timeout.C); err != nil { return nil, err @@ -648,15 +652,9 @@ func (s *Session) sendCommand(name string, args []string, data []byte) (*Respons s.pending = p s.mu.Unlock() - // A peer that stops reading cannot block a write for long. A command - // cut short would garble the ones after it, so the connection ends. - _ = s.conn.SetWriteDeadline(time.Now().Add(commandTimeout)) - _, err = s.conn.Write([]byte(line + "\x00")) - _ = s.conn.SetWriteDeadline(time.Time{}) - if err != nil { + if err := s.writeCommand(line); err != nil { s.clearPending(p) - _ = s.conn.Close() - return nil, fmt.Errorf("send command: %w", err) + return nil, err } select { @@ -735,7 +733,7 @@ func (s *Session) Continue(ctx context.Context, command string, wait time.Durati // startContinuation sends a continuation command without waiting for the // script to stop; the response arrives when it does. func (s *Session) startContinuation(command string) error { - timeout := time.NewTimer(commandTimeout) + timeout := time.NewTimer(s.commandTimeout()) defer timeout.Stop() if err := s.acquire(command, timeout.C); err != nil { return err @@ -760,18 +758,42 @@ func (s *Session) startContinuation(command string) error { s.setStateLocked(State{Status: StatusRunning, Reason: "ok"}) s.mu.Unlock() - if _, err := s.conn.Write([]byte(line + "\x00")); err != nil { + if err := s.writeCommand(line); err != nil { s.mu.Lock() if s.contID == id { s.contID = 0 s.setStateLocked(prev) } s.mu.Unlock() + return err + } + return nil +} + +// writeCommand sends a command line to the engine. A peer that stops +// reading cannot block it for longer than the command timeout. A command +// cut short would garble the ones after it, so then the connection ends. +func (s *Session) writeCommand(line string) error { + _ = s.conn.SetWriteDeadline(time.Now().Add(s.commandTimeout())) + _, err := s.conn.Write([]byte(line + "\x00")) + _ = s.conn.SetWriteDeadline(time.Time{}) + if err != nil { + _ = s.conn.Close() return fmt.Errorf("send command: %w", err) } return nil } +// commandTimeout bounds how long a command waits to be sent and for its +// response. Continuation commands wait for their response unbounded: see +// Session.Continue. +func (s *Session) commandTimeout() time.Duration { + if s.cmdTimeout > 0 { + return s.cmdTimeout + } + return defaultCommandTimeout +} + // Wait waits up to wait for a running script to stop and returns the // session state, which has Status running if it has not stopped yet. func (s *Session) Wait(ctx context.Context, wait time.Duration) (State, error) { diff --git a/session_test.go b/session_test.go index cbcbf46..988c4ef 100644 --- a/session_test.go +++ b/session_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/base64" "errors" + "fmt" "io" "reflect" "strconv" @@ -1171,3 +1172,65 @@ func TestPropertyMaxDataIsCapped(t *testing.T) { t.Errorf("args = %v, want -m %d", args, MaxValueBytes) } } + +// A command the engine does not answer fails after Config.CommandTimeout. +func TestCommandTimeout(t *testing.T) { + srv := newTestServer(t, Config{CommandTimeout: 200 * time.Millisecond}) + handlers := standardHandlers() + handlers["stack_get"] = func(*fakeEngine, fakeCommand) []string { return nil } + dialFakeEngine(t, srv, fixture(t, "init"), handlers) + sess := waitSession(t, srv) + start := time.Now() + if _, err := sess.GetStack(); err == nil || !strings.Contains(err.Error(), "timeout waiting for response") { + t.Errorf("GetStack = %v, want a timeout", err) + } + if took := time.Since(start); took > 2*time.Second { + t.Errorf("GetStack took %v, want about 200ms", took) + } +} + +// An engine that stops reading cannot block a command for longer than the +// command timeout: the write fails, and the session ends, since the rest of +// the command would garble what follows. +func TestWriteDeadline(t *testing.T) { + srv := newTestServer(t, Config{CommandTimeout: 200 * time.Millisecond}) + conn := dialServer(t, srv) + init := fixture(t, "init") + fmt.Fprintf(conn, "%d\x00%s\x00", len(init), init) + // Answer the setup commands, up to output capture, the last one. + reader := bufio.NewReader(conn) + for { + line, err := reader.ReadString(0) + if err != nil { + t.Fatal(err) + } + cmd := parseFakeCommand(strings.TrimSuffix(line, "\x00")) + resp := fmt.Sprintf(``, cmd.Name, cmd.Args["i"]) + fmt.Fprintf(conn, "%d\x00%s\x00", len(resp), resp) + if cmd.Name == "stdout" { + break + } + } + sess := waitSession(t, srv) + + // From now on the engine reads nothing: a command larger than the + // socket buffers cannot be written whole. + done := make(chan error, 1) + go func() { + _, err := sess.EvalProperty(strings.Repeat("1+", 16<<20)+"1", 0) + done <- err + }() + select { + case err := <-done: + if err == nil || !strings.Contains(err.Error(), "send command") { + t.Errorf("EvalProperty = %v, want the write to fail", err) + } + case <-time.After(3 * time.Second): + t.Fatal("a command to an engine that stopped reading was still being written after 3s") + } + select { + case <-sess.Done(): + case <-time.After(3 * time.Second): + t.Error("the session did not end after a write failed") + } +} From e34a86faad10b00e77c8b1803b314a4b7bf47a51 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Tue, 20 Oct 2026 20:41:15 +0200 Subject: [PATCH 09/12] close sessions --- cmd/dbgp/mcp_daemon.go | 67 ++++++++++++++++++++++++++++++++--- cmd/dbgp/mcp_sessions_test.go | 61 +++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/cmd/dbgp/mcp_daemon.go b/cmd/dbgp/mcp_daemon.go index 3c7e5fe..1b3fee7 100644 --- a/cmd/dbgp/mcp_daemon.go +++ b/cmd/dbgp/mcp_daemon.go @@ -18,6 +18,7 @@ import ( "os/exec" "path/filepath" "strconv" + "sync" "time" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -439,8 +440,9 @@ func attachServer(ctx context.Context, client *http.Client, rawURL string) (io.C } // attach holds each request open until the agent it is from, by its -// agentHeader, has gone, then forgets that agent. -func attach(t *tools) http.Handler { +// agentHeader, has gone, then forgets that agent and closes its MCP +// sessions. +func attach(t *tools, sessions *mcpSessions) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if req.Method != http.MethodPost { w.Header().Set("Allow", http.MethodPost) @@ -459,9 +461,65 @@ func attach(t *tools) http.Handler { if err := t.leave(id); err != nil { fmt.Fprintf(os.Stderr, "dbgp: agent left: %v\n", err) } + sessions.close(id) }) } +// mcpSessionHeader carries the id of the MCP session a request belongs to, +// which the MCP library assigns in its answer to initialize. +const mcpSessionHeader = "Mcp-Session-Id" + +// mcpSessions records which MCP sessions each agent uses, to close them when +// it leaves: the MCP library keeps a session until its client ends it, which +// a dbgp mcp that crashed or was killed never does. +type mcpSessions struct { + server *mcp.Server + + mu sync.Mutex + byAgent map[string]map[string]bool // session ids, by agent id +} + +// track passes requests on to next, recording the MCP session each is from +// for the agent it is from. +func (m *mcpSessions) track(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + agent := req.Header.Get(agentHeader) + m.add(agent, req.Header.Get(mcpSessionHeader)) + next.ServeHTTP(w, req) + // The answer to initialize, which the session starts with. + m.add(agent, w.Header().Get(mcpSessionHeader)) + }) +} + +func (m *mcpSessions) add(agent, session string) { + if agent == "" || session == "" { + return + } + m.mu.Lock() + defer m.mu.Unlock() + if m.byAgent == nil { + m.byAgent = map[string]map[string]bool{} + } + if m.byAgent[agent] == nil { + m.byAgent[agent] = map[string]bool{} + } + m.byAgent[agent][session] = true +} + +// close closes the MCP sessions of the agent with the given id, which has +// left, and forgets them. +func (m *mcpSessions) close(agent string) { + m.mu.Lock() + ids := m.byAgent[agent] + delete(m.byAgent, agent) + m.mu.Unlock() + for ss := range m.server.Sessions() { + if ids[ss.ID()] { + _ = ss.Close() + } + } +} + // agentHeader carries the id of the agent a request is from, which its // dbgp mcp chooses: see agentID. const agentHeader = "Dbgp-Agent" @@ -540,9 +598,10 @@ func serveHTTP(ctx context.Context, server *mcp.Server, t *tools, port int) erro func httpHandler(server *mcp.Server, t *tools, st daemonState) http.Handler { handler := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return server }, &mcp.StreamableHTTPOptions{CrossOriginProtection: http.NewCrossOriginProtection()}) + sessions := &mcpSessions{server: server} mux := http.NewServeMux() - mux.Handle("/mcp", requireToken(st.Token, handler)) - mux.Handle(attachPath, requireToken(st.Token, attach(t))) + mux.Handle("/mcp", requireToken(st.Token, sessions.track(handler))) + mux.Handle(attachPath, requireToken(st.Token, attach(t, sessions))) mux.Handle(identifyPath, identify(st)) return mux } diff --git a/cmd/dbgp/mcp_sessions_test.go b/cmd/dbgp/mcp_sessions_test.go index e098b86..3d832b4 100644 --- a/cmd/dbgp/mcp_sessions_test.go +++ b/cmd/dbgp/mcp_sessions_test.go @@ -526,6 +526,67 @@ func TestMCPLeaveRace(t *testing.T) { } } +// An agent that leaves without ending its MCP session, as when its dbgp mcp +// is killed, does not leave the session behind; other agents keep theirs. +func TestMCPLeaveClosesMCPSessions(t *testing.T) { + srv, err := dbgp.NewServer(dbgp.Config{Addr: phpAddr(0)}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = srv.Close() }) + server, tl := newMCPServer(srv, true, true) + hs := httptest.NewServer(httpHandler(server, tl, daemonState{Token: "token"})) + t.Cleanup(hs.Close) + url := hs.URL + "/mcp" + sessions := func() []string { + var ids []string + for ss := range server.Sessions() { + ids = append(ids, ss.ID()) + } + return ids + } + + waitSessions := func(want int, after string) { + t.Helper() + for deadline := time.Now().Add(5 * time.Second); len(sessions()) != want; time.Sleep(10 * time.Millisecond) { + if time.Now().After(deadline) { + t.Fatalf("MCP sessions after %s = %v, want %d", after, sessions(), want) + } + } + } + + leaveA := attachAs(t, url, tl, "agent-a") + attachAs(t, url, tl, "agent-b") + // Neither closes its MCP session. + for _, agent := range []string{"agent-a", "agent-b"} { + callTool(t, connectAgent(t, url, "token", agent), "status", nil, &statusOutput{}) + } + waitSessions(2, "two agents connected") + leaveA() + waitSessions(1, "a left") + + // An agent killed right after initialize, before it sent anything + // with its session id. + leaveC := attachAs(t, url, tl, "agent-c") + body := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"c","version":"0"}}}` + req, err := http.NewRequest(http.MethodPost, url, strings.NewReader(body)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer token") + req.Header.Set(agentHeader, "agent-c") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + waitSessions(2, "c initialized") + leaveC() + waitSessions(1, "c left") +} + // On the background server, dbgp listens while any agent that called // listen has not called unlisten, or left: one agent does not stop PHP // connections that another is waiting for. From 4302eba1ca84fa07140b8e61903af71f824772df Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Wed, 21 Oct 2026 08:52:23 +0200 Subject: [PATCH 10/12] MacOS path mapping --- pathmap.go | 26 ++++++++++++++--------- pathmap_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++- server.go | 4 ++-- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/pathmap.go b/pathmap.go index 8afa316..6fbb647 100644 --- a/pathmap.go +++ b/pathmap.go @@ -4,9 +4,15 @@ import ( "fmt" "net/url" "path/filepath" + "runtime" "strings" ) +// localIgnoresCase reports whether local paths are compared regardless of +// case: on Windows, and on macOS, whose disks ignore case unless they were +// formatted otherwise. It is a variable so tests can change it. +var localIgnoresCase = runtime.GOOS == "windows" || runtime.GOOS == "darwin" + // PathMapping maps a local directory to the directory the engine sees, for // PHP running in a container or on a remote server. type PathMapping struct { @@ -52,7 +58,7 @@ func (m PathMap) ToRemote(local string) string { local = filepath.Clean(local) best, rest := -1, "" for i, pm := range m { - if r, ok := cutPathPrefix(local, pm.Local, string(filepath.Separator)); ok && (best < 0 || len(pm.Local) > len(m[best].Local)) { + if r, ok := cutPathPrefix(local, pm.Local, string(filepath.Separator), localIgnoresCase); ok && (best < 0 || len(pm.Local) > len(m[best].Local)) { best, rest = i, r } } @@ -67,7 +73,7 @@ func (m PathMap) ToLocal(remote string) string { remote = cleanRemote(remote) best, rest := -1, "" for i, pm := range m { - if r, ok := cutPathPrefix(remote, pm.Remote, "/"); ok && (best < 0 || len(pm.Remote) > len(m[best].Remote)) { + if r, ok := cutPathPrefix(remote, pm.Remote, "/", false); ok && (best < 0 || len(pm.Remote) > len(m[best].Remote)) { best, rest = i, r } } @@ -79,25 +85,25 @@ func (m PathMap) ToLocal(remote string) string { // cutPathPrefix reports whether p is prefix or inside it, and returns the // remainder. Matching is by whole path components: /var/www does not -// contain /var/www2. For a Windows prefix it ignores case, as Windows does: -// C:\Project contains c:\project\a.php. -func cutPathPrefix(p, prefix, sep string) (string, bool) { - if samePath(p, prefix) { +// contain /var/www2. It ignores case if ignoreCase is set, and for a Windows +// prefix, as Windows does: C:\Project contains c:\project\a.php. +func cutPathPrefix(p, prefix, sep string, ignoreCase bool) (string, bool) { + if samePath(p, prefix, ignoreCase) { return "", true } if !strings.HasSuffix(prefix, sep) { prefix += sep } - if len(p) < len(prefix) || !samePath(p[:len(prefix)], prefix) { + if len(p) < len(prefix) || !samePath(p[:len(prefix)], prefix, ignoreCase) { return "", false } return p[len(prefix):], true } // samePath reports whether paths a and b are the same: regardless of case -// for Windows paths, exactly for others. -func samePath(a, b string) bool { - if windowsPath(a) || windowsPath(b) { +// if ignoreCase is set or for Windows paths, exactly otherwise. +func samePath(a, b string, ignoreCase bool) bool { + if ignoreCase || windowsPath(a) || windowsPath(b) { return strings.EqualFold(a, b) } return a == b diff --git a/pathmap_test.go b/pathmap_test.go index 60ec530..6cfd3a9 100644 --- a/pathmap_test.go +++ b/pathmap_test.go @@ -398,7 +398,7 @@ func TestPathMapWindowsCase(t *testing.T) { {`C:\Project2\a.php`, `C:\Project`, `\`, "", false}, {"/home/Me/a.php", "/home/me", "/", "", false}, } { - if rest, ok := cutPathPrefix(tt.p, tt.prefix, tt.sep); rest != tt.rest || ok != tt.ok { + if rest, ok := cutPathPrefix(tt.p, tt.prefix, tt.sep, false); rest != tt.rest || ok != tt.ok { t.Errorf("cutPathPrefix(%q, %q) = %q, %v; want %q, %v", tt.p, tt.prefix, rest, ok, tt.rest, tt.ok) } } @@ -417,3 +417,57 @@ func TestAddPathMappingWindowsCase(t *testing.T) { t.Errorf("PathMap = %+v, want only the newer mapping", got) } } + +// On macOS and Windows, whose disks ignore case, local paths match a +// mapping in any case. Remote paths are still compared exactly, since PHP +// in a Linux container sees /var/www and /VAR/www as different. +func TestPathMapLocalCase(t *testing.T) { + for _, ignoreCase := range []bool{true, false} { + setLocalIgnoresCase(t, ignoreCase) + m := mustPathMap(t, PathMapping{Local: "/Users/me/Project", Remote: "/var/www/html"}) + + local := filepath.FromSlash("/users/ME/project/src/A.php") + want := local // no mapping matches + if ignoreCase { + want = "/var/www/html/src/A.php" // the rest keeps its own case + } + if got := m.ToRemote(local); got != want { + t.Errorf("ignoreCase=%v: ToRemote(%q) = %q, want %q", ignoreCase, local, got, want) + } + // Whole components only. + other := filepath.FromSlash("/users/me/project2/a.php") + if got := m.ToRemote(other); got != other { + t.Errorf("ignoreCase=%v: ToRemote(%q) = %q, want it unchanged", ignoreCase, other, got) + } + if got, want := m.ToLocal("/VAR/www/html/a.php"), filepath.FromSlash("/VAR/www/html/a.php"); got != want { + t.Errorf("ignoreCase=%v: ToLocal = %q, want %q", ignoreCase, got, want) + } + } +} + +// On macOS and Windows, a mapping for a local directory replaces one for +// the same directory in another case. +func TestAddPathMappingLocalCase(t *testing.T) { + for _, ignoreCase := range []bool{true, false} { + setLocalIgnoresCase(t, ignoreCase) + srv := newTestServer(t, Config{}) + for _, local := range []string{"/Users/me/Project", "/users/me/project"} { + if err := srv.AddPathMapping(PathMapping{Local: local, Remote: "/app"}); err != nil { + t.Fatal(err) + } + } + want := 2 + if ignoreCase { + want = 1 + } + if got := srv.PathMap(); len(got) != want || got[len(got)-1].Local != filepath.FromSlash("/users/me/project") { + t.Errorf("ignoreCase=%v: PathMap = %+v, want %d mappings, ending with the newer one", ignoreCase, got, want) + } + } +} + +func setLocalIgnoresCase(t *testing.T, v bool) { + old := localIgnoresCase + localIgnoresCase = v + t.Cleanup(func() { localIgnoresCase = old }) +} diff --git a/server.go b/server.go index 099450a..611fcea 100644 --- a/server.go +++ b/server.go @@ -525,7 +525,7 @@ func (s *Server) PathMap() []PathMapping { } // AddPathMapping adds a mapping, or replaces the one for the same local -// directory. It applies to every session at once: paths from the engine are +// directory, in any case on Windows and macOS. It applies to every session at once: paths from the engine are // translated with it from now on, and breakpoints already set are set again // with their new paths, now in stopped sessions and on the next Continue in // running ones. The returned error lists sessions that rejected a breakpoint. @@ -538,7 +538,7 @@ func (s *Server) AddPathMapping(m PathMapping) error { s.pathsMu.Lock() paths := make(PathMap, 0, len(s.paths)+1) for _, old := range s.paths { - if !samePath(old.Local, m.Local) { + if !samePath(old.Local, m.Local, localIgnoresCase) { paths = append(paths, old) } } From 146269c94ee242d76acb141a9e60e54ad134d562 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Thu, 22 Oct 2026 18:24:42 +0200 Subject: [PATCH 11/12] Windows tests --- .github/workflows/tests.yml | 36 +++++++++++++---- cmd/dbgp/mcp_output.go | 1 + cmd/dbgp/mcp_output_test.go | 2 +- cmd/dbgp/mcp_test.go | 25 +++++++++--- cmd/dbgp/repl.go | 1 + cmd/dbgp/repl_test.go | 23 +++++------ integration_test.go | 8 +++- listen.go | 27 +++++++++++++ listen_darwin.go | 15 ------- listen_nonwindows.go | 8 ++++ listen_other.go | 3 +- listen_darwin_test.go => listen_test.go | 52 ++++++++++++++++++++++--- listen_windows.go | 22 +++++++++++ pathmap.go | 2 +- pathmap_test.go | 8 ++-- protocol_test.go | 7 ++-- server.go | 2 +- server_test.go | 6 +++ session_test.go | 17 +++++--- 19 files changed, 201 insertions(+), 64 deletions(-) create mode 100644 listen.go create mode 100644 listen_nonwindows.go rename listen_darwin_test.go => listen_test.go (52%) create mode 100644 listen_windows.go diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1d47cc3..dbdf73e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -34,12 +34,20 @@ jobs: run: go vet ./... test: - name: test (PHP ${{ matrix.php }}) - runs-on: ubuntu-latest + # Linux with every PHP version, and Windows with the newest. + name: test (${{ matrix.os == 'windows-latest' && 'Windows, ' || '' }}PHP ${{ matrix.php }}) + runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: + os: [ubuntu-latest] php: ['8.2', '8.3', '8.4', '8.5'] + include: + - os: windows-latest + php: '8.5' + defaults: + run: + shell: bash steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -47,18 +55,30 @@ jobs: - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: go-version-file: go.mod - # Self-contained PHP interpreter with PHP Debugger compiled in, from the - # latest php-debugger release. + # PHP Debugger, from its latest release: on Linux, a self-contained PHP + # interpreter with it compiled in; on Windows, where the release has only + # the extension, the official PHP build with the extension loaded. - name: Install PHP Debugger env: GH_TOKEN: ${{ github.token }} run: | dir="$RUNNER_TEMP/php-debugger/bin" mkdir -p "$dir" - gh release download --repo php-debugger/php-debugger \ - --pattern "php-php${{ matrix.php }}-nts-linux-x86_64" \ - --output "$dir/php" - chmod +x "$dir/php" + if [ "$RUNNER_OS" = Windows ]; then + curl -fsSL --retry 5 --retry-all-errors -o "$RUNNER_TEMP/php.zip" \ + "https://downloads.php.net/~windows/releases/latest/php-${{ matrix.php }}-nts-Win32-vs17-x64-latest.zip" + unzip -q "$RUNNER_TEMP/php.zip" -d "$dir" + gh release download --repo php-debugger/php-debugger \ + --pattern "php_php-debugger-php${{ matrix.php }}-nts-windows-x64.dll" \ + --output "$dir/ext/php_php_debugger.dll" + # PHP reads the php.ini next to php.exe. + printf 'extension_dir="%s"\nzend_extension=php_debugger\n' "$(cygpath -w "$dir/ext")" > "$dir/php.ini" + else + gh release download --repo php-debugger/php-debugger \ + --pattern "php-php${{ matrix.php }}-nts-linux-x86_64" \ + --output "$dir/php" + chmod +x "$dir/php" + fi echo "$dir" >> "$GITHUB_PATH" - name: Show PHP and debugger versions run: | diff --git a/cmd/dbgp/mcp_output.go b/cmd/dbgp/mcp_output.go index 9e2e9ad..8e69ead 100644 --- a/cmd/dbgp/mcp_output.go +++ b/cmd/dbgp/mcp_output.go @@ -107,6 +107,7 @@ func (t *tools) source(_ context.Context, req *mcp.CallToolRequest, in sourceInp return nil, out, nil } for i, code := range strings.Split(strings.TrimSuffix(text, "\n"), "\n") { + code = strings.TrimSuffix(code, "\r") // Windows line endings n := begin + i short := cutText(code, maxLineBytes) out.Lines = append(out.Lines, sourceLine{ diff --git a/cmd/dbgp/mcp_output_test.go b/cmd/dbgp/mcp_output_test.go index 5874658..d20647d 100644 --- a/cmd/dbgp/mcp_output_test.go +++ b/cmd/dbgp/mcp_output_test.go @@ -47,7 +47,7 @@ func TestMCPSource(t *testing.T) { }{ {map[string]any{"around": 101}, "around must be between 1 and 100"}, {map[string]any{"around": -1}, "around must be between 1 and 100"}, - {map[string]any{"file": "/no/such/file.php"}, "PHP cannot read /no/such/file.php"}, + {map[string]any{"file": "/no/such/file.php"}, "PHP cannot read " + absPath(t, "/no/such/file.php")}, {map[string]any{"file": "../../testdata/php/basic.php"}, `file must be an absolute path, not "../../testdata/php/basic.php"`}, } { if msg := callToolError(t, cs, "source", tc.args); !strings.Contains(msg, tc.want) { diff --git a/cmd/dbgp/mcp_test.go b/cmd/dbgp/mcp_test.go index e4618fa..0096421 100644 --- a/cmd/dbgp/mcp_test.go +++ b/cmd/dbgp/mcp_test.go @@ -10,6 +10,7 @@ import ( "os/exec" "path/filepath" "reflect" + "runtime" "slices" "strconv" "strings" @@ -210,7 +211,7 @@ func TestMCPStatusShowsBreakpointsAndMappings(t *testing.T) { if fmt.Sprint(status.Breakpoints) != fmt.Sprint(want) { t.Errorf("breakpoints = %+v, want %+v", status.Breakpoints, want) } - if len(status.PathMappings) != 1 || status.PathMappings[0] != (pathMapping{Local: "/home/me/app", Remote: "/var/www/html"}) { + if len(status.PathMappings) != 1 || status.PathMappings[0] != (pathMapping{Local: filepath.FromSlash("/home/me/app"), Remote: "/var/www/html"}) { t.Errorf("pathMappings = %+v", status.PathMappings) } } @@ -249,6 +250,17 @@ func TestMCPStatusShowsSession(t *testing.T) { } } +// absPath returns the absolute path dbgp makes of p, a path from the root +// such as /app/a.php: on Windows, it is on the current drive, D:\app\a.php. +func absPath(t *testing.T, p string) string { + t.Helper() + abs, err := filepath.Abs(filepath.FromSlash(p)) + if err != nil { + t.Fatal(err) + } + return abs +} + // The built binary in MCP mode, driven over stdin and stdout. // buildBinary builds dbgp into a temporary directory and returns its path. func buildBinary(t *testing.T, args ...string) string { @@ -257,6 +269,9 @@ func buildBinary(t *testing.T, args ...string) string { t.Skip("builds the binary") } bin := filepath.Join(t.TempDir(), "dbgp") + if runtime.GOOS == "windows" { + bin += ".exe" // Windows runs only files with an executable extension + } if out, err := exec.Command("go", append(append([]string{"build", "-o", bin}, args...), ".")...).CombinedOutput(); err != nil { t.Fatalf("go build: %v\n%s", err, out) } @@ -385,9 +400,9 @@ func TestMCPAddBreakpoint(t *testing.T) { args map[string]any want breakpointInfo }{ - {map[string]any{"file": "/app/a.php", "line": 12}, breakpointInfo{ID: 1, Type: "line", File: "/app/a.php", Line: 12}}, - {map[string]any{"type": "line", "file": "/app/../app/b.php", "line": 3}, breakpointInfo{ID: 2, Type: "line", File: "/app/b.php", Line: 3}}, - {map[string]any{"file": "/app/a.php", "line": 5, "condition": "$i > 2"}, breakpointInfo{ID: 3, Type: "conditional", File: "/app/a.php", Line: 5, Condition: "$i > 2"}}, + {map[string]any{"file": "/app/a.php", "line": 12}, breakpointInfo{ID: 1, Type: "line", File: absPath(t, "/app/a.php"), Line: 12}}, + {map[string]any{"type": "line", "file": "/app/../app/b.php", "line": 3}, breakpointInfo{ID: 2, Type: "line", File: absPath(t, "/app/b.php"), Line: 3}}, + {map[string]any{"file": "/app/a.php", "line": 5, "condition": "$i > 2"}, breakpointInfo{ID: 3, Type: "conditional", File: absPath(t, "/app/a.php"), Line: 5, Condition: "$i > 2"}}, {map[string]any{"type": "call", "function": "add"}, breakpointInfo{ID: 4, Type: "call", Function: "add"}}, {map[string]any{"type": "return", "function": "add"}, breakpointInfo{ID: 5, Type: "return", Function: "add"}}, {map[string]any{"type": "exception", "exception": "RuntimeException"}, breakpointInfo{ID: 6, Type: "exception", Exception: "RuntimeException"}}, @@ -457,7 +472,7 @@ func TestMCPPathMappings(t *testing.T) { callTool(t, cs, "add_path_mapping", map[string]any{"local": "/home/me/app", "remote": "/var/www/html"}, &out) callTool(t, cs, "add_path_mapping", map[string]any{"local": "/srv/../srv/php", "remote": "/srv/php"}, &out) callTool(t, cs, "add_path_mapping", map[string]any{"local": "/home/me/app/", "remote": "/app"}, &out) - want := []pathMapping{{Local: "/srv/php", Remote: "/srv/php"}, {Local: "/home/me/app", Remote: "/app"}} + want := []pathMapping{{Local: absPath(t, "/srv/php"), Remote: "/srv/php"}, {Local: absPath(t, "/home/me/app"), Remote: "/app"}} if fmt.Sprint(out.PathMappings) != fmt.Sprint(want) || out.Warning != "" { t.Errorf("add_path_mapping = %+v, want %+v", out, want) } diff --git a/cmd/dbgp/repl.go b/cmd/dbgp/repl.go index d4fc9b4..eb9b0de 100644 --- a/cmd/dbgp/repl.go +++ b/cmd/dbgp/repl.go @@ -900,6 +900,7 @@ func (r *repl) sourceLines(s *dbgp.Session, file string, line, current int) (str } var b strings.Builder for i, text := range strings.Split(strings.TrimRight(src, "\n"), "\n") { + text = strings.TrimSuffix(text, "\r") // Windows line endings n := begin + i mark := " " if n == current { diff --git a/cmd/dbgp/repl_test.go b/cmd/dbgp/repl_test.go index 98bc9c9..5de5ef0 100644 --- a/cmd/dbgp/repl_test.go +++ b/cmd/dbgp/repl_test.go @@ -39,7 +39,7 @@ func newTestREPL(t *testing.T) (*repl, *dbgp.Server, func() string) { func TestParsePathMap(t *testing.T) { abs, _ := filepath.Abs("rel") got, err := parsePathMap([]string{"/home/me/app=/var/www", "/a=C:/b", "rel=/srv"}) - want := []dbgp.PathMapping{{Local: "/home/me/app", Remote: "/var/www"}, {Local: "/a", Remote: "C:/b"}, {Local: abs, Remote: "/srv"}} + want := []dbgp.PathMapping{{Local: absPath(t, "/home/me/app"), Remote: "/var/www"}, {Local: absPath(t, "/a"), Remote: "C:/b"}, {Local: abs, Remote: "/srv"}} if err != nil || !reflect.DeepEqual(got, want) { t.Errorf("parsePathMap = %+v, %v", got, err) } @@ -57,9 +57,9 @@ func TestParseBreakpoint(t *testing.T) { args string want dbgp.Breakpoint }{ - {"/app/a.php:12", dbgp.Breakpoint{Type: "line", File: "/app/a.php", Line: 12}}, + {"/app/a.php:12", dbgp.Breakpoint{Type: "line", File: absPath(t, "/app/a.php"), Line: 12}}, {"a.php:3", dbgp.Breakpoint{Type: "line", File: abs, Line: 3}}, - {"/app/a.php:5 if $i > 2", dbgp.Breakpoint{Type: "conditional", File: "/app/a.php", Line: 5, Condition: "$i > 2"}}, + {"/app/a.php:5 if $i > 2", dbgp.Breakpoint{Type: "conditional", File: absPath(t, "/app/a.php"), Line: 5, Condition: "$i > 2"}}, {"call App\\run", dbgp.Breakpoint{Type: "call", Function: "App\\run"}}, {"return add", dbgp.Breakpoint{Type: "return", Function: "add"}}, {"exception RuntimeException", dbgp.Breakpoint{Type: "exception", Exception: "RuntimeException"}}, @@ -447,13 +447,7 @@ func TestREPLBreakpointKindsAndDetach(t *testing.T) { // The built command: flags, startup and quitting. func TestBinary(t *testing.T) { - if testing.Short() { - t.Skip("builds the binary") - } - bin := filepath.Join(t.TempDir(), "dbgp") - if out, err := exec.Command("go", "build", "-o", bin, ".").CombinedOutput(); err != nil { - t.Fatalf("go build: %v\n%s", err, out) - } + bin := buildBinary(t) out, _ := exec.Command(bin, "-h").CombinedOutput() if !strings.Contains(string(out), "Usage: dbgp [options]") || !strings.Contains(string(out), "-map LOCAL=REMOTE") { @@ -485,7 +479,7 @@ func TestBinary(t *testing.T) { cmd.Stdin = strings.NewReader("breaks\nquit\n") out, err = cmd.CombinedOutput() if err != nil || !strings.Contains(string(out), "Listening for PHP on port ") || - !strings.Contains(string(out), "1 /app/a.php:3\n") || !strings.Contains(string(out), "2 /app/b.php:5\n") { + !strings.Contains(string(out), "1 "+absPath(t, "/app/a.php")+":3\n") || !strings.Contains(string(out), "2 "+absPath(t, "/app/b.php")+":5\n") { t.Errorf("session: %v\n%s", err, out) } } @@ -577,17 +571,18 @@ func TestMapAndMaps(t *testing.T) { r.execLine("maps") abs, _ := filepath.Abs("testdata/php") - want := []dbgp.PathMapping{{Local: abs, Remote: "/srv/php"}, {Local: "/home/me/app", Remote: "/app"}} + app := absPath(t, "/home/me/app") + want := []dbgp.PathMapping{{Local: abs, Remote: "/srv/php"}, {Local: app, Remote: "/app"}} if got := srv.PathMap(); !reflect.DeepEqual(got, want) { t.Errorf("PathMap() = %+v, want %+v", got, want) } out := output() for _, line := range []string{ "No path mappings.\n", - "Mapped /home/me/app => /var/www/html\n", + "Mapped " + app + " => /var/www/html\n", "Mapped " + abs + " => /srv/php\n", "Error: usage: map LOCAL=REMOTE\n", - abs + " => /srv/php\n/home/me/app => /app\n", + abs + " => /srv/php\n" + app + " => /app\n", } { if !strings.Contains(out, line) { t.Errorf("output lacks %q", line) diff --git a/integration_test.go b/integration_test.go index 8f6bdbb..4095834 100644 --- a/integration_test.go +++ b/integration_test.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" "time" @@ -375,6 +376,11 @@ func TestEnginePathMapping(t *testing.T) { // detects it and passes local paths through. func TestEngineOwnPathMapping(t *testing.T) { requireDebugEngine(t) + if runtime.GOOS == "windows" { + // PHP Debugger reports path mapping as enabled there, but applies + // no map file. + t.Skip("PHP Debugger does not apply map files on Windows") + } src, err := os.ReadFile(debuggeePath(t)) if err != nil { t.Fatal(err) @@ -386,7 +392,7 @@ func TestEngineOwnPathMapping(t *testing.T) { } remoteScript := filepath.Join(remoteDir, "basic.php") localDir := "/home/agent/project" - script := localDir + "/basic.php" + script := filepath.FromSlash(localDir + "/basic.php") files := map[string]string{ remoteScript: string(src), filepath.Join(remoteDir, ".xdebug", "agent.map"): "remote_prefix: " + filepath.ToSlash(remoteDir) + diff --git a/listen.go b/listen.go new file mode 100644 index 0000000..6f3ddd0 --- /dev/null +++ b/listen.go @@ -0,0 +1,27 @@ +package dbgp + +import ( + "context" + "net" +) + +// listenTCP listens on addr for PHP. +func listenTCP(addr string) (net.Listener, error) { + lc := net.ListenConfig{Control: listenControl} + return lc.Listen(context.Background(), "tcp", addr) +} + +// localIPs returns this machine's addresses, except link-local ones, +// always including the loopback addresses. +func localIPs() []net.IP { + ips := []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback} + addrs, _ := net.InterfaceAddrs() + for _, a := range addrs { + ipNet, ok := a.(*net.IPNet) + if !ok || ipNet.IP.IsLinkLocalUnicast() || ipNet.IP.IsLoopback() { + continue + } + ips = append(ips, ipNet.IP) + } + return ips +} diff --git a/listen_darwin.go b/listen_darwin.go index a995029..2715307 100644 --- a/listen_darwin.go +++ b/listen_darwin.go @@ -34,21 +34,6 @@ func shadowedAddr(l net.Listener) string { return "" } -// localIPs returns this machine's addresses, except link-local ones, -// always including the loopback addresses. -func localIPs() []net.IP { - ips := []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback} - addrs, _ := net.InterfaceAddrs() - for _, a := range addrs { - ipNet, ok := a.(*net.IPNet) - if !ok || ipNet.IP.IsLinkLocalUnicast() || ipNet.IP.IsLoopback() { - continue - } - ips = append(ips, ipNet.IP) - } - return ips -} - // bindInUse reports whether binding a TCP socket to ip and port fails // because another socket is bound there. The socket is only bound, never // listening, so no connection reaches it. diff --git a/listen_nonwindows.go b/listen_nonwindows.go new file mode 100644 index 0000000..ae219ec --- /dev/null +++ b/listen_nonwindows.go @@ -0,0 +1,8 @@ +//go:build !windows + +package dbgp + +import "syscall" + +// listenControl is only needed on Windows. +var listenControl func(network, address string, c syscall.RawConn) error diff --git a/listen_other.go b/listen_other.go index 94090d1..7267ff8 100644 --- a/listen_other.go +++ b/listen_other.go @@ -5,5 +5,6 @@ package dbgp import "net" // shadowedAddr is only needed on macOS: Linux refuses to listen on every -// address when another program listens on one of them. +// address when another program listens on one of them, and so does +// Windows with the exclusive addresses dbgp asks for. func shadowedAddr(net.Listener) string { return "" } diff --git a/listen_darwin_test.go b/listen_test.go similarity index 52% rename from listen_darwin_test.go rename to listen_test.go index fe2635f..a7bbe1b 100644 --- a/listen_darwin_test.go +++ b/listen_test.go @@ -2,13 +2,24 @@ package dbgp import ( "net" + "runtime" "strings" "testing" ) -// macOS lets a server listen on every address while another program -// listens on one of them, which would then get the connections to it: -// StartListening refuses, as Linux does. +// shadowedError returns what StartListening's error says when another +// program listens on addr: dbgp's own message on macOS, which would let +// dbgp listen, and "" elsewhere, where the system refuses by itself. +func shadowedError(addr string) string { + if runtime.GOOS == "darwin" { + return "another program is listening on " + addr + } + return "" +} + +// StartListening refuses to listen on every address while another program +// listens on one of them, which would then get the connections to it. +// Linux refuses by itself; macOS does not, so dbgp checks. func TestListenRefusesShadowedAddress(t *testing.T) { hosts := []string{"127.0.0.1", "::1"} for _, ip := range localIPs() { @@ -28,7 +39,7 @@ func TestListenRefusesShadowedAddress(t *testing.T) { t.Fatal(err) } err = srv.StartListening() - want := "another program is listening on " + net.JoinHostPort(host, port) + want := shadowedError(net.JoinHostPort(host, port)) if err == nil || !strings.Contains(err.Error(), want) { t.Errorf("with %s taken: StartListening = %v, want %q", host, err, want) } @@ -58,8 +69,39 @@ func TestListenOnIPv4AnyRefusesShadowedIPv6(t *testing.T) { t.Fatal(err) } defer srv.Close() - want := "another program is listening on [::1]:" + port + want := shadowedError("[::1]:" + port) if err := srv.StartListening(); err == nil || !strings.Contains(err.Error(), want) { t.Errorf("StartListening = %v, want %q", err, want) } } + +// dbgp can listen on its port again at once after it stops listening, even +// while connections it closed itself still linger on the port (TIME_WAIT), +// as after debugging a few requests. +func TestListenAgainAfterClosingConnections(t *testing.T) { + for _, host := range []string{"", "127.0.0.1"} { + l, err := listenTCP(net.JoinHostPort(host, "0")) + if err != nil { + t.Fatal(err) + } + _, port, _ := net.SplitHostPort(l.Addr().String()) + client, err := net.Dial("tcp", net.JoinHostPort("127.0.0.1", port)) + if err != nil { + t.Fatal(err) + } + conn, err := l.Accept() + if err != nil { + t.Fatal(err) + } + _ = conn.Close() // dbgp's side closes first, so it lingers + _, _ = client.Read(make([]byte, 1)) + _ = client.Close() + _ = l.Close() + + l, err = listenTCP(net.JoinHostPort(host, port)) + if err != nil { + t.Fatalf("listening on %q again: %v", host, err) + } + _ = l.Close() + } +} diff --git a/listen_windows.go b/listen_windows.go new file mode 100644 index 0000000..c5b5aa7 --- /dev/null +++ b/listen_windows.go @@ -0,0 +1,22 @@ +//go:build windows + +package dbgp + +import "syscall" + +// soExclusiveAddrUse is Windows' SO_EXCLUSIVEADDRUSE, which syscall lacks. +const soExclusiveAddrUse = ^syscall.SO_REUSEADDR + +// listenControl makes the addresses dbgp listens on its own. Windows +// otherwise lets dbgp listen on every address while another program +// listens on one of them, which then gets the connections to it, and lets +// another program listen on one of dbgp's addresses later. +func listenControl(_, _ string, c syscall.RawConn) error { + var err error + if cerr := c.Control(func(fd uintptr) { + err = syscall.SetsockoptInt(syscall.Handle(fd), syscall.SOL_SOCKET, soExclusiveAddrUse, 1) + }); cerr != nil { + return cerr + } + return err +} diff --git a/pathmap.go b/pathmap.go index 6fbb647..91d2b53 100644 --- a/pathmap.go +++ b/pathmap.go @@ -63,7 +63,7 @@ func (m PathMap) ToRemote(local string) string { } } if best < 0 { - return local + return filepath.ToSlash(local) } return joinPath(m[best].Remote, filepath.ToSlash(rest), "/") } diff --git a/pathmap_test.go b/pathmap_test.go index 6cfd3a9..9421d16 100644 --- a/pathmap_test.go +++ b/pathmap_test.go @@ -50,7 +50,7 @@ func TestPathMapTranslation(t *testing.T) { } // Mappings match whole path components only. - if got := m.ToRemote(filepath.FromSlash("/home/me/project2/a.php")); got != filepath.FromSlash("/home/me/project2/a.php") { + if got := m.ToRemote(filepath.FromSlash("/home/me/project2/a.php")); got != "/home/me/project2/a.php" { t.Errorf("ToRemote matched a partial component: %q", got) } if got := m.ToLocal("/var/www/html2/a.php"); got != filepath.FromSlash("/var/www/html2/a.php") { @@ -282,7 +282,7 @@ func TestAddPathMapping(t *testing.T) { if got := e.LastCommand("breakpoint_set").Args["f"]; got != "file:///home/me/app/basic.php" { t.Fatalf("breakpoint before mapping sent for %q", got) } - if sess.Script() != "/app/basic.php" { + if sess.Script() != filepath.FromSlash("/app/basic.php") { t.Fatalf("Script() before mapping = %q", sess.Script()) } e.ClearReceived() @@ -427,7 +427,7 @@ func TestPathMapLocalCase(t *testing.T) { m := mustPathMap(t, PathMapping{Local: "/Users/me/Project", Remote: "/var/www/html"}) local := filepath.FromSlash("/users/ME/project/src/A.php") - want := local // no mapping matches + want := filepath.ToSlash(local) // no mapping matches if ignoreCase { want = "/var/www/html/src/A.php" // the rest keeps its own case } @@ -436,7 +436,7 @@ func TestPathMapLocalCase(t *testing.T) { } // Whole components only. other := filepath.FromSlash("/users/me/project2/a.php") - if got := m.ToRemote(other); got != other { + if got := m.ToRemote(other); got != filepath.ToSlash(other) { t.Errorf("ignoreCase=%v: ToRemote(%q) = %q, want it unchanged", ignoreCase, other, got) } if got, want := m.ToLocal("/VAR/www/html/a.php"), filepath.FromSlash("/VAR/www/html/a.php"); got != want { diff --git a/protocol_test.go b/protocol_test.go index e727e11..6a35a8a 100644 --- a/protocol_test.go +++ b/protocol_test.go @@ -133,7 +133,7 @@ func TestParseResponseFields(t *testing.T) { } for name, want := range map[string]int{"run_break": lineAddBody, "step_over": lineAddReturn, "run_break_conditional": lineLoopBody} { file, line := parse(name).ParseMessage() - if file != "/app/basic.php" || line != want { + if file != filepath.FromSlash("/app/basic.php") || line != want { t.Errorf("%s: ParseMessage = %s:%d, want /app/basic.php:%d", name, file, line, want) } } @@ -205,7 +205,8 @@ func TestFormatStack(t *testing.T) { {Level: 0, Where: "add", Filename: "file:///app/basic.php", Lineno: 24}, {Level: 1, Where: "{main}", Filename: "file:///app/basic.php", Lineno: 36}, } - want := []string{"#0 add() at /app/basic.php:24", "#1 {main}() at /app/basic.php:36"} + file := filepath.FromSlash("/app/basic.php") + want := []string{"#0 add() at " + file + ":24", "#1 {main}() at " + file + ":36"} if got := FormatStack(frames); !reflect.DeepEqual(got, want) { t.Errorf("FormatStack = %q, want %q", got, want) } @@ -213,7 +214,7 @@ func TestFormatStack(t *testing.T) { func TestFormatStackDecodesURI(t *testing.T) { got := FormatStack([]StackFrame{{Where: "f", Filename: "file:///my%20app/a.php", Lineno: 1}}) - if want := "#0 f() at /my app/a.php:1"; got[0] != want { + if want := "#0 f() at " + filepath.FromSlash("/my app/a.php") + ":1"; got[0] != want { t.Errorf("FormatStack = %q, want %q", got[0], want) } } diff --git a/server.go b/server.go index 611fcea..b62629d 100644 --- a/server.go +++ b/server.go @@ -214,7 +214,7 @@ func (s *Server) StartListening() error { var listeners []net.Listener var failures []string for _, h := range hosts { - listener, err := net.Listen("tcp", net.JoinHostPort(h, port)) + listener, err := listenTCP(net.JoinHostPort(h, port)) if err != nil { failures = append(failures, err.Error()) continue diff --git a/server_test.go b/server_test.go index 3643325..1a7a341 100644 --- a/server_test.go +++ b/server_test.go @@ -934,6 +934,12 @@ func TestServerDropsSilentSetup(t *testing.T) { if !silent.WaitClosed(3 * time.Second) { t.Fatal("a connection that stopped answering during setup was kept") } + // The slot is freed just after the connection is closed. + waitFor(t, func() bool { + srv.mu.Lock() + defer srv.mu.Unlock() + return len(srv.handshakes) == 0 + }) dialFakeEngine(t, srv, fixture(t, "init"), standardHandlers()) sess := waitSession(t, srv) if got := srv.Sessions(); len(got) != 1 || got[0] != sess { diff --git a/session_test.go b/session_test.go index 988c4ef..4d2a10d 100644 --- a/session_test.go +++ b/session_test.go @@ -7,7 +7,9 @@ import ( "errors" "fmt" "io" + "path/filepath" "reflect" + "runtime" "strconv" "strings" "sync" @@ -319,7 +321,7 @@ func TestResponseDecoding(t *testing.T) { if want := []string{"line", "conditional", "call", "exception"}; !reflect.DeepEqual(types, want) { t.Fatalf("types = %v, want %v", types, want) } - if bps[0].ID != 42420001 || bps[0].Filename != "/app/basic.php" || bps[0].Lineno != 24 || bps[0].State != "enabled" { + if bps[0].ID != 42420001 || bps[0].Filename != filepath.FromSlash("/app/basic.php") || bps[0].Lineno != 24 || bps[0].State != "enabled" { t.Errorf("line breakpoint = %+v", bps[0]) } if bps[2].State != "disabled" { @@ -332,8 +334,8 @@ func TestResponseDecoding(t *testing.T) { t.Fatal(err) } want := []StackFrame{ - {Level: 0, Type: "file", Filename: "/app/basic.php", Lineno: 24, Where: "add"}, - {Level: 1, Type: "file", Filename: "/app/basic.php", Lineno: 36, Where: "{main}"}, + {Level: 0, Type: "file", Filename: filepath.FromSlash("/app/basic.php"), Lineno: 24, Where: "add"}, + {Level: 1, Type: "file", Filename: filepath.FromSlash("/app/basic.php"), Lineno: 36, Where: "{main}"}, } if !reflect.DeepEqual(stack, want) { t.Errorf("stack = %+v, want %+v", stack, want) @@ -722,7 +724,7 @@ func TestContinueStopsAtBreak(t *testing.T) { if err != nil { t.Fatal(err) } - want := State{Status: StatusBreak, Reason: "ok", File: "/app/basic.php", Line: lineAddBody} + want := State{Status: StatusBreak, Reason: "ok", File: filepath.FromSlash("/app/basic.php"), Line: lineAddBody} if st != want { t.Errorf("State = %+v, want %+v", st, want) } @@ -944,7 +946,7 @@ func TestNotificationsAreCollected(t *testing.T) { if next != 2 || notes[0].Name != "error" || notes[1].Name != "breakpoint_resolved" { t.Fatalf("Notifications = %+v, %d", notes, next) } - want := Message{Filename: "/app/basic.php", Lineno: lineWarning, Type: "Warning", Text: "Undefined variable $undefinedVariable"} + want := Message{Filename: filepath.FromSlash("/app/basic.php"), Lineno: lineWarning, Type: "Warning", Text: "Undefined variable $undefinedVariable"} if *notes[0].Message != want { t.Errorf("error notification = %+v, want %+v", *notes[0].Message, want) } @@ -1193,6 +1195,11 @@ func TestCommandTimeout(t *testing.T) { // command timeout: the write fails, and the session ends, since the rest of // the command would garble what follows. func TestWriteDeadline(t *testing.T) { + if runtime.GOOS == "windows" { + // Windows takes in a 40 MB command whole, even with a small receive + // buffer, so the write does not block; the command timeout applies. + t.Skip("writes to an engine that stopped reading do not block on Windows") + } srv := newTestServer(t, Config{CommandTimeout: 200 * time.Millisecond}) conn := dialServer(t, srv) init := fixture(t, "init") From 029e0d7d40d922faf347a76a039a66715629d561 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Thu, 22 Oct 2026 18:36:36 +0200 Subject: [PATCH 12/12] Raise go version --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 918dbd0..b1b80e5 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/php-debugger/dbgp-client -go 1.26.9 +go 1.26.10 require ( github.com/ergochat/readline v0.1.3