Compare commits

...

5 Commits

Author SHA1 Message Date
Hein c120b49529 fix(router): prevent HTML escaping in JSON responses
Build , Vet Test, and Lint / Run Vet Tests (1.24.x) (push) Waiting to run
Build , Vet Test, and Lint / Lint Code (push) Waiting to run
Build , Vet Test, and Lint / Build (push) Waiting to run
Build , Vet Test, and Lint / Run Vet Tests (1.23.x) (push) Waiting to run
Tests / Integration Tests (push) Waiting to run
Tests / Unit Tests (push) Waiting to run
fix(sql_helpers): avoid prefix extraction in subqueries
2026-06-08 15:13:58 +02:00
Hein 66348dac97 test(handler): add tests for valid nested request verbs 2026-06-08 09:06:29 +02:00
Hein a87cd18b1b fix(handler): validate nested request structure for relations
* added checks for valid _request values in single and multiple relations
* introduced isValidNestedRequest function to encapsulate validation logic
fix(crud): expand operation handling for nested CUD
* added "add" to insert operations and "modify" to update operations
* included "remove" in delete operations
2026-06-08 09:02:29 +02:00
Hein 29449c93d5 fix(test): add tests for asymmetric join column handling
Build , Vet Test, and Lint / Run Vet Tests (1.23.x) (push) Waiting to run
Build , Vet Test, and Lint / Run Vet Tests (1.24.x) (push) Waiting to run
Build , Vet Test, and Lint / Lint Code (push) Waiting to run
Build , Vet Test, and Lint / Build (push) Waiting to run
Tests / Unit Tests (push) Waiting to run
Tests / Integration Tests (push) Waiting to run
2026-06-07 19:13:59 +02:00
Hein 3b6e5c75be fix(handler): update foreign key field resolution logic
Build , Vet Test, and Lint / Run Vet Tests (1.23.x) (push) Waiting to run
Build , Vet Test, and Lint / Run Vet Tests (1.24.x) (push) Waiting to run
Build , Vet Test, and Lint / Lint Code (push) Waiting to run
Build , Vet Test, and Lint / Build (push) Waiting to run
Tests / Unit Tests (push) Waiting to run
Tests / Integration Tests (push) Waiting to run
* Adjust foreign key field name selection for has-many/has-one relationships
* Improve logging to clarify foreign key and child field usage
2026-06-07 14:20:55 +02:00
7 changed files with 319 additions and 18 deletions
+3 -1
View File
@@ -174,7 +174,9 @@ func (h *HTTPResponseWriter) Write(data []byte) (int, error) {
func (h *HTTPResponseWriter) WriteJSON(data interface{}) error { func (h *HTTPResponseWriter) WriteJSON(data interface{}) error {
h.SetHeader("Content-Type", "application/json") h.SetHeader("Content-Type", "application/json")
return json.NewEncoder(h.resp).Encode(data) enc := json.NewEncoder(h.resp)
enc.SetEscapeHTML(false)
return enc.Encode(data)
} }
// UnderlyingResponseWriter returns the underlying http.ResponseWriter // UnderlyingResponseWriter returns the underlying http.ResponseWriter
+3 -1
View File
@@ -178,7 +178,9 @@ func (s *StandardResponseWriter) Write(data []byte) (int, error) {
func (s *StandardResponseWriter) WriteJSON(data interface{}) error { func (s *StandardResponseWriter) WriteJSON(data interface{}) error {
s.SetHeader("Content-Type", "application/json") s.SetHeader("Content-Type", "application/json")
return json.NewEncoder(s.w).Encode(data) enc := json.NewEncoder(s.w)
enc.SetEscapeHTML(false)
return enc.Encode(data)
} }
func (s *StandardResponseWriter) UnderlyingResponseWriter() http.ResponseWriter { func (s *StandardResponseWriter) UnderlyingResponseWriter() http.ResponseWriter {
+13 -9
View File
@@ -113,7 +113,7 @@ func (p *NestedCUDProcessor) ProcessNestedCUD(
// Process based on operation // Process based on operation
switch strings.ToLower(operation) { switch strings.ToLower(operation) {
case "insert", "create": case "insert", "create", "add":
// Only perform insert if we have data to insert // Only perform insert if we have data to insert
if hasData { if hasData {
id, err := p.processInsert(ctx, regularData, tableName) id, err := p.processInsert(ctx, regularData, tableName)
@@ -141,7 +141,7 @@ func (p *NestedCUDProcessor) ProcessNestedCUD(
logger.Debug("Skipping insert for %s - no data columns besides _request", tableName) logger.Debug("Skipping insert for %s - no data columns besides _request", tableName)
} }
case "update", "change": case "update", "change", "modify":
// Only perform update if we have data to update // Only perform update if we have data to update
if reflection.IsEmptyValue(data[pkName]) { if reflection.IsEmptyValue(data[pkName]) {
logger.Warn("Skipping update for %s - no primary key", tableName) logger.Warn("Skipping update for %s - no primary key", tableName)
@@ -174,7 +174,7 @@ func (p *NestedCUDProcessor) ProcessNestedCUD(
result.ID = data[pkName] result.ID = data[pkName]
} }
case "delete": case "delete", "remove":
if reflection.IsEmptyValue(data[pkName]) { if reflection.IsEmptyValue(data[pkName]) {
logger.Warn("Skipping delete for %s - no primary key", tableName) logger.Warn("Skipping delete for %s - no primary key", tableName)
return result, nil return result, nil
@@ -471,13 +471,17 @@ func (p *NestedCUDProcessor) processChildRelations(
// Priority: Use foreign key field name if specified // Priority: Use foreign key field name if specified
var foreignKeyFieldName string var foreignKeyFieldName string
if relInfo.ForeignKey != "" { if relInfo.ForeignKey != "" {
// Get the JSON name for the foreign key field in the child model // For has-many/has-one: join:parentCol=childCol
foreignKeyFieldName = reflection.GetJSONNameForField(relatedModelType, relInfo.ForeignKey) // ForeignKey = parent side, References = child side (where we actually set the value)
if foreignKeyFieldName == "" { childField := relInfo.ForeignKey
// Fallback to lowercase field name if (relInfo.RelationType == "hasMany" || relInfo.RelationType == "hasOne") && relInfo.References != "" {
foreignKeyFieldName = strings.ToLower(relInfo.ForeignKey) childField = relInfo.References
} }
logger.Debug("Using foreign key field for direct assignment: %s (from FK %s)", foreignKeyFieldName, relInfo.ForeignKey) foreignKeyFieldName = reflection.GetJSONNameForField(relatedModelType, childField)
if foreignKeyFieldName == "" {
foreignKeyFieldName = strings.ToLower(childField)
}
logger.Debug("Using foreign key field for direct assignment: %s (from FK %s -> child %s)", foreignKeyFieldName, relInfo.ForeignKey, childField)
} }
// Get the primary key name for the child model to avoid overwriting it in recursive relationships // Get the primary key name for the child model to avoid overwriting it in recursive relationships
+214
View File
@@ -713,6 +713,220 @@ func TestInjectForeignKeys(t *testing.T) {
} }
} }
// Models for asymmetric join column tests (mirrors the bun has-many join:parentCol=childCol pattern).
// ActionOption has-many ActionOptionLinks via join:rid_actionoption=rid_actionoption_child.
// The child column ("rid_actionoption_child") differs from the parent column ("rid_actionoption").
type ActionOption struct {
RidActionoption int64 `json:"rid_actionoption" bun:"rid_actionoption,pk"`
Label string `json:"label"`
Links []*ActionOptionLink `json:"aol_rid_actionoption_child,omitempty"`
}
func (a ActionOption) TableName() string { return "action_options" }
func (a ActionOption) GetIDName() string { return "RidActionoption" }
type ActionOptionLink struct {
RidActionoptionlink int64 `json:"rid_actionoptionlink" bun:"rid_actionoptionlink,pk"`
RidActionoptionChild int64 `json:"rid_actionoption_child" bun:"rid_actionoption_child"`
Label string `json:"label"`
// Note: no field named "rid_actionoption" — that is the parent's column.
}
func (a ActionOptionLink) TableName() string { return "action_option_links" }
func (a ActionOptionLink) GetIDName() string { return "RidActionoptionlink" }
// TestProcessNestedCUD_AsymmetricJoinColumns verifies that for a has-many relation with
// join:parentCol=childCol, the child rows are stamped with the child-side column (References),
// not the parent-side column (ForeignKey).
func TestProcessNestedCUD_AsymmetricJoinColumns(t *testing.T) {
db := newMockDatabase()
registry := &mockModelRegistry{}
relProvider := newMockRelationshipProvider()
// Mirrors: bun:"rel:has-many,join:rid_actionoption=rid_actionoption_child"
relProvider.RegisterRelation("ActionOption", "aol_rid_actionoption_child", &RelationshipInfo{
FieldName: "Links",
JSONName: "aol_rid_actionoption_child",
RelationType: "hasMany",
ForeignKey: "rid_actionoption", // parent-side column (left of join:)
References: "rid_actionoption_child", // child-side column (right of join:)
RelatedModel: ActionOptionLink{},
})
processor := NewNestedCUDProcessor(db, registry, relProvider)
data := map[string]interface{}{
"label": "option-a",
"aol_rid_actionoption_child": []interface{}{
map[string]interface{}{"label": "link-1"},
},
}
_, err := processor.ProcessNestedCUD(
context.Background(),
"insert",
data,
ActionOption{},
nil,
"action_options",
)
if err != nil {
t.Fatalf("ProcessNestedCUD failed: %v", err)
}
if len(db.insertCalls) < 2 {
t.Fatalf("Expected at least 2 insert calls (parent + child), got %d", len(db.insertCalls))
}
childInsert := db.insertCalls[1]
// The fix: child must receive "rid_actionoption_child", NOT "rid_actionoption".
if childInsert["rid_actionoption_child"] == nil {
t.Error("Expected child to have rid_actionoption_child set (child-side FK column)")
}
if childInsert["rid_actionoption"] != nil {
t.Errorf("Child must not receive parent-side column rid_actionoption, got %v", childInsert["rid_actionoption"])
}
}
// TestProcessNestedCUD_BelongsToUnchanged verifies that the fix does not regress belongsTo
// relations, where ForeignKey is already the local (child) column.
func TestProcessNestedCUD_BelongsToUnchanged(t *testing.T) {
db := newMockDatabase()
registry := &mockModelRegistry{}
relProvider := newMockRelationshipProvider()
// For belongsTo, ForeignKey is the column on the child; References is on the parent.
// The old and new code must behave identically here.
relProvider.RegisterRelation("Employee", "department", &RelationshipInfo{
FieldName: "Department",
JSONName: "department",
RelationType: "belongsTo",
ForeignKey: "DepartmentID", // child's own column
References: "ID", // parent's PK
RelatedModel: Department{},
})
relProvider.RegisterRelation("Department", "employees", &RelationshipInfo{
FieldName: "Employees",
JSONName: "employees",
RelationType: "has_many",
ForeignKey: "DepartmentID",
RelatedModel: Employee{},
})
processor := NewNestedCUDProcessor(db, registry, relProvider)
data := map[string]interface{}{
"name": "Engineering",
"employees": []interface{}{
map[string]interface{}{"name": "Alice"},
},
}
_, err := processor.ProcessNestedCUD(
context.Background(),
"insert",
data,
Department{},
nil,
"departments",
)
if err != nil {
t.Fatalf("ProcessNestedCUD failed: %v", err)
}
if len(db.insertCalls) < 2 {
t.Fatalf("Expected at least 2 inserts, got %d", len(db.insertCalls))
}
// Employees relation uses has_many (old-style) so it goes through the parentIDs injection path,
// not the foreignKeyFieldName path. Just confirm no panic and employee is inserted.
if db.insertCalls[0]["name"] != "Engineering" {
t.Errorf("Expected department name 'Engineering', got %v", db.insertCalls[0]["name"])
}
}
func TestProcessNestedCUD_AddAlias(t *testing.T) {
db := newMockDatabase()
registry := &mockModelRegistry{}
relProvider := newMockRelationshipProvider()
processor := NewNestedCUDProcessor(db, registry, relProvider)
data := map[string]interface{}{
"_request": "add",
"name": "New Department",
}
result, err := processor.ProcessNestedCUD(context.Background(), "insert", data, Department{}, nil, "departments")
if err != nil {
t.Fatalf("ProcessNestedCUD with _request=add failed: %v", err)
}
if result.ID == nil {
t.Error("Expected result.ID to be set after add")
}
if len(db.insertCalls) != 1 {
t.Errorf("Expected 1 insert call, got %d", len(db.insertCalls))
}
}
func TestProcessNestedCUD_RemoveAlias(t *testing.T) {
db := newMockDatabase()
registry := &mockModelRegistry{}
relProvider := newMockRelationshipProvider()
processor := NewNestedCUDProcessor(db, registry, relProvider)
data := map[string]interface{}{
"_request": "remove",
"ID": int64(42),
}
_, err := processor.ProcessNestedCUD(context.Background(), "delete", data, Department{}, nil, "departments")
if err != nil {
t.Fatalf("ProcessNestedCUD with _request=remove failed: %v", err)
}
if len(db.deleteCalls) != 1 {
t.Errorf("Expected 1 delete call, got %d", len(db.deleteCalls))
}
}
func TestProcessNestedCUD_NestedAddRemoveAliases(t *testing.T) {
db := newMockDatabase()
registry := &mockModelRegistry{}
relProvider := newMockRelationshipProvider()
relProvider.RegisterRelation("Department", "employees", &RelationshipInfo{
FieldName: "Employees",
JSONName: "employees",
RelationType: "has_many",
ForeignKey: "DepartmentID",
RelatedModel: Employee{},
})
processor := NewNestedCUDProcessor(db, registry, relProvider)
data := map[string]interface{}{
"ID": int64(1),
"name": "Engineering",
"employees": []interface{}{
map[string]interface{}{"_request": "add", "name": "Alice"},
map[string]interface{}{"_request": "remove", "ID": int64(5)},
},
}
_, err := processor.ProcessNestedCUD(context.Background(), "update", data, Department{}, nil, "departments")
if err != nil {
t.Fatalf("ProcessNestedCUD with nested add/remove failed: %v", err)
}
if len(db.insertCalls) != 1 {
t.Errorf("Expected 1 insert (add alias) for employee, got %d", len(db.insertCalls))
}
if len(db.deleteCalls) != 1 {
t.Errorf("Expected 1 delete (remove alias) for employee, got %d", len(db.deleteCalls))
}
}
func TestGetPrimaryKeyName(t *testing.T) { func TestGetPrimaryKeyName(t *testing.T) {
dept := Department{} dept := Department{}
pkName := reflection.GetPrimaryKeyName(dept) pkName := reflection.GetPrimaryKeyName(dept)
+9
View File
@@ -614,6 +614,15 @@ func extractTableAndColumn(cond string) (table string, column string) {
// Remove any quotes // Remove any quotes
columnRef = strings.Trim(columnRef, "`\"'") columnRef = strings.Trim(columnRef, "`\"'")
// If the left side is a parenthesized subquery (starts with '(' and contains SQL keywords),
// don't attempt prefix extraction from inside it.
if len(columnRef) > 0 && columnRef[0] == '(' {
lowerRef := strings.ToLower(columnRef)
if strings.Contains(lowerRef, "select ") || strings.Contains(lowerRef, " from ") || strings.Contains(lowerRef, " where ") {
return "", ""
}
}
// Check if there's a function call (contains opening parenthesis) // Check if there's a function call (contains opening parenthesis)
openParenIdx := strings.Index(columnRef, "(") openParenIdx := strings.Index(columnRef, "(")
+38 -7
View File
@@ -2011,11 +2011,15 @@ func (h *Handler) processChildRelationsForField(
// Priority: Use foreign key field name if specified, otherwise use parent's PK name // Priority: Use foreign key field name if specified, otherwise use parent's PK name
var foreignKeyFieldName string var foreignKeyFieldName string
if relInfo.ForeignKey != "" { if relInfo.ForeignKey != "" {
// Get the JSON name for the foreign key field in the child model // For has-many/has-one: join:parentCol=childCol
foreignKeyFieldName = reflection.GetJSONNameForField(relatedModelType, relInfo.ForeignKey) // ForeignKey = parent side, References = child side (where we actually set the value)
childField := relInfo.ForeignKey
if (relInfo.RelationType == "hasMany" || relInfo.RelationType == "hasOne") && relInfo.References != "" {
childField = relInfo.References
}
foreignKeyFieldName = reflection.GetJSONNameForField(relatedModelType, childField)
if foreignKeyFieldName == "" { if foreignKeyFieldName == "" {
// Fallback to lowercase field name foreignKeyFieldName = strings.ToLower(childField)
foreignKeyFieldName = strings.ToLower(relInfo.ForeignKey)
} }
} else { } else {
// Fallback: use parent's primary key name // Fallback: use parent's primary key name
@@ -2039,7 +2043,10 @@ func (h *Handler) processChildRelationsForField(
// Process based on relation type and data structure // Process based on relation type and data structure
switch v := relationValue.(type) { switch v := relationValue.(type) {
case map[string]interface{}: case map[string]interface{}:
// Single related object - add parent ID to foreign key field if !isValidNestedRequest(v) {
logger.Debug("Skipping single relation %s - missing or invalid _request value", relationName)
return nil
}
// IMPORTANT: In recursive relationships, don't overwrite the primary key // IMPORTANT: In recursive relationships, don't overwrite the primary key
if parentID != nil && foreignKeyFieldName != "" && foreignKeyFieldName != childPKFieldName { if parentID != nil && foreignKeyFieldName != "" && foreignKeyFieldName != childPKFieldName {
v[foreignKeyFieldName] = parentID v[foreignKeyFieldName] = parentID
@@ -2056,7 +2063,10 @@ func (h *Handler) processChildRelationsForField(
// Multiple related objects // Multiple related objects
for i, item := range v { for i, item := range v {
if itemMap, ok := item.(map[string]interface{}); ok { if itemMap, ok := item.(map[string]interface{}); ok {
// Add parent ID to foreign key field if !isValidNestedRequest(itemMap) {
logger.Debug("Skipping relation array[%d] %s - missing or invalid _request value", i, relationName)
continue
}
// IMPORTANT: In recursive relationships, don't overwrite the primary key // IMPORTANT: In recursive relationships, don't overwrite the primary key
if parentID != nil && foreignKeyFieldName != "" && foreignKeyFieldName != childPKFieldName { if parentID != nil && foreignKeyFieldName != "" && foreignKeyFieldName != childPKFieldName {
itemMap[foreignKeyFieldName] = parentID itemMap[foreignKeyFieldName] = parentID
@@ -2074,7 +2084,10 @@ func (h *Handler) processChildRelationsForField(
case []map[string]interface{}: case []map[string]interface{}:
// Multiple related objects (typed slice) // Multiple related objects (typed slice)
for i, itemMap := range v { for i, itemMap := range v {
// Add parent ID to foreign key field if !isValidNestedRequest(itemMap) {
logger.Debug("Skipping relation typed array[%d] %s - missing or invalid _request value", i, relationName)
continue
}
// IMPORTANT: In recursive relationships, don't overwrite the primary key // IMPORTANT: In recursive relationships, don't overwrite the primary key
if parentID != nil && foreignKeyFieldName != "" && foreignKeyFieldName != childPKFieldName { if parentID != nil && foreignKeyFieldName != "" && foreignKeyFieldName != childPKFieldName {
itemMap[foreignKeyFieldName] = parentID itemMap[foreignKeyFieldName] = parentID
@@ -2095,6 +2108,24 @@ func (h *Handler) processChildRelationsForField(
return nil return nil
} }
// isValidNestedRequest returns true only when the item carries a _request key
// whose value is one of the recognised mutation verbs.
func isValidNestedRequest(item map[string]interface{}) bool {
raw, ok := item["_request"]
if !ok {
return false
}
s, ok := raw.(string)
if !ok {
return false
}
switch strings.ToLower(strings.TrimSpace(s)) {
case "insert", "add", "change", "update", "delete", "remove":
return true
}
return false
}
// getTableNameForRelatedModel gets the table name for a related model. // getTableNameForRelatedModel gets the table name for a related model.
// If the model's TableName() is schema-qualified (e.g. "public.users") the // If the model's TableName() is schema-qualified (e.g. "public.users") the
// separator is adjusted for the active driver: underscore for SQLite, dot otherwise. // separator is adjusted for the active driver: underscore for SQLite, dot otherwise.
+39
View File
@@ -352,6 +352,45 @@ func (m *mockRegistry) GetAllModels() map[string]interface{} {
return m.models return m.models
} }
// TestIsValidNestedRequest verifies that only the allowed _request verbs are accepted
// and that items missing the key are rejected.
func TestIsValidNestedRequest(t *testing.T) {
tests := []struct {
name string
item map[string]interface{}
expected bool
}{
// Valid verbs
{name: "insert", item: map[string]interface{}{"_request": "insert"}, expected: true},
{name: "add", item: map[string]interface{}{"_request": "add"}, expected: true},
{name: "update", item: map[string]interface{}{"_request": "update"}, expected: true},
{name: "change", item: map[string]interface{}{"_request": "change"}, expected: true},
{name: "delete", item: map[string]interface{}{"_request": "delete"}, expected: true},
{name: "remove", item: map[string]interface{}{"_request": "remove"}, expected: true},
// Case-insensitive
{name: "INSERT uppercase", item: map[string]interface{}{"_request": "INSERT"}, expected: true},
{name: "Remove mixed case", item: map[string]interface{}{"_request": "Remove"}, expected: true},
// Whitespace trimmed
{name: "insert with spaces", item: map[string]interface{}{"_request": " insert "}, expected: true},
// Invalid / missing
{name: "missing _request", item: map[string]interface{}{"name": "foo"}, expected: false},
{name: "empty string", item: map[string]interface{}{"_request": ""}, expected: false},
{name: "unknown verb", item: map[string]interface{}{"_request": "create"}, expected: false},
{name: "unknown verb modify", item: map[string]interface{}{"_request": "modify"}, expected: false},
{name: "non-string value", item: map[string]interface{}{"_request": 42}, expected: false},
{name: "nil value", item: map[string]interface{}{"_request": nil}, expected: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isValidNestedRequest(tt.item)
if got != tt.expected {
t.Errorf("isValidNestedRequest(%v) = %v, want %v", tt.item, got, tt.expected)
}
})
}
}
// TestMultiLevelRelationExtraction tests extracting deeply nested relations // TestMultiLevelRelationExtraction tests extracting deeply nested relations
func TestMultiLevelRelationExtraction(t *testing.T) { func TestMultiLevelRelationExtraction(t *testing.T) {
registry := &mockRegistry{ registry := &mockRegistry{