From 4b13996dcaae1acc1f824bfdaf75e4bef755f358 Mon Sep 17 00:00:00 2001
From: "David W. Hankins" <dhankins@infoblox.com>
Date: Tue, 14 Nov 2023 13:20:56 -0800
Subject: [PATCH] Resolve panic `reflect.Value.Interface on zero value` (#396)

PROBLEM
=======

In HTTP API services, a user may specify a `_filter=` specifying a
field name and an empty string (`field == ""`).

The `ToORM()` codec translates the empty string to a `nil` pointer.

`ProcessStringCondition()` then attempts to take a
`reflect.Value.Interface` on this nil pointer, causing a panic.

SOLUTION
========

Detect the nil pointer value, and return a nil value. The gorm codec
translates this to a `NULL` value in SQL syntax. This resolves the
panic, but results in an SQL query that matches no rows
(`field == NULL`).

DISCUSSION
==========

It's arguable what the user intent is here, considering that there is
existing Atlas filter syntax for `field == null` (resulting in the
correct identity SQL expression `field IS NULL`). The explicit empty
string value cannot exist in our interpretation because the codec
translates it to NULL reliably; a user that POSTs an object with a
string field set `""` will result in a table row set to NULL.

But we might want to consider that `field == ""` should be an alias for
the NULL identity expression; the user intent is probably to find rows
where the field is `""` in their experience of the object.

From one point of view, it is correct that there are never any rows in
our table with fields of `""` value. From another point of view, there
are rows in our table with fields not equal to `""` value and that query
should have results (the `field != ""` syntax seems like it should work).
---
 gorm/converter.go | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gorm/converter.go b/gorm/converter.go
index 001f5461..fd49b927 100644
--- a/gorm/converter.go
+++ b/gorm/converter.go
@@ -175,6 +175,12 @@ func (converter *DefaultFilteringConditionConverter) StringConditionToGorm(ctx c
 		return converter.insensitiveCaseStringConditionToGorm(neg, dbName, o), []interface{}{value}, assocToJoin, nil
 	}
 
+	// N.B. if the user specifies a value that the codec translates to NULL
+	// (e.g. `field1 == ""` for string columns) instead of using the explicit
+	// support for identity (`field1 == null`), the results of this syntax may
+	// not match user expectations - `(col_name = NULL)` will match no rows,
+	// not even rows with NULL values. Did the user intend to match rows with
+	// NULL values (`field1 IS NULL`)?
 	return fmt.Sprintf("%s(%s %s ?)", neg, dbName, o), []interface{}{value}, assocToJoin, nil
 }
 
@@ -226,6 +232,14 @@ func (p *DefaultFilteringConditionProcessor) ProcessStringCondition(ctx context.
 				if !ormId.IsValid() {
 					return nil, fmt.Errorf("Cannot find field %s in %s", part, objType)
 				}
+				// For type values where the codec translates a NULL value in
+				// SQL, we receive a pointer of nil value. E.g. `""`.
+				switch ormId.Kind() {
+				case reflect.Ptr, reflect.UnsafePointer:
+					if ormId.IsNil() {
+						return nil, nil
+					}
+				}
 				return reflect.Indirect(ormId).Interface(), nil
 
 			}