Skip to content

Commit 8f2eb7b

Browse files
authored
PR #7: Fix Critical Bugs in Argument Matching (#390)
This fixes three critical bugs discovered during validation testing. The first fix prevents wildcard pattern flags from incorrectly propagating to argument constraints. The second ensures argument validation is actually performed during scans. The third changes tuple extraction to properly distinguish between error conditions and valid empty string values using Go idioms. These fixes are essential for correct operation of the argument matching features.
1 parent 392e963 commit 8f2eb7b

File tree

3 files changed

+103
-62
lines changed

3 files changed

+103
-62
lines changed

python-dsl/codepathfinder/matchers.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ def _make_constraint(self, value: ArgumentValue) -> Dict[str, Any]:
7070
Dictionary with 'value' and 'wildcard' keys
7171
"""
7272
# Check if wildcard characters are present in string values
73+
# NOTE: Argument wildcard is independent of pattern wildcard (self.wildcard)
74+
# Pattern wildcard applies to function name matching (e.g., "*.bind")
75+
# Argument wildcard applies to argument value matching (e.g., "192.168.*")
7376
has_wildcard = False
7477
if isinstance(value, str) and ("*" in value or "?" in value):
7578
has_wildcard = True
@@ -78,7 +81,7 @@ def _make_constraint(self, value: ArgumentValue) -> Dict[str, Any]:
7881
isinstance(v, str) and ("*" in v or "?" in v) for v in value
7982
)
8083

81-
return {"value": value, "wildcard": has_wildcard or self.wildcard}
84+
return {"value": value, "wildcard": has_wildcard}
8285

8386
def to_ir(self) -> dict:
8487
"""

sourcecode-parser/dsl/call_matcher.go

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,14 @@ func (e *CallMatcherExecutor) ExecuteWithContext() []CallMatchResult {
152152
}
153153

154154
// getMatchedPattern returns which pattern matched (or empty string if no match).
155+
// Also checks argument constraints to ensure full matching logic is applied.
155156
func (e *CallMatcherExecutor) getMatchedPattern(cs *core.CallSite) string {
157+
// Check if the callsite matches (both pattern AND arguments)
158+
if !e.matchesCallSite(cs) {
159+
return "" // Doesn't match pattern or arguments don't satisfy constraints
160+
}
161+
162+
// Find which pattern matched
156163
for _, pattern := range e.IR.Patterns {
157164
if e.matchesPattern(cs.Target, pattern) {
158165
return pattern
@@ -293,10 +300,12 @@ func parseTupleIndex(posStr string) (int, int, bool, bool) {
293300
// 5. Clean up quotes and whitespace
294301
//
295302
// Examples:
296-
// - extractTupleElement("(\"0.0.0.0\", 8080)", 0) → "0.0.0.0"
297-
// - extractTupleElement("(\"0.0.0.0\", 8080)", 1) → "8080"
298-
// - extractTupleElement("(\"a\", \"b\", \"c\")", 1) → "b"
299-
// - extractTupleElement("not_a_tuple", 0) → "not_a_tuple"
303+
// - extractTupleElement("(\"0.0.0.0\", 8080)", 0) → ("0.0.0.0", true)
304+
// - extractTupleElement("(\"0.0.0.0\", 8080)", 1) → ("8080", true)
305+
// - extractTupleElement("(\"a\", \"b\", \"c\")", 1) → ("b", true)
306+
// - extractTupleElement("(\"a\", \"b\")", 5) → ("", false) // out of bounds
307+
// - extractTupleElement("(\"\", 8080)", 0) → ("", true) // empty string is valid
308+
// - extractTupleElement("not_a_tuple", 0) → ("not_a_tuple", true)
300309
//
301310
// Limitations:
302311
// - Does not handle nested tuples/lists
@@ -307,8 +316,9 @@ func parseTupleIndex(posStr string) (int, int, bool, bool) {
307316
// - index: 0-indexed position of element to extract
308317
//
309318
// Returns:
310-
// - Extracted element as string, or empty string if index out of bounds
311-
func extractTupleElement(tupleStr string, index int) string {
319+
// - value: Extracted element as string (can be empty string if that's the actual value)
320+
// - ok: true if extraction succeeded, false if index out of bounds
321+
func extractTupleElement(tupleStr string, index int) (string, bool) {
312322
tupleStr = strings.TrimSpace(tupleStr)
313323

314324
// Check if it's a tuple or list
@@ -317,29 +327,35 @@ func extractTupleElement(tupleStr string, index int) string {
317327
if index == 0 {
318328
// Remove quotes from plain strings too
319329
result := strings.Trim(tupleStr, `"'`)
320-
return result
330+
return result, true
321331
}
322-
return "" // Index out of bounds for non-tuple
332+
return "", false // Index out of bounds for non-tuple
323333
}
324334

325335
// Strip outer parentheses or brackets
326336
inner := tupleStr[1 : len(tupleStr)-1]
337+
inner = strings.TrimSpace(inner)
338+
339+
// Handle empty tuple/list
340+
if inner == "" {
341+
return "", false // Empty tuple has no elements
342+
}
327343

328344
// Split by comma
329345
// Note: This is a simple implementation that doesn't handle nested structures
330346
// For production, we'd need a proper parser
331347
elements := strings.Split(inner, ",")
332348

333349
if index >= len(elements) {
334-
return "" // Index out of bounds
350+
return "", false // Index out of bounds
335351
}
336352

337353
element := strings.TrimSpace(elements[index])
338354

339355
// Remove quotes if present (handles both single and double quotes)
340356
element = strings.Trim(element, `"'`)
341357

342-
return element
358+
return element, true
343359
}
344360

345361
// matchesPositionalArguments checks positional argument constraints.
@@ -381,10 +397,12 @@ func (e *CallMatcherExecutor) matchesPositionalArguments(args []core.Argument) b
381397

382398
// Extract tuple element if tuple indexing used
383399
if isTupleIndex {
384-
actualValue = extractTupleElement(actualValue, tupleIdx)
385-
if actualValue == "" {
400+
var ok bool
401+
actualValue, ok = extractTupleElement(actualValue, tupleIdx)
402+
if !ok {
386403
return false // Tuple index out of bounds
387404
}
405+
// Note: actualValue can be empty string if that's the actual tuple element value
388406
}
389407

390408
// Match against constraint

sourcecode-parser/dsl/call_matcher_test.go

Lines changed: 69 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,83 +1564,103 @@ func TestParseTupleIndex(t *testing.T) {
15641564
// TestExtractTupleElement tests extraction of elements from tuple strings.
15651565
func TestExtractTupleElement(t *testing.T) {
15661566
tests := []struct {
1567-
name string
1568-
input string
1569-
index int
1570-
expected string
1567+
name string
1568+
input string
1569+
index int
1570+
expected string
1571+
expectedOk bool
15711572
}{
15721573
{
1573-
name: "extract first element from string tuple",
1574-
input: `("0.0.0.0", 8080)`,
1575-
index: 0,
1576-
expected: "0.0.0.0",
1574+
name: "extract first element from string tuple",
1575+
input: `("0.0.0.0", 8080)`,
1576+
index: 0,
1577+
expected: "0.0.0.0",
1578+
expectedOk: true,
1579+
},
1580+
{
1581+
name: "extract second element from string tuple",
1582+
input: `("0.0.0.0", 8080)`,
1583+
index: 1,
1584+
expected: "8080",
1585+
expectedOk: true,
15771586
},
15781587
{
1579-
name: "extract second element from string tuple",
1580-
input: `("0.0.0.0", 8080)`,
1581-
index: 1,
1582-
expected: "8080",
1588+
name: "extract from single-quoted string",
1589+
input: `('0.0.0.0', 8080)`,
1590+
index: 0,
1591+
expected: "0.0.0.0",
1592+
expectedOk: true,
15831593
},
15841594
{
1585-
name: "extract from single-quoted string",
1586-
input: `('0.0.0.0', 8080)`,
1587-
index: 0,
1588-
expected: "0.0.0.0",
1595+
name: "extract from tuple with spaces",
1596+
input: `( "0.0.0.0" , 8080 )`,
1597+
index: 0,
1598+
expected: "0.0.0.0",
1599+
expectedOk: true,
15891600
},
15901601
{
1591-
name: "extract from tuple with spaces",
1592-
input: `( "0.0.0.0" , 8080 )`,
1593-
index: 0,
1594-
expected: "0.0.0.0",
1602+
name: "extract from three-element tuple",
1603+
input: `("a", "b", "c")`,
1604+
index: 1,
1605+
expected: "b",
1606+
expectedOk: true,
15951607
},
15961608
{
1597-
name: "extract from three-element tuple",
1598-
input: `("a", "b", "c")`,
1599-
index: 1,
1600-
expected: "b",
1609+
name: "extract from list syntax",
1610+
input: `["host", 8080]`,
1611+
index: 0,
1612+
expected: "host",
1613+
expectedOk: true,
16011614
},
16021615
{
1603-
name: "extract from list syntax",
1604-
input: `["host", 8080]`,
1605-
index: 0,
1606-
expected: "host",
1616+
name: "index out of bounds",
1617+
input: `("0.0.0.0", 8080)`,
1618+
index: 5,
1619+
expected: "",
1620+
expectedOk: false,
16071621
},
16081622
{
1609-
name: "index out of bounds",
1610-
input: `("0.0.0.0", 8080)`,
1611-
index: 5,
1612-
expected: "",
1623+
name: "not a tuple - return as is for index 0",
1624+
input: `"plain_string"`,
1625+
index: 0,
1626+
expected: "plain_string",
1627+
expectedOk: true,
16131628
},
16141629
{
1615-
name: "not a tuple - return as is for index 0",
1616-
input: `"plain_string"`,
1617-
index: 0,
1618-
expected: "plain_string",
1630+
name: "not a tuple - empty for index > 0",
1631+
input: `"plain_string"`,
1632+
index: 1,
1633+
expected: "",
1634+
expectedOk: false,
16191635
},
16201636
{
1621-
name: "not a tuple - empty for index > 0",
1622-
input: `"plain_string"`,
1623-
index: 1,
1624-
expected: "",
1637+
name: "empty tuple",
1638+
input: `()`,
1639+
index: 0,
1640+
expected: "",
1641+
expectedOk: false,
16251642
},
16261643
{
1627-
name: "empty tuple",
1628-
input: `()`,
1629-
index: 0,
1630-
expected: "",
1644+
name: "tuple with numeric values",
1645+
input: `(80, 443, 8080)`,
1646+
index: 1,
1647+
expected: "443",
1648+
expectedOk: true,
16311649
},
16321650
{
1633-
name: "tuple with numeric values",
1634-
input: `(80, 443, 8080)`,
1635-
index: 1,
1636-
expected: "443",
1651+
name: "tuple with empty string - should extract empty string successfully",
1652+
input: `("", 8080)`,
1653+
index: 0,
1654+
expected: "",
1655+
expectedOk: true,
16371656
},
16381657
}
16391658

16401659
for _, tt := range tests {
16411660
t.Run(tt.name, func(t *testing.T) {
1642-
result := extractTupleElement(tt.input, tt.index)
1661+
result, ok := extractTupleElement(tt.input, tt.index)
16431662
assert.Equal(t, tt.expected, result)
1663+
assert.Equal(t, tt.expectedOk, ok)
16441664
})
16451665
}
16461666
}

0 commit comments

Comments
 (0)