From ae9e06c98b9268c13eec07c7d034364234ae3d7f Mon Sep 17 00:00:00 2001 From: Hein Date: Fri, 15 May 2026 13:35:24 +0200 Subject: [PATCH] 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 --- pkg/common/adapters/database/bun.go | 12 ++++++-- pkg/common/sql_helpers.go | 46 +++++++++++++++++++++++++++++ pkg/common/sql_helpers_test.go | 24 +++++++++++++++ pkg/restheadspec/handler.go | 20 +++++++++++-- pkg/restheadspec/headers.go | 44 ++++++++++++++++++++++----- 5 files changed, 133 insertions(+), 13 deletions(-) diff --git a/pkg/common/adapters/database/bun.go b/pkg/common/adapters/database/bun.go index 18c98bc..19a7259 100644 --- a/pkg/common/adapters/database/bun.go +++ b/pkg/common/adapters/database/bun.go @@ -487,6 +487,14 @@ func normalizeTableAlias(query, expectedAlias, tableName string) string { 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 { b.query = b.query.WhereOr(query, args...) 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 query doesn't already have AS, check if it's a simple table name 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" joinClause = fmt.Sprintf("%s AS %s", parts[0], prefix) if len(parts) > 1 { @@ -552,7 +560,7 @@ func (b *BunSelectQuery) LeftJoin(query string, args ...interface{}) common.Sele joinClause := query if prefix != "" && !strings.Contains(strings.ToUpper(query), " AS ") { 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) if len(parts) > 1 { joinClause += " " + strings.Join(parts[1:], " ") diff --git a/pkg/common/sql_helpers.go b/pkg/common/sql_helpers.go index 32ba54e..294ebf7 100644 --- a/pkg/common/sql_helpers.go +++ b/pkg/common/sql_helpers.go @@ -59,6 +59,38 @@ func IsSQLExpression(cond string) bool { 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 // These conditions should be removed from WHERE clauses as they have no filtering effect func IsTrivialCondition(cond string) bool { @@ -147,6 +179,14 @@ func SanitizeWhereClause(where string, tableName string, options ...*RequestOpti 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 // If so, we need to preserve the outer parentheses to prevent OR logic from escaping hasOuterParens := false @@ -212,6 +252,12 @@ func SanitizeWhereClause(where string, tableName string, options ...*RequestOpti 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 != "" && hasTablePrefix(condToCheck) { // Extract the current prefix and column name diff --git a/pkg/common/sql_helpers_test.go b/pkg/common/sql_helpers_test.go index acfd831..96e0e5a 100644 --- a/pkg/common/sql_helpers_test.go +++ b/pkg/common/sql_helpers_test.go @@ -134,6 +134,30 @@ func TestSanitizeWhereClause(t *testing.T) { 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))", }, + { + 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 { diff --git a/pkg/restheadspec/handler.go b/pkg/restheadspec/handler.go index ab4f086..b4d39d1 100644 --- a/pkg/restheadspec/handler.go +++ b/pkg/restheadspec/handler.go @@ -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 { - 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) - // Joins are already sanitized during parsing, so we can apply them directly query = query.Join(joinClause) } } diff --git a/pkg/restheadspec/headers.go b/pkg/restheadspec/headers.go index 4af6e48..1418bfc 100644 --- a/pkg/restheadspec/headers.go +++ b/pkg/restheadspec/headers.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "reflect" + "regexp" "strconv" "strings" @@ -501,6 +502,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 // Format: Single JOIN clause or multiple JOIN clauses separated by | // Example: "LEFT JOIN departments d ON d.id = employees.department_id" @@ -533,17 +559,19 @@ func (h *Handler) parseCustomSQLJoin(options *ExtendedRequestOptions, value stri continue } - // Extract table alias from the JOIN clause - alias := extractJoinAlias(sanitizedJoin) - if alias != "" { + // Split into individual JOIN clauses so each clause gets its own alias entry. + // CustomSQLJoin and JoinAliases are kept parallel (one entry per individual clause). + 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) - // Also add to the embedded RequestOptions for validation options.RequestOptions.JoinAliases = append(options.RequestOptions.JoinAliases, alias) - logger.Debug("Extracted join alias: %s", alias) + if alias != "" { + logger.Debug("Extracted join alias: %s", alias) + } + logger.Debug("Adding custom SQL join: %s", clause) + options.CustomSQLJoin = append(options.CustomSQLJoin, clause) } - - logger.Debug("Adding custom SQL join: %s", sanitizedJoin) - options.CustomSQLJoin = append(options.CustomSQLJoin, sanitizedJoin) } }