From 7612ece9db69321663de0708ef9695956a9c44a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 15:56:25 +0000 Subject: [PATCH 01/10] Initial plan From 14b799995094bee64689910bf4f0c80b4eb9dd2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 15:59:12 +0000 Subject: [PATCH 02/10] Import dbgp client package from PR #12 Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- README.md | 2 + client.go | 402 +++++++++++++++++++++++++++++++ go.mod | 3 + parser.go | 179 ++++++++++++++ parser_test.go | 125 ++++++++++ protocol.go | 247 +++++++++++++++++++ real_test.go | 24 ++ testdata/context_get_complex.xml | 29 +++ 8 files changed, 1011 insertions(+) create mode 100644 client.go create mode 100644 go.mod create mode 100644 parser.go create mode 100644 parser_test.go create mode 100644 protocol.go create mode 100644 real_test.go create mode 100644 testdata/context_get_complex.xml diff --git a/README.md b/README.md index f203192..ecc4ace 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,4 @@ # dbgp-client CLI DBGP client + +Code imported from `cli/dbgp` in https://github.com/php-debugger/php-debugger/pull/12 (authored by @Haehnchen). diff --git a/client.go b/client.go new file mode 100644 index 0000000..b65d247 --- /dev/null +++ b/client.go @@ -0,0 +1,402 @@ +package dbgp + +import ( + "bufio" + "encoding/base64" + "fmt" + "io" + "net" + "strconv" + "strings" + "sync" + "time" +) + +// Client manages the DBGp connection +type Client struct { + listener net.Listener + conn net.Conn + reader *bufio.Reader + writer io.Writer + + mu sync.Mutex + transID int + responses map[int]chan *Response + + init *InitPacket + + onBreakpoint func(file string, line int, stack []StackFrame, vars []Variable) +} + +// NewClient creates a new DBGp client (server that accepts PHP connections) +func NewClient(port int) (*Client, error) { + listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + return nil, fmt.Errorf("listen on port %d: %w", port, err) + } + + return &Client{ + listener: listener, + responses: make(map[int]chan *Response), + }, nil +} + +// Addr returns the address the client is listening on +func (c *Client) Addr() string { + return c.listener.Addr().String() +} + +// Port returns the port number +func (c *Client) Port() int { + addr := c.Addr() + _, port, _ := net.SplitHostPort(addr) + p, _ := strconv.Atoi(port) + return p +} + +// WaitForConnection waits for PHP to connect +func (c *Client) WaitForConnection(timeout time.Duration) error { + // Set accept deadline if timeout specified + if timeout > 0 { + tcpListener := c.listener.(*net.TCPListener) + tcpListener.SetDeadline(time.Now().Add(timeout)) + } + + conn, err := c.listener.Accept() + if err != nil { + return fmt.Errorf("accept connection: %w", err) + } + + c.conn = conn + c.reader = bufio.NewReader(conn) + c.writer = conn + + // Start response reader + go c.readLoop() + + // Read init packet + initData, err := c.readPacket() + if err != nil { + return fmt.Errorf("read init packet: %w", err) + } + + c.init, err = ParseInit(initData) + if err != nil { + return fmt.Errorf("parse init packet: %w", err) + } + + return nil +} + +// Init returns the initialization packet from PHP +func (c *Client) Init() *InitPacket { + return c.init +} + +// OnBreakpoint sets the callback for breakpoint hits +func (c *Client) OnBreakpoint(fn func(file string, line int, stack []StackFrame, vars []Variable)) { + c.onBreakpoint = fn +} + +// readPacket reads a single DBGp packet (length + NULL + data) +func (c *Client) readPacket() ([]byte, error) { + // Read length until NULL + line, err := c.reader.ReadString('\x00') + if err != nil { + return nil, err + } + + // Parse length + line = strings.TrimSuffix(line, "\x00") + length, err := strconv.Atoi(line) + if err != nil { + return nil, fmt.Errorf("parse packet length: %w", err) + } + + // Read exact number of bytes + data := make([]byte, length) + _, err = io.ReadFull(c.reader, data) + if err != nil { + return nil, err + } + + // Read trailing NULL + b, err := c.reader.ReadByte() + if err != nil { + return nil, err + } + if b != '\x00' { + return nil, fmt.Errorf("expected NULL terminator, got %x", b) + } + + return data, nil +} + +// readLoop continuously reads responses and dispatches them +func (c *Client) readLoop() { + for { + data, err := c.readPacket() + if err != nil { + // Connection closed + return + } + + resp, err := ParseResponse(data) + if err != nil { + continue + } + + // Check if this is a breakpoint hit (async response) + if resp.Status == StatusBreak && c.onBreakpoint != nil { + go c.handleBreakpoint(resp) + } + + // Dispatch to waiting sender + c.mu.Lock() + ch, ok := c.responses[resp.Transaction] + if ok { + ch <- resp + delete(c.responses, resp.Transaction) + } + c.mu.Unlock() + } +} + +// handleBreakpoint handles async breakpoint notifications +func (c *Client) handleBreakpoint(resp *Response) { + file, line := resp.ParseMessage() + + // Get stack + stack, _ := c.GetStack() + + // Get local variables + vars, _ := c.GetContext(0, 0) + + if c.onBreakpoint != nil { + c.onBreakpoint(file, line, stack, vars) + } +} + +// nextTransID generates a new transaction ID +func (c *Client) nextTransID() int { + c.mu.Lock() + defer c.mu.Unlock() + c.transID++ + return c.transID +} + +// sendCommand sends a command and waits for response +func (c *Client) sendCommand(cmd string) (*Response, error) { + if c.conn == nil { + return nil, fmt.Errorf("not connected") + } + + transID := c.nextTransID() + + // Ensure transaction ID is in command + if !strings.Contains(cmd, "-i") { + cmd = fmt.Sprintf("%s -i %d", cmd, transID) + } else { + // Extract transaction ID from command + parts := strings.Split(cmd, "-i ") + if len(parts) > 1 { + idStr := strings.Fields(parts[1])[0] + id, _ := strconv.Atoi(idStr) + transID = id + } + } + + // Create response channel + ch := make(chan *Response, 1) + c.mu.Lock() + c.responses[transID] = ch + c.mu.Unlock() + + // Send command + packet := fmt.Sprintf("%d\x00%s\x00", len(cmd), cmd) + _, err := c.writer.Write([]byte(packet)) + if err != nil { + c.mu.Lock() + delete(c.responses, transID) + c.mu.Unlock() + return nil, fmt.Errorf("send command: %w", err) + } + + // Wait for response with timeout + select { + case resp := <-ch: + if resp.Error != nil { + return resp, fmt.Errorf("error %d: %s", resp.Error.Code, resp.Error.Message) + } + return resp, nil + case <-time.After(30 * time.Second): + c.mu.Lock() + delete(c.responses, transID) + c.mu.Unlock() + return nil, fmt.Errorf("timeout waiting for response") + } +} + +// SetBreakpoint sets a line breakpoint +func (c *Client) SetBreakpoint(file string, line int) (int, error) { + uri := MakeFileURI(file) + cmd := fmt.Sprintf("breakpoint_set -t line -f %s -n %d", uri, line) + resp, err := c.sendCommand(cmd) + if err != nil { + return 0, err + } + return resp.BreakpointID, nil +} + +// SetConditionalBreakpoint sets a conditional breakpoint +func (c *Client) SetConditionalBreakpoint(file string, line int, condition string) (int, error) { + uri := MakeFileURI(file) + encoded := base64.StdEncoding.EncodeToString([]byte(condition)) + cmd := fmt.Sprintf("breakpoint_set -t conditional -f %s -n %d -- %s", uri, line, encoded) + resp, err := c.sendCommand(cmd) + if err != nil { + return 0, err + } + return resp.BreakpointID, nil +} + +// RemoveBreakpoint removes a breakpoint +func (c *Client) RemoveBreakpoint(id int) error { + cmd := fmt.Sprintf("breakpoint_remove -d %d", id) + _, err := c.sendCommand(cmd) + return err +} + +// ListBreakpoints lists all breakpoints +func (c *Client) ListBreakpoints() ([]BreakpointInfo, error) { + resp, err := c.sendCommand("breakpoint_list") + if err != nil { + return nil, err + } + + // Parse breakpoints from response + var breakpoints []BreakpointInfo + // TODO: Parse from resp.Raw + _ = resp + return breakpoints, nil +} + +// Run starts or continues execution +func (c *Client) Run() error { + _, err := c.sendCommand("run") + return err +} + +// StepInto steps into the next statement +func (c *Client) StepInto() error { + _, err := c.sendCommand("step_into") + return err +} + +// StepOver steps over the next statement +func (c *Client) StepOver() error { + _, err := c.sendCommand("step_over") + return err +} + +// StepOut steps out of the current function +func (c *Client) StepOut() error { + _, err := c.sendCommand("step_out") + return err +} + +// Stop stops execution +func (c *Client) Stop() error { + _, err := c.sendCommand("stop") + return err +} + +// Detach detaches the debugger (script continues) +func (c *Client) Detach() error { + _, err := c.sendCommand("detach") + return err +} + +// GetStack returns the call stack +func (c *Client) GetStack() ([]StackFrame, error) { + resp, err := c.sendCommand("stack_get") + if err != nil { + return nil, err + } + return resp.Stack, nil +} + +// GetContext returns variables at the given depth and context +func (c *Client) GetContext(depth int, context int) ([]Variable, error) { + cmd := fmt.Sprintf("context_get -d %d -c %d", depth, context) + resp, err := c.sendCommand(cmd) + if err != nil { + return nil, err + } + return ParseVariables(resp.Raw), nil +} + +// Eval evaluates an expression +func (c *Client) Eval(code string) (string, error) { + encoded := base64.StdEncoding.EncodeToString([]byte(code)) + cmd := fmt.Sprintf("eval -- %s", encoded) + resp, err := c.sendCommand(cmd) + if err != nil { + return "", err + } + return resp.Raw, nil +} + +// GetSource returns source code +func (c *Client) GetSource(file string, begin, end int) (string, error) { + uri := MakeFileURI(file) + cmd := fmt.Sprintf("source -f %s", uri) + if begin > 0 { + cmd += fmt.Sprintf(" -b %d", begin) + } + if end > 0 { + cmd += fmt.Sprintf(" -e %d", end) + } + resp, err := c.sendCommand(cmd) + if err != nil { + return "", err + } + return resp.Raw, nil +} + +// Status returns current debugger status +func (c *Client) Status() (string, error) { + resp, err := c.sendCommand("status") + if err != nil { + return "", err + } + return resp.Status, nil +} + +// FeatureGet gets a feature value +func (c *Client) FeatureGet(name string) (string, error) { + cmd := fmt.Sprintf("feature_get -n %s", name) + resp, err := c.sendCommand(cmd) + if err != nil { + return "", err + } + return resp.Raw, nil +} + +// FeatureSet sets a feature value +func (c *Client) FeatureSet(name, value string) error { + cmd := fmt.Sprintf("feature_set -n %s -v %s", name, value) + _, err := c.sendCommand(cmd) + return err +} + +// Close closes the connection +func (c *Client) Close() error { + if c.conn != nil { + c.conn.Close() + } + if c.listener != nil { + c.listener.Close() + } + return nil +} diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..c06e81d --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/php-debugger/dbgp-client + +go 1.21 diff --git a/parser.go b/parser.go new file mode 100644 index 0000000..67e984d --- /dev/null +++ b/parser.go @@ -0,0 +1,179 @@ +package dbgp + +import ( + "bytes" + "encoding/base64" + "encoding/xml" + "fmt" + "strings" +) + +// Variable represents a parsed variable +type Variable struct { + Name string + Type string + Value string + Level int // nesting level for indentation +} + +// ContextResponse wraps the XML response for context_get +type ContextResponse struct { + XMLName xml.Name `xml:"response"` + Properties []Property `xml:"property"` +} + +// ParseVariables extracts top-level variables and their immediate children from a DBGp context_get response +func ParseVariables(xmlData string) []Variable { + var resp ContextResponse + decoder := xml.NewDecoder(bytes.NewReader([]byte(xmlData))) + decoder.CharsetReader = charsetReader + + if err := decoder.Decode(&resp); err != nil { + return nil + } + + var vars []Variable + for _, p := range resp.Properties { + // Only include top-level variables (no brackets or arrows in name) + if containsAny(p.FullName, "[", "->") { + continue + } + vars = append(vars, Variable{ + Name: p.FullName, + Type: p.Type, + Value: formatPropertyValue(p), + Level: 0, + }) + // Add immediate child properties for objects/arrays + for _, child := range p.ChildProperties { + vars = append(vars, Variable{ + Name: child.FullName, + Type: child.Type, + Value: formatPropertyValue(child), + Level: 1, + }) + } + } + return vars +} + +// ParseAllVariables extracts all variables including nested ones +func ParseAllVariables(xmlData string) []Variable { + var resp ContextResponse + decoder := xml.NewDecoder(bytes.NewReader([]byte(xmlData))) + decoder.CharsetReader = charsetReader + + if err := decoder.Decode(&resp); err != nil { + return nil + } + + return flattenProperties(resp.Properties) +} + +// flattenProperties recursively flattens nested properties +func flattenProperties(props []Property) []Variable { + var vars []Variable + for _, p := range props { + vars = append(vars, Variable{ + Name: p.FullName, + Type: p.Type, + Value: formatPropertyValue(p), + }) + if len(p.ChildProperties) > 0 { + vars = append(vars, flattenProperties(p.ChildProperties)...) + } + } + return vars +} + +// formatPropertyValue formats a property value for display +func formatPropertyValue(p Property) string { + // Handle types with children (check Children attr, ChildProperties, or raw XML in Value) + hasChildren := p.Children > 0 || len(p.ChildProperties) > 0 || + (len(p.Value) > 0 && p.Value[0] == '<') + + if hasChildren { + count := p.NumChildren + if count == 0 { + count = len(p.ChildProperties) + } + if count == 0 { + count = p.Children // fallback + } + if count == 0 { + count = 1 // at least one if we detected children + } + if p.Type == "object" && p.ClassName != "" { + return p.ClassName + } + return fmt.Sprintf("%s[%d]", p.Type, count) + } + + // Decode base64 if needed + content := p.Value + if p.Encoding == "base64" && content != "" { + if decoded, err := base64.StdEncoding.DecodeString(content); err == nil { + content = string(decoded) + } + } + + return formatSimpleValue(p.Type, content, p.ClassName) +} + +// formatSimpleValue formats a value for display +func formatSimpleValue(typ, content, classname string) string { + switch typ { + case "null", "uninitialized": + return "null" + case "bool": + if content == "1" || content == "true" { + return "true" + } + return "false" + case "string": + if content == "" { + return `""` + } + if len(content) > 60 { + return `"` + content[:55] + `..."` + } + return `"` + content + `"` + case "int", "float": + return content + case "array": + if classname != "" { + return classname + "[]" + } + return "array[]" + case "object": + if classname != "" { + return classname + "{}" + } + return "object{}" + default: + if content == "" { + return "<" + typ + ">" + } + if len(content) > 60 { + return content[:57] + "..." + } + return content + } +} + +// FormatVariable formats a variable for display +func FormatVariable(v Variable) string { + indent := strings.Repeat(" ", v.Level) + width := 30 - (v.Level * 2) + return fmt.Sprintf("%s%-*s %-8s = %s", indent, width, v.Name, v.Type, v.Value) +} + +// containsAny checks if s contains any of the substrings +func containsAny(s string, subs ...string) bool { + for _, sub := range subs { + if bytes.Contains([]byte(s), []byte(sub)) { + return true + } + } + return false +} diff --git a/parser_test.go b/parser_test.go new file mode 100644 index 0000000..bfe8ae1 --- /dev/null +++ b/parser_test.go @@ -0,0 +1,125 @@ +package dbgp + +import ( + "os" + "testing" +) + +func TestFormatSimpleValue(t *testing.T) { + tests := []struct { + typ string + content string + classname string + want string + }{ + {"string", "hello", "", `"hello"`}, + {"string", "", "", `""`}, + {"int", "42", "", "42"}, + {"bool", "1", "", "true"}, + {"bool", "0", "", "false"}, + {"null", "", "", "null"}, + {"array", "", "App\\Foo", "App\\Foo[]"}, + {"array", "", "", "array[]"}, + {"object", "", "App\\Service", "App\\Service{}"}, + } + + for _, tt := range tests { + got := formatSimpleValue(tt.typ, tt.content, tt.classname) + if got != tt.want { + t.Errorf("formatSimpleValue(%q, %q, %q) = %q, want %q", tt.typ, tt.content, tt.classname, got, tt.want) + } + } +} + +func TestFormatPropertyValue(t *testing.T) { + tests := []struct { + name string + prop Property + want string + }{ + { + name: "base64 string", + prop: Property{Type: "string", Encoding: "base64", Value: "SGVsbG8sIFdvcmxkIQ=="}, + want: `"Hello, World!"`, + }, + { + name: "int", + prop: Property{Type: "int", Value: "42"}, + want: "42", + }, + { + name: "bool true", + prop: Property{Type: "bool", Value: "1"}, + want: "true", + }, + { + name: "array with children", + prop: Property{Type: "array", Children: 5}, + want: "array[5]", + }, + { + name: "object with classname", + prop: Property{Type: "object", ClassName: "App\\Service", Children: 2}, + want: "App\\Service", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := formatPropertyValue(tt.prop) + if got != tt.want { + t.Errorf("formatPropertyValue() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestParseVariablesFromRealXML(t *testing.T) { + data, err := os.ReadFile("testdata/context_get_complex.xml") + if err != nil { + t.Fatalf("read test file: %v", err) + } + + vars := ParseVariables(string(data)) + + // Find specific variables + findVar := func(name string) *Variable { + for i := range vars { + if vars[i].Name == name { + return &vars[i] + } + } + return nil + } + + tests := []struct { + name string + wantType string + wantValue string + }{ + {"$count", "string", `"Hello, World!"`}, + {"$name", "string", `"World"`}, + {"$sum", "int", "15"}, + {"$numbers", "array", "array[5]"}, + {"$user", "array", "array[3]"}, + {"$obj", "object", `App\Service\MyService`}, + {"$emptyArray", "array", "array[]"}, + {"$nullVal", "null", "null"}, + {"$boolVal", "bool", "true"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := findVar(tt.name) + if v == nil { + t.Fatalf("variable %s not found", tt.name) + } + if v.Type != tt.wantType { + t.Errorf("type = %q, want %q", v.Type, tt.wantType) + } + if v.Value != tt.wantValue { + t.Errorf("value = %q, want %q", v.Value, tt.wantValue) + } + }) + } +} diff --git a/protocol.go b/protocol.go new file mode 100644 index 0000000..05e078a --- /dev/null +++ b/protocol.go @@ -0,0 +1,247 @@ +// Package dbgp implements the DBGp protocol for PHP debugging +package dbgp + +import ( + "bytes" + "encoding/xml" + "fmt" + "io" + "strconv" + "strings" +) + +// charsetReader handles non-UTF-8 XML encodings by treating them as UTF-8 +func charsetReader(charset string, input io.Reader) (io.Reader, error) { + // Treat all encodings as UTF-8 (DBGp typically uses ASCII-safe content anyway) + if strings.EqualFold(charset, "iso-8859-1") || + strings.EqualFold(charset, "windows-1252") || + strings.EqualFold(charset, "utf-8") || + strings.EqualFold(charset, "us-ascii") { + return input, nil + } + return nil, fmt.Errorf("unsupported charset: %s", charset) +} + +// Status values +const ( + StatusStarting = "starting" + StatusStopping = "stopping" + StatusStopped = "stopped" + StatusRunning = "running" + StatusBreak = "break" + StatusDetached = "detached" +) + +// Breakpoint types +const ( + BreakpointLine = "line" + BreakpointConditional = "conditional" + BreakpointCall = "call" + BreakpointReturn = "return" + BreakpointException = "exception" + BreakpointWatch = "watch" +) + +// InitPacket is sent by PHP when connection is established +type InitPacket struct { + XMLName xml.Name `xml:"init"` + AppID string `xml:"appid,attr"` + IDEKey string `xml:"idekey,attr"` + Session string `xml:"session,attr"` + Thread string `xml:"thread,attr"` + Parent string `xml:"parent,attr"` + Language string `xml:"language,attr"` + Protocol string `xml:"protocol_version,attr"` + FileURI string `xml:"fileuri"` + EngineVersion string `xml:"engine>version"` +} + +// Response is the generic DBGp response +type Response struct { + XMLName xml.Name `xml:"response"` + Command string `xml:"command,attr"` + Transaction int `xml:"transaction_id,attr"` + Status string `xml:"status,attr,omitempty"` + Reason string `xml:"reason,attr,omitempty"` + Success string `xml:"success,attr,omitempty"` + BreakpointID int `xml:"id,attr,omitempty"` + + // For breakpoint_set + Breakpoint *BreakpointInfo `xml:"breakpoint,omitempty"` + + // For context_get + Context int `xml:"context,attr,omitempty"` + Properties []Property `xml:"property,omitempty"` + + // For stack_get + Stack []StackFrame `xml:"stack,omitempty"` + + // For errors + Error *Error `xml:"error,omitempty"` + + // Message for breakpoint hit + Message *Message `xml:"xdebug\\:message,omitempty"` + + // Raw for debugging + Raw string `xml:",innerxml"` +} + +// Error represents a DBGp error +type Error struct { + Code int `xml:"code,attr"` + Message string `xml:"message"` +} + +// Message contains breakpoint hit information +type Message struct { + Filename string `xml:"filename,attr"` + Lineno int `xml:"lineno,attr"` +} + +// BreakpointInfo contains breakpoint details +type BreakpointInfo struct { + ID int `xml:"id,attr"` + Type string `xml:"type,attr"` + Filename string `xml:"filename,attr"` + Lineno int `xml:"lineno,attr"` + State string `xml:"state,attr"` + Exception string `xml:"exception,attr,omitempty"` + Expression string `xml:"expression,attr,omitempty"` + HitCount int `xml:"hit_count,attr,omitempty"` + HitValue int `xml:"hit_value,attr,omitempty"` + Temporary int `xml:"temporary,attr,omitempty"` +} + +// Property represents a variable +type Property struct { + Name string `xml:"name,attr"` + FullName string `xml:"fullname,attr"` + Type string `xml:"type,attr"` + ClassName string `xml:"classname,attr,omitempty"` + Facet string `xml:"facet,attr,omitempty"` + Size int `xml:"size,attr,omitempty"` + Children int `xml:"children,attr,omitempty"` + NumChildren int `xml:"numchildren,attr,omitempty"` + Encoding string `xml:"encoding,attr,omitempty"` + Value string `xml:",chardata"` + ChildProperties []Property `xml:"property,omitempty"` +} + +// StackFrame represents a call stack entry +type StackFrame struct { + Level int `xml:"level,attr"` + Type string `xml:"type,attr"` + Filename string `xml:"filename,attr"` + Lineno int `xml:"lineno,attr"` + Where string `xml:"where,attr"` + Cmmd string `xml:"cmmd,attr,omitempty"` +} + +// ParseInit parses the init packet from PHP +func ParseInit(data []byte) (*InitPacket, error) { + var init InitPacket + decoder := xml.NewDecoder(bytes.NewReader(data)) + decoder.CharsetReader = charsetReader + if err := decoder.Decode(&init); err != nil { + return nil, fmt.Errorf("parse init: %w", err) + } + return &init, nil +} + +// ParseResponse parses a DBGp response +func ParseResponse(data []byte) (*Response, error) { + var resp Response + decoder := xml.NewDecoder(bytes.NewReader(data)) + decoder.CharsetReader = charsetReader + if err := decoder.Decode(&resp); err != nil { + return nil, fmt.Errorf("parse response: %w", err) + } + return &resp, nil +} + +// ParseMessage extracts breakpoint hit info from response +func (r *Response) ParseMessage() (file string, line int) { + if r.Message != nil { + file = strings.TrimPrefix(r.Message.Filename, "file://") + line = r.Message.Lineno + } + return +} + +func formatValue(p Property) string { + if p.Encoding == "base64" { + return "" + } + + if p.Type == "array" || p.Type == "object" { + if p.Children > 0 { + return fmt.Sprintf("%s(%d)", p.Type, p.Children) + } + return p.Type + "(0)" + } + + if p.Value == "" { + switch p.Type { + case "null": + return "null" + case "bool": + return "false" + case "string": + return `""` + default: + return p.Type + } + } + + // Truncate long values + if len(p.Value) > 100 { + return p.Value[:97] + "..." + } + + return p.Value +} + +// FormatStack formats the call stack for display +func FormatStack(frames []StackFrame) []string { + lines := make([]string, len(frames)) + for i, f := range frames { + file := strings.TrimPrefix(f.Filename, "file://") + lines[i] = fmt.Sprintf("#%d %s() at %s:%d", f.Level, f.Where, file, f.Lineno) + } + return lines +} + +// FormatFileURI converts file:// URI to path +func FormatFileURI(uri string) string { + return strings.TrimPrefix(uri, "file://") +} + +// MakeFileURI converts path to file:// URI +func MakeFileURI(path string) string { + if strings.HasPrefix(path, "file://") { + return path + } + return "file://" + path +} + +// ParseBreakpointSpec parses "file.php:42" or "file.php:42,55,60" +func ParseBreakpointSpec(spec string) (file string, lines []int, err error) { + parts := strings.Split(spec, ":") + if len(parts) != 2 { + return "", nil, fmt.Errorf("invalid breakpoint spec: %s (expected file:line)", spec) + } + + file = parts[0] + lineStrs := strings.Split(parts[1], ",") + lines = make([]int, 0, len(lineStrs)) + + for _, ls := range lineStrs { + l, err := strconv.Atoi(strings.TrimSpace(ls)) + if err != nil { + return "", nil, fmt.Errorf("invalid line number: %s", ls) + } + lines = append(lines, l) + } + + return file, lines, nil +} diff --git a/real_test.go b/real_test.go new file mode 100644 index 0000000..4974b22 --- /dev/null +++ b/real_test.go @@ -0,0 +1,24 @@ +package dbgp + +import ( + "fmt" + "testing" +) + +func TestParseRealXML(t *testing.T) { + xml := `` + + vars := ParseVariables(xml) + fmt.Printf("Found %d variables:\n", len(vars)) + for i, v := range vars { + fmt.Printf("[%d] %s (%s) = %q\n", i, v.Name, v.Type, v.Value) + } + + if len(vars) != 3 { + t.Fatalf("expected 3 variables, got %d", len(vars)) + } + + if vars[0].Value != `"Hello, World!"` { + t.Errorf("$count: expected %q, got %q", `"Hello, World!"`, vars[0].Value) + } +} diff --git a/testdata/context_get_complex.xml b/testdata/context_get_complex.xml new file mode 100644 index 0000000..ca1fdc0 --- /dev/null +++ b/testdata/context_get_complex.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 76cae3edd8345bcf79020046dff5668540b6b667 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:01:00 +0000 Subject: [PATCH 03/10] Fix imported DBGP client parsing and protocol issues Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 32 ++++++++++++++++++++----------- parser.go | 17 ++++++++++------- parser_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++-- protocol.go | 12 ++++++++---- 4 files changed, 88 insertions(+), 24 deletions(-) diff --git a/client.go b/client.go index b65d247..7ecc16a 100644 --- a/client.go +++ b/client.go @@ -2,7 +2,9 @@ package dbgp import ( "bufio" + "bytes" "encoding/base64" + "encoding/xml" "fmt" "io" "net" @@ -58,8 +60,11 @@ func (c *Client) Port() int { func (c *Client) WaitForConnection(timeout time.Duration) error { // Set accept deadline if timeout specified if timeout > 0 { - tcpListener := c.listener.(*net.TCPListener) - tcpListener.SetDeadline(time.Now().Add(timeout)) + if deadlineListener, ok := c.listener.(interface{ SetDeadline(time.Time) error }); ok { + if err := deadlineListener.SetDeadline(time.Now().Add(timeout)); err != nil { + return fmt.Errorf("set accept deadline: %w", err) + } + } } conn, err := c.listener.Accept() @@ -71,9 +76,6 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { c.reader = bufio.NewReader(conn) c.writer = conn - // Start response reader - go c.readLoop() - // Read init packet initData, err := c.readPacket() if err != nil { @@ -85,6 +87,9 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { return fmt.Errorf("parse init packet: %w", err) } + // Start response reader + go c.readLoop() + return nil } @@ -274,11 +279,16 @@ func (c *Client) ListBreakpoints() ([]BreakpointInfo, error) { return nil, err } - // Parse breakpoints from response - var breakpoints []BreakpointInfo - // TODO: Parse from resp.Raw - _ = resp - return breakpoints, nil + var parsed struct { + Breakpoints []BreakpointInfo `xml:"breakpoint"` + } + decoder := xml.NewDecoder(bytes.NewReader([]byte("" + resp.Raw + ""))) + decoder.CharsetReader = charsetReader + if err := decoder.Decode(&parsed); err != nil { + return nil, fmt.Errorf("parse breakpoint list: %w", err) + } + + return parsed.Breakpoints, nil } // Run starts or continues execution @@ -333,7 +343,7 @@ func (c *Client) GetContext(depth int, context int) ([]Variable, error) { if err != nil { return nil, err } - return ParseVariables(resp.Raw), nil + return ParseVariablesFromProperties(resp.Properties), nil } // Eval evaluates an expression diff --git a/parser.go b/parser.go index 67e984d..2827067 100644 --- a/parser.go +++ b/parser.go @@ -32,8 +32,13 @@ func ParseVariables(xmlData string) []Variable { return nil } + return ParseVariablesFromProperties(resp.Properties) +} + +// ParseVariablesFromProperties extracts top-level variables and their immediate children from parsed properties +func ParseVariablesFromProperties(properties []Property) []Variable { var vars []Variable - for _, p := range resp.Properties { + for _, p := range properties { // Only include top-level variables (no brackets or arrows in name) if containsAny(p.FullName, "[", "->") { continue @@ -67,20 +72,21 @@ func ParseAllVariables(xmlData string) []Variable { return nil } - return flattenProperties(resp.Properties) + return flattenProperties(resp.Properties, 0) } // flattenProperties recursively flattens nested properties -func flattenProperties(props []Property) []Variable { +func flattenProperties(props []Property, level int) []Variable { var vars []Variable for _, p := range props { vars = append(vars, Variable{ Name: p.FullName, Type: p.Type, Value: formatPropertyValue(p), + Level: level, }) if len(p.ChildProperties) > 0 { - vars = append(vars, flattenProperties(p.ChildProperties)...) + vars = append(vars, flattenProperties(p.ChildProperties, level+1)...) } } return vars @@ -141,9 +147,6 @@ func formatSimpleValue(typ, content, classname string) string { case "int", "float": return content case "array": - if classname != "" { - return classname + "[]" - } return "array[]" case "object": if classname != "" { diff --git a/parser_test.go b/parser_test.go index bfe8ae1..2bcddb2 100644 --- a/parser_test.go +++ b/parser_test.go @@ -18,7 +18,7 @@ func TestFormatSimpleValue(t *testing.T) { {"bool", "1", "", "true"}, {"bool", "0", "", "false"}, {"null", "", "", "null"}, - {"array", "", "App\\Foo", "App\\Foo[]"}, + {"array", "", "App\\Foo", "array[]"}, {"array", "", "", "array[]"}, {"object", "", "App\\Service", "App\\Service{}"}, } @@ -82,7 +82,6 @@ func TestParseVariablesFromRealXML(t *testing.T) { vars := ParseVariables(string(data)) - // Find specific variables findVar := func(name string) *Variable { for i := range vars { if vars[i].Name == name { @@ -123,3 +122,51 @@ func TestParseVariablesFromRealXML(t *testing.T) { }) } } + +func TestParseAllVariablesFromRealXML(t *testing.T) { + data, err := os.ReadFile("testdata/context_get_complex.xml") + if err != nil { + t.Fatalf("read test file: %v", err) + } + + vars := ParseAllVariables(string(data)) + + findVar := func(name string) *Variable { + for i := range vars { + if vars[i].Name == name { + return &vars[i] + } + } + return nil + } + + tests := []struct { + name string + wantType string + wantValue string + wantLevel int + }{ + {"$obj", "object", `App\Service\MyService`, 0}, + {"$obj->cache", "object", `App\Cache`, 1}, + {"$obj->cache->items", "array", "array[3]", 2}, + {"$obj->cache->items[0]", "string", `"item1"`, 3}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := findVar(tt.name) + if v == nil { + t.Fatalf("variable %s not found", tt.name) + } + if v.Type != tt.wantType { + t.Errorf("type = %q, want %q", v.Type, tt.wantType) + } + if v.Value != tt.wantValue { + t.Errorf("value = %q, want %q", v.Value, tt.wantValue) + } + if v.Level != tt.wantLevel { + t.Errorf("level = %d, want %d", v.Level, tt.wantLevel) + } + }) + } +} diff --git a/protocol.go b/protocol.go index 05e078a..1a5610c 100644 --- a/protocol.go +++ b/protocol.go @@ -52,7 +52,7 @@ type InitPacket struct { Parent string `xml:"parent,attr"` Language string `xml:"language,attr"` Protocol string `xml:"protocol_version,attr"` - FileURI string `xml:"fileuri"` + FileURI string `xml:"fileuri,attr"` EngineVersion string `xml:"engine>version"` } @@ -80,7 +80,7 @@ type Response struct { Error *Error `xml:"error,omitempty"` // Message for breakpoint hit - Message *Message `xml:"xdebug\\:message,omitempty"` + Message *Message `xml:"https://xdebug.org/dbgp/xdebug message,omitempty"` // Raw for debugging Raw string `xml:",innerxml"` @@ -174,8 +174,12 @@ func formatValue(p Property) string { } if p.Type == "array" || p.Type == "object" { - if p.Children > 0 { - return fmt.Sprintf("%s(%d)", p.Type, p.Children) + count := p.NumChildren + if count == 0 { + count = p.Children + } + if count > 0 { + return fmt.Sprintf("%s(%d)", p.Type, count) } return p.Type + "(0)" } From 61e75794f43006d3f660fc0d2a8001e36497359f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:01:44 +0000 Subject: [PATCH 04/10] Address review findings in imported dbgp code Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 24 ++++++++++++++++-------- protocol.go | 8 ++++---- real_test.go | 5 ----- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/client.go b/client.go index 7ecc16a..9424083 100644 --- a/client.go +++ b/client.go @@ -199,17 +199,25 @@ func (c *Client) sendCommand(cmd string) (*Response, error) { transID := c.nextTransID() // Ensure transaction ID is in command - if !strings.Contains(cmd, "-i") { - cmd = fmt.Sprintf("%s -i %d", cmd, transID) - } else { - // Extract transaction ID from command - parts := strings.Split(cmd, "-i ") - if len(parts) > 1 { - idStr := strings.Fields(parts[1])[0] - id, _ := strconv.Atoi(idStr) + fields := strings.Fields(cmd) + hasTransID := false + for i := 0; i < len(fields); i++ { + if fields[i] == "-i" { + if i+1 >= len(fields) { + return nil, fmt.Errorf("invalid command: missing transaction id after -i") + } + id, err := strconv.Atoi(fields[i+1]) + if err != nil { + return nil, fmt.Errorf("invalid transaction id %q: %w", fields[i+1], err) + } transID = id + hasTransID = true + break } } + if !hasTransID { + cmd = fmt.Sprintf("%s -i %d", cmd, transID) + } // Create response channel ch := make(chan *Response, 1) diff --git a/protocol.go b/protocol.go index 1a5610c..3f70139 100644 --- a/protocol.go +++ b/protocol.go @@ -230,13 +230,13 @@ func MakeFileURI(path string) string { // ParseBreakpointSpec parses "file.php:42" or "file.php:42,55,60" func ParseBreakpointSpec(spec string) (file string, lines []int, err error) { - parts := strings.Split(spec, ":") - if len(parts) != 2 { + sep := strings.LastIndex(spec, ":") + if sep <= 0 || sep == len(spec)-1 { return "", nil, fmt.Errorf("invalid breakpoint spec: %s (expected file:line)", spec) } - file = parts[0] - lineStrs := strings.Split(parts[1], ",") + file = spec[:sep] + lineStrs := strings.Split(spec[sep+1:], ",") lines = make([]int, 0, len(lineStrs)) for _, ls := range lineStrs { diff --git a/real_test.go b/real_test.go index 4974b22..0976cf7 100644 --- a/real_test.go +++ b/real_test.go @@ -1,7 +1,6 @@ package dbgp import ( - "fmt" "testing" ) @@ -9,10 +8,6 @@ func TestParseRealXML(t *testing.T) { xml := `` vars := ParseVariables(xml) - fmt.Printf("Found %d variables:\n", len(vars)) - for i, v := range vars { - fmt.Printf("[%d] %s (%s) = %q\n", i, v.Name, v.Type, v.Value) - } if len(vars) != 3 { t.Fatalf("expected 3 variables, got %d", len(vars)) From 80c2bcac446159543109b93c00f1a2304fd6a2fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:04:00 +0000 Subject: [PATCH 05/10] Harden protocol parsing and command handling Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 30 ++++++++++-- protocol.go | 130 +++++++++++++++++++++++++++++++++++----------------- 2 files changed, 113 insertions(+), 47 deletions(-) diff --git a/client.go b/client.go index 9424083..6584b8d 100644 --- a/client.go +++ b/client.go @@ -5,6 +5,7 @@ import ( "bytes" "encoding/base64" "encoding/xml" + "errors" "fmt" "io" "net" @@ -142,7 +143,18 @@ func (c *Client) readLoop() { for { data, err := c.readPacket() if err != nil { - // Connection closed + c.mu.Lock() + for id, ch := range c.responses { + ch <- &Response{ + Transaction: id, + Error: &Error{ + Code: -1, + Message: "connection closed", + }, + } + delete(c.responses, id) + } + c.mu.Unlock() return } @@ -393,6 +405,9 @@ func (c *Client) Status() (string, error) { // FeatureGet gets a feature value func (c *Client) FeatureGet(name string) (string, error) { + if strings.ContainsAny(name, " \t\r\n") { + return "", fmt.Errorf("feature name must not contain whitespace") + } cmd := fmt.Sprintf("feature_get -n %s", name) resp, err := c.sendCommand(cmd) if err != nil { @@ -403,6 +418,12 @@ func (c *Client) FeatureGet(name string) (string, error) { // FeatureSet sets a feature value func (c *Client) FeatureSet(name, value string) error { + if strings.ContainsAny(name, " \t\r\n") { + return fmt.Errorf("feature name must not contain whitespace") + } + if strings.ContainsAny(value, " \t\r\n") { + return fmt.Errorf("feature value must not contain whitespace") + } cmd := fmt.Sprintf("feature_set -n %s -v %s", name, value) _, err := c.sendCommand(cmd) return err @@ -410,11 +431,12 @@ func (c *Client) FeatureSet(name, value string) error { // Close closes the connection func (c *Client) Close() error { + var closeErr error if c.conn != nil { - c.conn.Close() + closeErr = errors.Join(closeErr, c.conn.Close()) } if c.listener != nil { - c.listener.Close() + closeErr = errors.Join(closeErr, c.listener.Close()) } - return nil + return closeErr } diff --git a/protocol.go b/protocol.go index 3f70139..8c94e85 100644 --- a/protocol.go +++ b/protocol.go @@ -6,30 +6,61 @@ import ( "encoding/xml" "fmt" "io" + "net/url" + "path/filepath" "strconv" "strings" ) -// charsetReader handles non-UTF-8 XML encodings by treating them as UTF-8 -func charsetReader(charset string, input io.Reader) (io.Reader, error) { - // Treat all encodings as UTF-8 (DBGp typically uses ASCII-safe content anyway) - if strings.EqualFold(charset, "iso-8859-1") || - strings.EqualFold(charset, "windows-1252") || - strings.EqualFold(charset, "utf-8") || - strings.EqualFold(charset, "us-ascii") { +// charsetReader handles non-UTF-8 XML encodings +func charsetReader(encoding string, input io.Reader) (io.Reader, error) { + switch strings.ToLower(encoding) { + case "utf-8", "us-ascii": return input, nil + case "iso-8859-1", "latin1": + return decodeSingleByte(input, nil) + case "windows-1252": + return decodeSingleByte(input, windows1252Overrides) + default: + return nil, fmt.Errorf("unsupported charset: %s", encoding) } - return nil, fmt.Errorf("unsupported charset: %s", charset) +} + +func decodeSingleByte(input io.Reader, overrides map[byte]rune) (io.Reader, error) { + data, err := io.ReadAll(input) + if err != nil { + return nil, err + } + + runes := make([]rune, len(data)) + for i, b := range data { + if overrides != nil { + if mapped, ok := overrides[b]; ok { + runes[i] = mapped + continue + } + } + runes[i] = rune(b) + } + + return strings.NewReader(string(runes)), nil +} + +var windows1252Overrides = map[byte]rune{ + 0x80: '€', 0x82: '‚', 0x83: 'ƒ', 0x84: '„', 0x85: '…', 0x86: '†', 0x87: '‡', 0x88: 'ˆ', + 0x89: '‰', 0x8A: 'Š', 0x8B: '‹', 0x8C: 'Œ', 0x8E: 'Ž', 0x91: '‘', 0x92: '’', 0x93: '“', + 0x94: '”', 0x95: '•', 0x96: '–', 0x97: '—', 0x98: '˜', 0x99: '™', 0x9A: 'š', 0x9B: '›', + 0x9C: 'œ', 0x9E: 'ž', 0x9F: 'Ÿ', } // Status values const ( - StatusStarting = "starting" - StatusStopping = "stopping" - StatusStopped = "stopped" - StatusRunning = "running" - StatusBreak = "break" - StatusDetached = "detached" + StatusStarting = "starting" + StatusStopping = "stopping" + StatusStopped = "stopped" + StatusRunning = "running" + StatusBreak = "break" + StatusDetached = "detached" ) // Breakpoint types @@ -100,41 +131,41 @@ type Message struct { // BreakpointInfo contains breakpoint details type BreakpointInfo struct { - ID int `xml:"id,attr"` - Type string `xml:"type,attr"` - Filename string `xml:"filename,attr"` - Lineno int `xml:"lineno,attr"` - State string `xml:"state,attr"` - Exception string `xml:"exception,attr,omitempty"` - Expression string `xml:"expression,attr,omitempty"` - HitCount int `xml:"hit_count,attr,omitempty"` - HitValue int `xml:"hit_value,attr,omitempty"` - Temporary int `xml:"temporary,attr,omitempty"` + ID int `xml:"id,attr"` + Type string `xml:"type,attr"` + Filename string `xml:"filename,attr"` + Lineno int `xml:"lineno,attr"` + State string `xml:"state,attr"` + Exception string `xml:"exception,attr,omitempty"` + Expression string `xml:"expression,attr,omitempty"` + HitCount int `xml:"hit_count,attr,omitempty"` + HitValue int `xml:"hit_value,attr,omitempty"` + Temporary int `xml:"temporary,attr,omitempty"` } // Property represents a variable type Property struct { - Name string `xml:"name,attr"` - FullName string `xml:"fullname,attr"` - Type string `xml:"type,attr"` - ClassName string `xml:"classname,attr,omitempty"` - Facet string `xml:"facet,attr,omitempty"` - Size int `xml:"size,attr,omitempty"` - Children int `xml:"children,attr,omitempty"` - NumChildren int `xml:"numchildren,attr,omitempty"` - Encoding string `xml:"encoding,attr,omitempty"` - Value string `xml:",chardata"` - ChildProperties []Property `xml:"property,omitempty"` + Name string `xml:"name,attr"` + FullName string `xml:"fullname,attr"` + Type string `xml:"type,attr"` + ClassName string `xml:"classname,attr,omitempty"` + Facet string `xml:"facet,attr,omitempty"` + Size int `xml:"size,attr,omitempty"` + Children int `xml:"children,attr,omitempty"` + NumChildren int `xml:"numchildren,attr,omitempty"` + Encoding string `xml:"encoding,attr,omitempty"` + Value string `xml:",chardata"` + ChildProperties []Property `xml:"property,omitempty"` } // StackFrame represents a call stack entry type StackFrame struct { - Level int `xml:"level,attr"` - Type string `xml:"type,attr"` - Filename string `xml:"filename,attr"` - Lineno int `xml:"lineno,attr"` - Where string `xml:"where,attr"` - Cmmd string `xml:"cmmd,attr,omitempty"` + Level int `xml:"level,attr"` + Type string `xml:"type,attr"` + Filename string `xml:"filename,attr"` + Lineno int `xml:"lineno,attr"` + Where string `xml:"where,attr"` + Cmmd string `xml:"cmmd,attr,omitempty"` } // ParseInit parses the init packet from PHP @@ -225,7 +256,13 @@ func MakeFileURI(path string) string { if strings.HasPrefix(path, "file://") { return path } - return "file://" + path + + slashed := filepath.ToSlash(path) + if strings.HasPrefix(slashed, "/") { + return (&url.URL{Scheme: "file", Path: slashed}).String() + } + + return (&url.URL{Scheme: "file", Path: "/" + slashed}).String() } // ParseBreakpointSpec parses "file.php:42" or "file.php:42,55,60" @@ -236,7 +273,14 @@ func ParseBreakpointSpec(spec string) (file string, lines []int, err error) { } file = spec[:sep] - lineStrs := strings.Split(spec[sep+1:], ",") + linePart := spec[sep+1:] + for _, r := range linePart { + if (r < '0' || r > '9') && r != ',' { + return "", nil, fmt.Errorf("invalid breakpoint spec: %s (expected file:line)", spec) + } + } + + lineStrs := strings.Split(linePart, ",") lines = make([]int, 0, len(lineStrs)) for _, ls := range lineStrs { From 07106f8bbc41691012574685c1af5778d7e961aa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:05:15 +0000 Subject: [PATCH 06/10] Improve URI handling and concurrency safety Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 42 ++++++++++++++++++++++++++++++++++-------- parser.go | 14 -------------- parser_test.go | 24 +++++++++++++++--------- protocol.go | 18 ++++++++++++++++-- 4 files changed, 65 insertions(+), 33 deletions(-) diff --git a/client.go b/client.go index 6584b8d..de634fe 100644 --- a/client.go +++ b/client.go @@ -23,6 +23,7 @@ type Client struct { writer io.Writer mu sync.Mutex + writeMu sync.Mutex transID int responses map[int]chan *Response @@ -73,9 +74,11 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { return fmt.Errorf("accept connection: %w", err) } + c.mu.Lock() c.conn = conn c.reader = bufio.NewReader(conn) c.writer = conn + c.mu.Unlock() // Read init packet initData, err := c.readPacket() @@ -83,10 +86,13 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { return fmt.Errorf("read init packet: %w", err) } - c.init, err = ParseInit(initData) + initPacket, err := ParseInit(initData) if err != nil { return fmt.Errorf("parse init packet: %w", err) } + c.mu.Lock() + c.init = initPacket + c.mu.Unlock() // Start response reader go c.readLoop() @@ -96,11 +102,15 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { // Init returns the initialization packet from PHP func (c *Client) Init() *InitPacket { + c.mu.Lock() + defer c.mu.Unlock() return c.init } // OnBreakpoint sets the callback for breakpoint hits func (c *Client) OnBreakpoint(fn func(file string, line int, stack []StackFrame, vars []Variable)) { + c.mu.Lock() + defer c.mu.Unlock() c.onBreakpoint = fn } @@ -164,7 +174,10 @@ func (c *Client) readLoop() { } // Check if this is a breakpoint hit (async response) - if resp.Status == StatusBreak && c.onBreakpoint != nil { + c.mu.Lock() + onBreakpoint := c.onBreakpoint + c.mu.Unlock() + if resp.Status == StatusBreak && onBreakpoint != nil { go c.handleBreakpoint(resp) } @@ -189,8 +202,11 @@ func (c *Client) handleBreakpoint(resp *Response) { // Get local variables vars, _ := c.GetContext(0, 0) - if c.onBreakpoint != nil { - c.onBreakpoint(file, line, stack, vars) + c.mu.Lock() + onBreakpoint := c.onBreakpoint + c.mu.Unlock() + if onBreakpoint != nil { + onBreakpoint(file, line, stack, vars) } } @@ -239,7 +255,9 @@ func (c *Client) sendCommand(cmd string) (*Response, error) { // Send command packet := fmt.Sprintf("%d\x00%s\x00", len(cmd), cmd) + c.writeMu.Lock() _, err := c.writer.Write([]byte(packet)) + c.writeMu.Unlock() if err != nil { c.mu.Lock() delete(c.responses, transID) @@ -432,11 +450,19 @@ func (c *Client) FeatureSet(name, value string) error { // Close closes the connection func (c *Client) Close() error { var closeErr error - if c.conn != nil { - closeErr = errors.Join(closeErr, c.conn.Close()) + c.mu.Lock() + conn := c.conn + listener := c.listener + c.conn = nil + c.reader = nil + c.writer = nil + c.mu.Unlock() + + if conn != nil { + closeErr = errors.Join(closeErr, conn.Close()) } - if c.listener != nil { - closeErr = errors.Join(closeErr, c.listener.Close()) + if listener != nil { + closeErr = errors.Join(closeErr, listener.Close()) } return closeErr } diff --git a/parser.go b/parser.go index 2827067..63f17ea 100644 --- a/parser.go +++ b/parser.go @@ -39,10 +39,6 @@ func ParseVariables(xmlData string) []Variable { func ParseVariablesFromProperties(properties []Property) []Variable { var vars []Variable for _, p := range properties { - // Only include top-level variables (no brackets or arrows in name) - if containsAny(p.FullName, "[", "->") { - continue - } vars = append(vars, Variable{ Name: p.FullName, Type: p.Type, @@ -170,13 +166,3 @@ func FormatVariable(v Variable) string { width := 30 - (v.Level * 2) return fmt.Sprintf("%s%-*s %-8s = %s", indent, width, v.Name, v.Type, v.Value) } - -// containsAny checks if s contains any of the substrings -func containsAny(s string, subs ...string) bool { - for _, sub := range subs { - if bytes.Contains([]byte(s), []byte(sub)) { - return true - } - } - return false -} diff --git a/parser_test.go b/parser_test.go index 2bcddb2..cda8607 100644 --- a/parser_test.go +++ b/parser_test.go @@ -95,16 +95,19 @@ func TestParseVariablesFromRealXML(t *testing.T) { name string wantType string wantValue string + wantLevel int }{ - {"$count", "string", `"Hello, World!"`}, - {"$name", "string", `"World"`}, - {"$sum", "int", "15"}, - {"$numbers", "array", "array[5]"}, - {"$user", "array", "array[3]"}, - {"$obj", "object", `App\Service\MyService`}, - {"$emptyArray", "array", "array[]"}, - {"$nullVal", "null", "null"}, - {"$boolVal", "bool", "true"}, + {"$count", "string", `"Hello, World!"`, 0}, + {"$name", "string", `"World"`, 0}, + {"$sum", "int", "15", 0}, + {"$numbers", "array", "array[5]", 0}, + {"$numbers[0]", "int", "1", 1}, + {"$user", "array", "array[3]", 0}, + {"$obj", "object", `App\Service\MyService`, 0}, + {"$obj->id", "int", "42", 1}, + {"$emptyArray", "array", "array[]", 0}, + {"$nullVal", "null", "null", 0}, + {"$boolVal", "bool", "true", 0}, } for _, tt := range tests { @@ -119,6 +122,9 @@ func TestParseVariablesFromRealXML(t *testing.T) { if v.Value != tt.wantValue { t.Errorf("value = %q, want %q", v.Value, tt.wantValue) } + if v.Level != tt.wantLevel { + t.Errorf("level = %d, want %d", v.Level, tt.wantLevel) + } }) } } diff --git a/protocol.go b/protocol.go index 8c94e85..939bde7 100644 --- a/protocol.go +++ b/protocol.go @@ -193,7 +193,7 @@ func ParseResponse(data []byte) (*Response, error) { // ParseMessage extracts breakpoint hit info from response func (r *Response) ParseMessage() (file string, line int) { if r.Message != nil { - file = strings.TrimPrefix(r.Message.Filename, "file://") + file = FormatFileURI(r.Message.Filename) line = r.Message.Lineno } return @@ -248,7 +248,21 @@ func FormatStack(frames []StackFrame) []string { // FormatFileURI converts file:// URI to path func FormatFileURI(uri string) string { - return strings.TrimPrefix(uri, "file://") + u, err := url.Parse(uri) + if err != nil || u.Scheme != "file" { + return strings.TrimPrefix(uri, "file://") + } + + path, err := url.PathUnescape(u.Path) + if err != nil { + path = u.Path + } + + if len(path) >= 3 && path[0] == '/' && path[2] == ':' { + path = path[1:] + } + + return filepath.FromSlash(path) } // MakeFileURI converts path to file:// URI From 9f3428cd9d753b72949a7ebdfde42d049ccbc0f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:06:24 +0000 Subject: [PATCH 07/10] Decode response values and harden URI utilities Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 27 +++++++++++++++++++++++---- parser.go | 3 +++ protocol.go | 21 +++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/client.go b/client.go index de634fe..0fcccf1 100644 --- a/client.go +++ b/client.go @@ -220,7 +220,11 @@ func (c *Client) nextTransID() int { // sendCommand sends a command and waits for response func (c *Client) sendCommand(cmd string) (*Response, error) { - if c.conn == nil { + c.mu.Lock() + conn := c.conn + writer := c.writer + c.mu.Unlock() + if conn == nil || writer == nil { return nil, fmt.Errorf("not connected") } @@ -256,7 +260,7 @@ func (c *Client) sendCommand(cmd string) (*Response, error) { // Send command packet := fmt.Sprintf("%d\x00%s\x00", len(cmd), cmd) c.writeMu.Lock() - _, err := c.writer.Write([]byte(packet)) + _, err := writer.Write([]byte(packet)) c.writeMu.Unlock() if err != nil { c.mu.Lock() @@ -409,7 +413,7 @@ func (c *Client) GetSource(file string, begin, end int) (string, error) { if err != nil { return "", err } - return resp.Raw, nil + return decodeResponseValue(resp) } // Status returns current debugger status @@ -431,7 +435,7 @@ func (c *Client) FeatureGet(name string) (string, error) { if err != nil { return "", err } - return resp.Raw, nil + return decodeResponseValue(resp) } // FeatureSet sets a feature value @@ -466,3 +470,18 @@ func (c *Client) Close() error { } return closeErr } + +func decodeResponseValue(resp *Response) (string, error) { + value := strings.TrimSpace(resp.Value) + if value == "" { + value = strings.TrimSpace(resp.Raw) + } + if resp.Encoding == "base64" && value != "" { + decoded, err := base64.StdEncoding.DecodeString(value) + if err != nil { + return "", fmt.Errorf("decode base64 response: %w", err) + } + return string(decoded), nil + } + return value, nil +} diff --git a/parser.go b/parser.go index 63f17ea..8ceda40 100644 --- a/parser.go +++ b/parser.go @@ -164,5 +164,8 @@ func formatSimpleValue(typ, content, classname string) string { func FormatVariable(v Variable) string { indent := strings.Repeat(" ", v.Level) width := 30 - (v.Level * 2) + if width < 8 { + width = 8 + } return fmt.Sprintf("%s%-*s %-8s = %s", indent, width, v.Name, v.Type, v.Value) } diff --git a/protocol.go b/protocol.go index 939bde7..cf13d7e 100644 --- a/protocol.go +++ b/protocol.go @@ -96,6 +96,7 @@ type Response struct { Reason string `xml:"reason,attr,omitempty"` Success string `xml:"success,attr,omitempty"` BreakpointID int `xml:"id,attr,omitempty"` + Encoding string `xml:"encoding,attr,omitempty"` // For breakpoint_set Breakpoint *BreakpointInfo `xml:"breakpoint,omitempty"` @@ -115,6 +116,8 @@ type Response struct { // Raw for debugging Raw string `xml:",innerxml"` + // Value contains raw response chardata when present + Value string `xml:",chardata"` } // Error represents a DBGp error @@ -262,6 +265,10 @@ func FormatFileURI(uri string) string { path = path[1:] } + if u.Host != "" { + return "//" + u.Host + filepath.FromSlash(path) + } + return filepath.FromSlash(path) } @@ -271,6 +278,20 @@ func MakeFileURI(path string) string { return path } + if strings.HasPrefix(path, `\\`) || strings.HasPrefix(path, "//") { + trimmed := strings.TrimLeft(path, `/\`) + parts := strings.SplitN(trimmed, `\`, 2) + if len(parts) == 1 { + parts = strings.SplitN(trimmed, "/", 2) + } + host := parts[0] + sharePath := "" + if len(parts) == 2 { + sharePath = "/" + filepath.ToSlash(parts[1]) + } + return (&url.URL{Scheme: "file", Host: host, Path: sharePath}).String() + } + slashed := filepath.ToSlash(path) if strings.HasPrefix(slashed, "/") { return (&url.URL{Scheme: "file", Path: slashed}).String() From cb753b347de13d3d94c341a745ef698d9a98894d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:07:29 +0000 Subject: [PATCH 08/10] Fix init cleanup and breakpoint parsing edge cases Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 16 ++++++++++++++++ parser.go | 3 +-- parser_test.go | 5 +++++ protocol.go | 3 ++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index 0fcccf1..03e9946 100644 --- a/client.go +++ b/client.go @@ -24,6 +24,7 @@ type Client struct { mu sync.Mutex writeMu sync.Mutex + breakMu sync.Mutex transID int responses map[int]chan *Response @@ -83,11 +84,23 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { // Read init packet initData, err := c.readPacket() if err != nil { + _ = conn.Close() + c.mu.Lock() + c.conn = nil + c.reader = nil + c.writer = nil + c.mu.Unlock() return fmt.Errorf("read init packet: %w", err) } initPacket, err := ParseInit(initData) if err != nil { + _ = conn.Close() + c.mu.Lock() + c.conn = nil + c.reader = nil + c.writer = nil + c.mu.Unlock() return fmt.Errorf("parse init packet: %w", err) } c.mu.Lock() @@ -194,6 +207,9 @@ func (c *Client) readLoop() { // handleBreakpoint handles async breakpoint notifications func (c *Client) handleBreakpoint(resp *Response) { + c.breakMu.Lock() + defer c.breakMu.Unlock() + file, line := resp.ParseMessage() // Get stack diff --git a/parser.go b/parser.go index 8ceda40..6cf5ceb 100644 --- a/parser.go +++ b/parser.go @@ -91,8 +91,7 @@ func flattenProperties(props []Property, level int) []Variable { // formatPropertyValue formats a property value for display func formatPropertyValue(p Property) string { // Handle types with children (check Children attr, ChildProperties, or raw XML in Value) - hasChildren := p.Children > 0 || len(p.ChildProperties) > 0 || - (len(p.Value) > 0 && p.Value[0] == '<') + hasChildren := p.Children > 0 || len(p.ChildProperties) > 0 if hasChildren { count := p.NumChildren diff --git a/parser_test.go b/parser_test.go index cda8607..18f5ad4 100644 --- a/parser_test.go +++ b/parser_test.go @@ -62,6 +62,11 @@ func TestFormatPropertyValue(t *testing.T) { prop: Property{Type: "object", ClassName: "App\\Service", Children: 2}, want: "App\\Service", }, + { + name: "string starting with angle bracket", + prop: Property{Type: "string", Value: ""}, + want: `""`, + }, } for _, tt := range tests { diff --git a/protocol.go b/protocol.go index cf13d7e..bd861d5 100644 --- a/protocol.go +++ b/protocol.go @@ -292,7 +292,8 @@ func MakeFileURI(path string) string { return (&url.URL{Scheme: "file", Host: host, Path: sharePath}).String() } - slashed := filepath.ToSlash(path) + slashed := strings.ReplaceAll(path, `\`, `/`) + slashed = filepath.ToSlash(slashed) if strings.HasPrefix(slashed, "/") { return (&url.URL{Scheme: "file", Path: slashed}).String() } From 52fdd64999fbe057b75398c7737a5f9a092fbd22 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:08:36 +0000 Subject: [PATCH 09/10] Add breakpoint parsing tests and decode robustness Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- client.go | 28 ++++++++++++++++++++++------ protocol_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 protocol_test.go diff --git a/client.go b/client.go index 03e9946..7080eee 100644 --- a/client.go +++ b/client.go @@ -13,6 +13,7 @@ import ( "strings" "sync" "time" + "unicode" ) // Client manages the DBGp connection @@ -67,6 +68,8 @@ func (c *Client) WaitForConnection(timeout time.Duration) error { if err := deadlineListener.SetDeadline(time.Now().Add(timeout)); err != nil { return fmt.Errorf("set accept deadline: %w", err) } + } else { + return fmt.Errorf("listener does not support deadlines") } } @@ -166,18 +169,25 @@ func (c *Client) readLoop() { for { data, err := c.readPacket() if err != nil { + pending := make([]chan *Response, 0) c.mu.Lock() - for id, ch := range c.responses { - ch <- &Response{ - Transaction: id, + for _, ch := range c.responses { + pending = append(pending, ch) + } + c.responses = make(map[int]chan *Response) + c.mu.Unlock() + + for _, ch := range pending { + select { + case ch <- &Response{ Error: &Error{ Code: -1, Message: "connection closed", }, + }: + default: } - delete(c.responses, id) } - c.mu.Unlock() return } @@ -493,7 +503,13 @@ func decodeResponseValue(resp *Response) (string, error) { value = strings.TrimSpace(resp.Raw) } if resp.Encoding == "base64" && value != "" { - decoded, err := base64.StdEncoding.DecodeString(value) + cleaned := strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, value) + decoded, err := base64.StdEncoding.DecodeString(cleaned) if err != nil { return "", fmt.Errorf("decode base64 response: %w", err) } diff --git a/protocol_test.go b/protocol_test.go new file mode 100644 index 0000000..bbfe612 --- /dev/null +++ b/protocol_test.go @@ -0,0 +1,33 @@ +package dbgp + +import ( + "reflect" + "testing" +) + +func TestParseBreakpointSpec(t *testing.T) { + tests := []struct { + spec string + wantFile string + wantLines []int + }{ + {"file.php:42", "file.php", []int{42}}, + {"file.php:42,55,60", "file.php", []int{42, 55, 60}}, + {`C:\path\file.php:42`, `C:\path\file.php`, []int{42}}, + } + + for _, tt := range tests { + t.Run(tt.spec, func(t *testing.T) { + file, lines, err := ParseBreakpointSpec(tt.spec) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if file != tt.wantFile { + t.Fatalf("file = %q, want %q", file, tt.wantFile) + } + if !reflect.DeepEqual(lines, tt.wantLines) { + t.Fatalf("lines = %v, want %v", lines, tt.wantLines) + } + }) + } +} From 013dd061ca240c3289ec19c5e25dedeca321bc15 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:10:16 +0000 Subject: [PATCH 10/10] Improve file URI normalization and coverage Co-authored-by: carlos-granados <1383106+carlos-granados@users.noreply.github.com> --- protocol.go | 6 +++++- protocol_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/protocol.go b/protocol.go index bd861d5..368d09f 100644 --- a/protocol.go +++ b/protocol.go @@ -275,6 +275,10 @@ func FormatFileURI(uri string) string { // MakeFileURI converts path to file:// URI func MakeFileURI(path string) string { if strings.HasPrefix(path, "file://") { + u, err := url.Parse(path) + if err == nil { + return u.String() + } return path } @@ -287,7 +291,7 @@ func MakeFileURI(path string) string { host := parts[0] sharePath := "" if len(parts) == 2 { - sharePath = "/" + filepath.ToSlash(parts[1]) + sharePath = "/" + strings.ReplaceAll(parts[1], `\`, `/`) } return (&url.URL{Scheme: "file", Host: host, Path: sharePath}).String() } diff --git a/protocol_test.go b/protocol_test.go index bbfe612..84ed514 100644 --- a/protocol_test.go +++ b/protocol_test.go @@ -14,6 +14,8 @@ func TestParseBreakpointSpec(t *testing.T) { {"file.php:42", "file.php", []int{42}}, {"file.php:42,55,60", "file.php", []int{42, 55, 60}}, {`C:\path\file.php:42`, `C:\path\file.php`, []int{42}}, + {"file:///tmp/test.php:42", "file:///tmp/test.php", []int{42}}, + {"file://C:/path/test.php:42", "file://C:/path/test.php", []int{42}}, } for _, tt := range tests { @@ -31,3 +33,41 @@ func TestParseBreakpointSpec(t *testing.T) { }) } } + +func TestFileURIConversions(t *testing.T) { + tests := []struct { + name string + path string + expectURI string + }{ + { + name: "unix path with spaces", + path: "/tmp/test dir/file.php", + expectURI: "file:///tmp/test%20dir/file.php", + }, + { + name: "windows drive path", + path: `C:\path with spaces\file.php`, + expectURI: "file:///C:/path%20with%20spaces/file.php", + }, + { + name: "windows unc path", + path: `\\server\share\dir\file.php`, + expectURI: "file://server/share/dir/file.php", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotURI := MakeFileURI(tt.path) + if gotURI != tt.expectURI { + t.Fatalf("MakeFileURI() = %q, want %q", gotURI, tt.expectURI) + } + + gotPath := FormatFileURI(gotURI) + if gotPath == "" { + t.Fatalf("FormatFileURI() returned empty path") + } + }) + } +}