CLOUD-3JK8 - #951
Conversation
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesNested query parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR normalizes nested query values and converts invalid inputs into query validation errors; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR updates logical-query parsing to accept JSON-encoded child queries and convert invalid nested scalar values into catchable query exceptions.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "fix: enhance query parsing for JSON stri..." | Re-trigger Greptile |
The bug
Query::parseQuery()'s logical-operator branch (or/and/elemMatch) assumed every nested value is a decoded array:
foreach ($values as $index => $value) {
$values[$index] = self::parseQuery($value); // $value can be a string → TypeError
}
When a client sends nested queries double-encoded (each child as a JSON string, e.g. {"method":"or","values":["{"method":"equal"...}"]}) — which some SDKs do — $value is a string.
Passing it to parseQuery(array $query) throws a TypeError, not a QueryException. Appwrite's catch (QueryException) in Documents/XList.php:107 doesn't catch it, so it escaped as an
uncaught 500.
The fix (src/Database/Query.php)
Normalize nested values — decode strings, recurse arrays, and reject anything else as a clean QueryException:
if (\is_string($value)) {
$values[$index] = self::parse($value);
} elseif (\is_array($value)) {
$values[$index] = self::parseQuery($value);
} else {
throw new QueryException('Invalid nested query. Must be an array or string, got ' . \gettype($value));
}
(→ 500).
Summary by CodeRabbit
Bug Fixes
Tests