Compare commits

...

4 Commits

Author SHA1 Message Date
Hein
cb416d49c4 fix(headers): handle decoding errors in header values
Some checks failed
Build , Vet Test, and Lint / Run Vet Tests (1.24.x) (push) Successful in -33m58s
Build , Vet Test, and Lint / Run Vet Tests (1.23.x) (push) Successful in -33m22s
Build , Vet Test, and Lint / Lint Code (push) Failing after -33m34s
Build , Vet Test, and Lint / Build (push) Successful in -33m45s
Tests / Unit Tests (push) Failing after -34m38s
Tests / Integration Tests (push) Failing after -34m48s
* return original value if decoding fails
* decode base64 strings when appropriate
2026-05-15 16:59:06 +02:00
Hein
cb921f2c5e fix(websocketspec): add transaction access to HookContext 2026-05-15 14:59:34 +02:00
Hein
1ebe0d7ac3 fix(funcspec): refine filter application logic for SQL queries
* update filter checks to only consider SELECT list
* add test for function parameters not matching filters
2026-05-15 14:28:12 +02:00
Hein
ae9e06c98b fix(sql_helpers): strip empty RHS conditions from SQL strings
* Add regex patterns to identify and remove empty comparisons
* Implement tests for stripping empty RHS conditions
fix(handler): prevent duplicate JOIN aliases from preload
* Skip custom SQL JOINs if alias already provided by preload
* Split multiple JOIN clauses for individual alias handling
2026-05-15 13:35:24 +02:00
9 changed files with 216 additions and 17 deletions

View File

@@ -487,6 +487,14 @@ func normalizeTableAlias(query, expectedAlias, tableName string) string {
return modified return modified
} }
func isJoinKeyword(word string) bool {
switch strings.ToUpper(word) {
case "JOIN", "INNER", "LEFT", "RIGHT", "FULL", "OUTER", "CROSS":
return true
}
return false
}
func (b *BunSelectQuery) WhereOr(query string, args ...interface{}) common.SelectQuery { func (b *BunSelectQuery) WhereOr(query string, args ...interface{}) common.SelectQuery {
b.query = b.query.WhereOr(query, args...) b.query = b.query.WhereOr(query, args...)
return b return b
@@ -517,7 +525,7 @@ func (b *BunSelectQuery) Join(query string, args ...interface{}) common.SelectQu
if prefix != "" && !strings.Contains(strings.ToUpper(query), " AS ") { if prefix != "" && !strings.Contains(strings.ToUpper(query), " AS ") {
// If query doesn't already have AS, check if it's a simple table name // If query doesn't already have AS, check if it's a simple table name
parts := strings.Fields(query) parts := strings.Fields(query)
if len(parts) > 0 && !strings.HasPrefix(strings.ToUpper(parts[0]), "JOIN") { if len(parts) > 0 && !isJoinKeyword(parts[0]) {
// Simple table name, add prefix: "table AS prefix" // Simple table name, add prefix: "table AS prefix"
joinClause = fmt.Sprintf("%s AS %s", parts[0], prefix) joinClause = fmt.Sprintf("%s AS %s", parts[0], prefix)
if len(parts) > 1 { if len(parts) > 1 {
@@ -552,7 +560,7 @@ func (b *BunSelectQuery) LeftJoin(query string, args ...interface{}) common.Sele
joinClause := query joinClause := query
if prefix != "" && !strings.Contains(strings.ToUpper(query), " AS ") { if prefix != "" && !strings.Contains(strings.ToUpper(query), " AS ") {
parts := strings.Fields(query) parts := strings.Fields(query)
if len(parts) > 0 && !strings.HasPrefix(strings.ToUpper(parts[0]), "LEFT") && !strings.HasPrefix(strings.ToUpper(parts[0]), "JOIN") { if len(parts) > 0 && !isJoinKeyword(parts[0]) {
joinClause = fmt.Sprintf("%s AS %s", parts[0], prefix) joinClause = fmt.Sprintf("%s AS %s", parts[0], prefix)
if len(parts) > 1 { if len(parts) > 1 {
joinClause += " " + strings.Join(parts[1:], " ") joinClause += " " + strings.Join(parts[1:], " ")

View File

@@ -59,6 +59,38 @@ func IsSQLExpression(cond string) bool {
return false return false
} }
// reEmptyCompMid matches a simple column comparison with an empty RHS that is immediately
// followed by AND/OR (only whitespace between the operator and the next keyword).
// Removing the match leaves the preceding AND/OR connector intact.
// Example: "cond1 and col = \n and cond2" → "cond1 and cond2"
var reEmptyCompMid = regexp.MustCompile(`(?i)[\w.]+\s*(?:=|<>|!=|>=|<=|>|<)\s+(?:and|or)\s+`)
// reEmptyCompEnd matches AND/OR + a simple column comparison with an empty RHS at the end
// of the string (or sub-clause).
// Example: "cond1 and col = " → "cond1"
var reEmptyCompEnd = regexp.MustCompile(`(?i)\s+(?:and|or)\s+[\w.]+\s*(?:=|<>|!=|>=|<=|>|<)\s*$`)
// stripEmptyComparisonClauses removes comparison conditions that have no right-hand side
// value from a raw SQL string. Operates on the whole string so it also cleans up conditions
// inside subqueries, not just top-level AND splits.
func stripEmptyComparisonClauses(sql string) string {
sql = reEmptyCompMid.ReplaceAllLiteralString(sql, "")
sql = reEmptyCompEnd.ReplaceAllLiteralString(sql, "")
return sql
}
// hasEmptyRHS returns true when a condition ends with a comparison operator and has no
// right-hand side value — e.g., "col = ", "com.rid_parent = ", "col >= ".
func hasEmptyRHS(cond string) bool {
cond = strings.TrimSpace(cond)
for _, op := range []string{"<>", "!=", ">=", "<=", "=", ">", "<"} {
if strings.HasSuffix(cond, op) {
return true
}
}
return false
}
// IsTrivialCondition checks if a condition is trivial and always evaluates to true // IsTrivialCondition checks if a condition is trivial and always evaluates to true
// These conditions should be removed from WHERE clauses as they have no filtering effect // These conditions should be removed from WHERE clauses as they have no filtering effect
func IsTrivialCondition(cond string) bool { func IsTrivialCondition(cond string) bool {
@@ -147,6 +179,14 @@ func SanitizeWhereClause(where string, tableName string, options ...*RequestOpti
return "" return ""
} }
// Strip comparison conditions with empty RHS throughout the SQL string (including
// inside subqueries), before condition splitting.
where = stripEmptyComparisonClauses(where)
if where == "" {
return ""
}
where = strings.TrimSpace(where)
// Check if the original clause has outer parentheses and contains OR operators // Check if the original clause has outer parentheses and contains OR operators
// If so, we need to preserve the outer parentheses to prevent OR logic from escaping // If so, we need to preserve the outer parentheses to prevent OR logic from escaping
hasOuterParens := false hasOuterParens := false
@@ -212,6 +252,12 @@ func SanitizeWhereClause(where string, tableName string, options ...*RequestOpti
continue continue
} }
// Skip conditions with no right-hand side value (e.g. "col = " with empty value)
if hasEmptyRHS(condToCheck) {
logger.Debug("Removing condition with empty value: '%s'", cond)
continue
}
// If tableName is provided and the condition HAS a table prefix, check if it's correct // If tableName is provided and the condition HAS a table prefix, check if it's correct
if tableName != "" && hasTablePrefix(condToCheck) { if tableName != "" && hasTablePrefix(condToCheck) {
// Extract the current prefix and column name // Extract the current prefix and column name

View File

@@ -134,6 +134,30 @@ func TestSanitizeWhereClause(t *testing.T) {
tableName: "apiprovider", tableName: "apiprovider",
expected: "apiprovider.type in ('softphone') AND (apiprovider.rid_apiprovider in (select l.rid_apiprovider from core.apiproviderlink l where l.rid_hub = 2576))", expected: "apiprovider.type in ('softphone') AND (apiprovider.rid_apiprovider in (select l.rid_apiprovider from core.apiproviderlink l where l.rid_hub = 2576))",
}, },
{
name: "empty RHS stripped mid-clause",
where: "com.tableprefix = 'tcli' and com.rid_parent = \n and com.status = 'Active'",
tableName: "",
expected: "com.tableprefix = 'tcli' AND com.status = 'Active'",
},
{
name: "empty RHS stripped at end of clause",
where: "com.tableprefix = 'tcli' and com.rid_parent =",
tableName: "",
expected: "com.tableprefix = 'tcli'",
},
{
name: "non-empty value not stripped",
where: "com.tableprefix = 'tcli' and com.rid_parent = 123 and com.status = 'Active'",
tableName: "",
expected: "com.tableprefix = 'tcli' AND com.rid_parent = 123 AND com.status = 'Active'",
},
{
name: "empty RHS inside subquery stripped",
where: "a = 1 and b in (select x from t where c.rid = \n and d = 2)",
tableName: "",
expected: "a = 1 AND b in (select x from t where d = 2)",
},
} }
for _, tt := range tests { for _, tt := range tests {

View File

@@ -729,9 +729,10 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m
propQry[parmk] = val propQry[parmk] = val
} }
// Apply filters if allowed — check against string-literal-stripped SQL to avoid // Apply filters if allowed — check only the SELECT list to avoid matching function
// matching column names that only appear inside quoted arguments (e.g. JSON strings) // parameters in the FROM clause (e.g. [p_rid_doctype] in a set-returning function call)
if allowFilter && len(parmk) > 1 && strings.Contains(strings.ToLower(sqlStripStringLiterals(sqlquery)), strings.ToLower(parmk)) { // or names inside quoted string arguments.
if allowFilter && len(parmk) > 1 && strings.Contains(sqlSelectList(sqlStripStringLiterals(sqlquery)), strings.ToLower(parmk)) {
if len(parmv) > 1 { if len(parmv) > 1 {
// Sanitize each value in the IN clause with appropriate quoting // Sanitize each value in the IN clause with appropriate quoting
sanitizedValues := make([]string, len(parmv)) sanitizedValues := make([]string, len(parmv))
@@ -847,6 +848,18 @@ func sqlStripStringLiterals(sql string) string {
return re.ReplaceAllString(sql, "''") return re.ReplaceAllString(sql, "''")
} }
// sqlSelectList returns the column list portion of a SELECT query (between SELECT and FROM).
// Returns the full query lowercased if no clear SELECT…FROM boundary is found.
func sqlSelectList(sql string) string {
lower := strings.ToLower(sql)
selectPos := strings.Index(lower, "select ")
fromPos := strings.Index(lower, " from ")
if selectPos < 0 || fromPos <= selectPos {
return lower
}
return lower[selectPos+7 : fromPos]
}
// replaceMetaVariables replaces meta variables like [rid_user], [user], etc. in the SQL query // replaceMetaVariables replaces meta variables like [rid_user], [user], etc. in the SQL query
func (h *Handler) replaceMetaVariables(sqlquery string, r *http.Request, userCtx *security.UserContext, metainfo map[string]interface{}, variables map[string]interface{}) string { func (h *Handler) replaceMetaVariables(sqlquery string, r *http.Request, userCtx *security.UserContext, metainfo map[string]interface{}, variables map[string]interface{}) string {
if strings.Contains(sqlquery, "[p_meta_default]") { if strings.Contains(sqlquery, "[p_meta_default]") {

View File

@@ -957,6 +957,60 @@ func TestAllowFilterDoesNotMatchInsideJsonArgument(t *testing.T) {
} }
} }
// TestAllowFilterDoesNotMatchFunctionParams verifies that query params that appear only
// as function call arguments in the FROM clause (e.g. [p_rid_doctype]) are not treated
// as column filters, since they are not in the SELECT list.
func TestAllowFilterDoesNotMatchFunctionParams(t *testing.T) {
handler := NewHandler(&MockDatabase{})
sqlQuery := `select rid, rid_parent, description, row_cnt, filterstring, tableprefix, rid_table, tooltip, additionalfilter, haschildren
from crm_get_doc_menu($JQ$[p_tableprefix]$JQ$,[p_rid_parent],[p_rid_doctype],[p_removedup],[p_showall]) r`
tests := []struct {
name string
queryParams map[string]string
checkResult func(t *testing.T, result string)
}{
{
name: "p_rid_doctype is a function param, not a column — no filter applied",
queryParams: map[string]string{"p_rid_doctype": "0"},
checkResult: func(t *testing.T, result string) {
if strings.Contains(strings.ToLower(result), "where") {
t.Errorf("Expected no WHERE clause for p_rid_doctype (function arg, not SELECT column), got:\n%s", result)
}
},
},
{
name: "p_showall is a function param, not a column — no filter applied",
queryParams: map[string]string{"p_showall": "1"},
checkResult: func(t *testing.T, result string) {
if strings.Contains(strings.ToLower(result), "where") {
t.Errorf("Expected no WHERE clause for p_showall (function arg, not SELECT column), got:\n%s", result)
}
},
},
{
name: "rid is a SELECT column — filter applied",
queryParams: map[string]string{"rid": "42"},
checkResult: func(t *testing.T, result string) {
if !strings.Contains(strings.ToLower(result), "where") {
t.Error("Expected WHERE clause for rid (real SELECT column)")
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req := createTestRequest("GET", "/test", tt.queryParams, nil, nil)
variables := make(map[string]interface{})
propQry := make(map[string]string)
result := handler.mergeQueryParams(req, sqlQuery, variables, true, propQry)
tt.checkResult(t, result)
})
}
}
// TestGetReplacementForBlankParamDoubleQuote verifies that placeholders surrounded by // TestGetReplacementForBlankParamDoubleQuote verifies that placeholders surrounded by
// double quotes (as in JSON string values) are blanked to "" not NULL. // double quotes (as in JSON string values) are blanked to "" not NULL.
func TestGetReplacementForBlankParamDoubleQuote(t *testing.T) { func TestGetReplacementForBlankParamDoubleQuote(t *testing.T) {

View File

@@ -575,11 +575,25 @@ func (h *Handler) handleRead(ctx context.Context, w common.ResponseWriter, id st
} }
} }
// Apply custom SQL JOIN clauses // Apply custom SQL JOIN clauses, skipping any whose alias is already provided by a
// preload LEFT JOIN (to prevent "table name specified more than once" errors).
if len(options.CustomSQLJoin) > 0 { if len(options.CustomSQLJoin) > 0 {
for _, joinClause := range options.CustomSQLJoin { preloadAliasSet := make(map[string]bool, len(options.Preload))
for _, p := range options.Preload {
if alias := common.RelationPathToBunAlias(p.Relation); alias != "" {
preloadAliasSet[alias] = true
}
}
for i, joinClause := range options.CustomSQLJoin {
if i < len(options.JoinAliases) && options.JoinAliases[i] != "" {
alias := strings.ToLower(options.JoinAliases[i])
if preloadAliasSet[alias] {
logger.Debug("Skipping custom SQL JOIN (alias '%s' already joined by preload): %s", alias, joinClause)
continue
}
}
logger.Debug("Applying custom SQL JOIN: %s", joinClause) logger.Debug("Applying custom SQL JOIN: %s", joinClause)
// Joins are already sanitized during parsing, so we can apply them directly
query = query.Join(joinClause) query = query.Join(joinClause)
} }
} }

View File

@@ -5,6 +5,7 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"reflect" "reflect"
"regexp"
"strconv" "strconv"
"strings" "strings"
@@ -63,7 +64,10 @@ type ExpandOption struct {
// decodeHeaderValue decodes base64 encoded header values // decodeHeaderValue decodes base64 encoded header values
// Supports ZIP_ and __ prefixes for base64 encoding // Supports ZIP_ and __ prefixes for base64 encoding
func decodeHeaderValue(value string) string { func decodeHeaderValue(value string) string {
str, _ := DecodeParam(value) str, err := DecodeParam(value)
if err != nil {
return value
}
return str return str
} }
@@ -97,6 +101,11 @@ func DecodeParam(pStr string) (string, error) {
if strings.HasPrefix(code, "ZIP_") || strings.HasPrefix(code, "__") { if strings.HasPrefix(code, "ZIP_") || strings.HasPrefix(code, "__") {
code, _ = DecodeParam(code) code, _ = DecodeParam(code)
} else {
strDat, err := base64.StdEncoding.DecodeString(code)
if err == nil {
code = string(strDat)
}
} }
return code, nil return code, nil
@@ -501,6 +510,31 @@ func (h *Handler) parseExpand(options *ExtendedRequestOptions, value string) {
} }
} }
// reMultiJoinBoundary finds the start of each individual JOIN clause within a string that
// may contain multiple consecutive JOIN clauses (e.g., "INNER JOIN ... LEFT OUTER JOIN ...").
var reMultiJoinBoundary = regexp.MustCompile(`(?i)(?:inner|left(?:\s+outer)?|right(?:\s+outer)?|full(?:\s+outer)?|cross)\s+join\b`)
// splitJoinClauses splits a SQL string that may contain multiple JOIN clauses into
// individual clauses. A plain pipe-separated segment may itself contain several JOINs;
// this function splits them so each gets its own alias entry.
func splitJoinClauses(joinStr string) []string {
indices := reMultiJoinBoundary.FindAllStringIndex(joinStr, -1)
if len(indices) <= 1 {
return []string{strings.TrimSpace(joinStr)}
}
parts := make([]string, 0, len(indices))
for i, idx := range indices {
end := len(joinStr)
if i+1 < len(indices) {
end = indices[i+1][0]
}
if part := strings.TrimSpace(joinStr[idx[0]:end]); part != "" {
parts = append(parts, part)
}
}
return parts
}
// parseCustomSQLJoin parses x-custom-sql-join header // parseCustomSQLJoin parses x-custom-sql-join header
// Format: Single JOIN clause or multiple JOIN clauses separated by | // Format: Single JOIN clause or multiple JOIN clauses separated by |
// Example: "LEFT JOIN departments d ON d.id = employees.department_id" // Example: "LEFT JOIN departments d ON d.id = employees.department_id"
@@ -533,17 +567,19 @@ func (h *Handler) parseCustomSQLJoin(options *ExtendedRequestOptions, value stri
continue continue
} }
// Extract table alias from the JOIN clause // Split into individual JOIN clauses so each clause gets its own alias entry.
alias := extractJoinAlias(sanitizedJoin) // CustomSQLJoin and JoinAliases are kept parallel (one entry per individual clause).
if alias != "" { for _, clause := range splitJoinClauses(sanitizedJoin) {
alias := extractJoinAlias(clause)
// Keep arrays parallel; use empty string when alias cannot be extracted.
options.JoinAliases = append(options.JoinAliases, alias) options.JoinAliases = append(options.JoinAliases, alias)
// Also add to the embedded RequestOptions for validation
options.RequestOptions.JoinAliases = append(options.RequestOptions.JoinAliases, alias) options.RequestOptions.JoinAliases = append(options.RequestOptions.JoinAliases, alias)
if alias != "" {
logger.Debug("Extracted join alias: %s", alias) logger.Debug("Extracted join alias: %s", alias)
} }
logger.Debug("Adding custom SQL join: %s", clause)
logger.Debug("Adding custom SQL join: %s", sanitizedJoin) options.CustomSQLJoin = append(options.CustomSQLJoin, clause)
options.CustomSQLJoin = append(options.CustomSQLJoin, sanitizedJoin) }
} }
} }

View File

@@ -174,6 +174,7 @@ func (h *Handler) handleRequest(conn *Connection, msg *Message) {
Options: msg.Options, Options: msg.Options,
ID: recordID, ID: recordID,
Data: msg.Data, Data: msg.Data,
Tx: h.db,
Metadata: make(map[string]interface{}), Metadata: make(map[string]interface{}),
} }

View File

@@ -111,6 +111,9 @@ type HookContext struct {
AbortMessage string // Message to return if aborted AbortMessage string // Message to return if aborted
AbortCode int // HTTP status code if aborted AbortCode int // HTTP status code if aborted
// Tx provides access to the database/transaction for executing additional SQL
Tx common.Database
// Metadata is additional context data // Metadata is additional context data
Metadata map[string]interface{} Metadata map[string]interface{}
} }