Skip to content

CLOUD-3JK8 - #951

Merged
abnegate merged 2 commits into
mainfrom
fix-query-validation-on-json-object
Aug 27, 2026
Merged

CLOUD-3JK8#951
abnegate merged 2 commits into
mainfrom
fix-query-validation-on-json-object

Conversation

@ArnabChatterjee20k

@ArnabChatterjee20k ArnabChatterjee20k commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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

    • Improved handling of nested logical queries provided as JSON strings or structured values.
    • Invalid nested query value types now return a clear query error instead of causing an unexpected runtime failure.
  • Tests

    • Added coverage for serialized nested queries and validation of invalid nested values.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 55 minutes.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1eceb802-c79e-4b5b-b199-fa0b2734a212

📥 Commits

Reviewing files that changed from the base of the PR and between be5cae2 and cd2493a.

📒 Files selected for processing (1)
  • tests/unit/QueryTest.php

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e87f6ade-ba4f-48de-836c-baaddabbf996

📥 Commits

Reviewing files that changed from the base of the PR and between 8c0f55a and be5cae2.

📒 Files selected for processing (2)
  • src/Database/Query.php
  • tests/unit/QueryTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Query::parseQuery now parses nested logical query values from JSON strings or arrays and throws QueryException for unsupported types. Unit tests cover string parsing and integer rejection.

Changes

Nested query parsing

Layer / File(s) Summary
Parse and validate nested logical values
src/Database/Query.php, tests/unit/QueryTest.php
parseQuery parses string children with parse(), array children with parseQuery(), and rejects other types with QueryException. Tests verify decoded child queries and the integer error message.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to be5ca

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is a ticket identifier, but it does not describe the change to nested logical query parsing or improved validation. Use a concise descriptive title, such as "Handle nested string values in logical queries".
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-query-validation-on-json-object

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR updates logical-query parsing to accept JSON-encoded child queries and convert invalid nested scalar values into catchable query exceptions.

  • Routes string children through the existing JSON query parser while preserving array-child recursion.
  • Adds unit coverage for serialized logical-query children and invalid nested value types.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/Database/Query.php Normalizes string and array children into Query objects and rejects unsupported nested types with QueryException.
tests/unit/QueryTest.php Covers double-encoded logical-query children and verifies invalid scalar children produce a clean QueryException.

Reviews (2): Last reviewed commit: "fix: enhance query parsing for JSON stri..." | Re-trigger Greptile

@abnegate
abnegate merged commit 704dd6e into main Aug 27, 2026
39 of 40 checks passed
@abnegate
abnegate deleted the fix-query-validation-on-json-object branch August 27, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants