infobloxopen / atlas-app-toolkit

This repository provides common Go utilities and helpers that are reusable from project-to-project. The goal is to prevent code duplication by encouraging teams to use and contribute to toolkit libraries. The toolkit is not a framework. Rather, it is a set of (mostly gRPC-related) plugins and helpers.
Apache License 2.0
99 stars 115 forks source link

Resolve panic `reflect.Value.Interface on zero value` #396

Closed davidhankins closed 9 months ago

davidhankins commented 9 months ago

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).