Skip to content

fix(server): support cypher bound queries safely - #3289

Open
imbajin wants to merge 4 commits into
apache:masterfrom
hugegraph:codex/cypher-minimal-compat
Open

imbajin wants to merge 4 commits into
apache:masterfrom
hugegraph:codex/cypher-minimal-compat

Conversation

@imbajin

@imbajin imbajin commented Oct 7, 2026

Copy link
Copy Markdown
Member

Purpose of the PR

Cypher POST previously accepted only raw query text, missing parameters could silently become null,
and a failed write could leave changes visible after later reads. This change accepts validated JSON
bindings and rolls back traversal failures on the thread that owns the transaction.

This is a standalone change on current Apache master, which now includes the Java 17 / TinkerPop 3.8.1
upgrade. The translator remains translation:1.0.4. Related organization PR:
hugegraph#238. Paired website documentation:
apache/hugegraph-doc#499; coordinate its merge with this change.

Main Changes

  • Preserve GET and legacy raw POST under application/json; accept
    { "cypher": "RETURN $name AS name", "parameters": { "name": "marko" } }.
    Reject malformed/non-object JSON and unsupported text/plain requests.
  • Keep values separate from query text, reject missing bindings and the translator's reserved null
    marker, and retain binding type/count validation. Real null and parameter names such as id remain valid.
  • Commit before the terminal success response; roll back and close the traversal on its execution
    thread after failures, including Java errors, and send a terminal failure response.
  • Preserve the merged REST client's native charset handling while recognizing JSON media types with
    charset parameters. Release Gremlin HTTP requests exactly once on success and error paths.
query + parameters -> validation -> traversal worker
                                   success: commit -> response
                                   failure: rollback -> error response

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests.
  • Need tests and can be verified as follows:
    • Java 17, TinkerPop 3.8.1, translator 1.0.4, and a live RocksDB-backed Server.
    • Cypher request, binding, regex, routing and write/rollback regression tests, including native readback.
    • Real-executor fatal-error response/rollback tests; Gremlin HTTP request ownership and auth-context tests.
    • REST JSON/map charset tests with UTF-8/UTF-16, gzip and an ASCII-default JVM.
    • Related Gremlin and Login API regressions, formatting, root clean compile and reactor installation.

All selected regressions, formatting, root clean compile and installation passed. Gremlin API retained
one existing non-shared-backend skip; Cypher tests had no skips. Advanced Cypher constructs, HStore, Bolt,
cross-request transactions and performance remain outside the verified scope. The compatibility note
also records the translator's reserved null-marker limitation for literals and stored properties.

Does this PR potentially affect the following parts?

  • Dependencies
  • Modify configurations
  • The public API
  • Other affects
  • Nope

Documentation Status

  • Doc - TODO: required documentation is pending; complete it before merging.
  • Doc - Done: documentation is included here or linked below.
  • Doc - No Need: no user-visible documentation is affected.

Repository documentation: docs/cypher-compatibility.md, the Server README, Commons README and
docs/upgrade-tinkerpop-3.8.md. Paired website PR: apache/hugegraph-doc#499.

- accept parameterized JSON and preserve legacy requests
- validate bindings and roll back failed traversal transactions
- verify core behavior on Java 17, TinkerPop 3.8.1, and RocksDB; document limits
- default text and JSON request bodies to UTF-8
- preserve explicitly declared media type encodings
- verify real request bytes including gzip in an ASCII JVM
- document charset behavior independent of platform defaults
Own HTTP request lifetime across TinkerPop error paths.
Preserve custom channelizers and WebSocket behavior.
Cover keepalive, early failures and reference counts.
Document the scoped TinkerPop compatibility handler.
- reject invalid JSON and reserved translator values
- roll back fatal errors and send terminal responses
- preserve JSON serialization with declared charsets
- align regression fixtures and compatibility notes
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.29730% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.76%. Comparing base (e62c961) to head (fd8f4a6).

Files with missing lines Patch % Lines
...apache/hugegraph/opencypher/CypherOpProcessor.java 64.55% 21 Missing and 7 partials ⚠️
.../org/apache/hugegraph/api/cypher/CypherClient.java 61.53% 1 Missing and 4 partials ⚠️
...ava/org/apache/hugegraph/api/cypher/CypherAPI.java 90.32% 0 Missing and 3 partials ⚠️
...he/hugegraph/server/HttpGremlinRequestHandler.java 72.72% 2 Missing and 1 partial ⚠️
...rg/apache/hugegraph/auth/ContextGremlinServer.java 75.00% 0 Missing and 1 partial ⚠️
...ugegraph/server/HugeGraphWsAndHttpChannelizer.java 90.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3289      +/-   ##
============================================
+ Coverage     41.19%   41.76%   +0.56%     
- Complexity     6773     6924     +151     
============================================
  Files           766      768       +2     
  Lines         66086    66205     +119     
  Branches       8773     8798      +25     
============================================
+ Hits          27225    27651     +426     
+ Misses        35818    35435     -383     
- Partials       3043     3119      +76     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant