Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-1354

Include all static enum imports in request validation for bindings

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Done
    • Affects Version/s: 3.1.2-incubating
    • Fix Version/s: 3.1.3, 3.2.1
    • Component/s: server
    • Labels:

      Description

      Gremlin Server validates the bindings of incoming requests and returns an error for any reserved terms. The list of reserved terms only includes T but should also include others like Order and Scope so that users don't inadvertently override them with a request binding. When that happens Gremlin Server throws a not so easy to understand error.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user spmallette opened a pull request:

          https://github.com/apache/tinkerpop/pull/354

          TINKERPOP-1354 Added more invalid binding keys to Gremlin Server validation

          https://issues.apache.org/jira/browse/TINKERPOP-1354

          These "invalid" keys are reserved terms for Gremlin Server as they are statically imported enums and shouldn't be used as binding keys. You get some less than easy to understand error messages if those keys are used.

          I would have CTR'd but wanted to see if anyone had other suggestions for additional validations at play. Also, this change is "breaking" in the sense that users who were somehow successfully using some of these newly reserved keys on previous versions (not fully sure if that was even possible) will have to update their code. I don't think this is a massive problem for someone to fix, so while "breaking" it doesn't seem massively detrimental and shouldn't be widely problematic.

          Builds with `mvn clean install -DskipTests && mvn verify -pl gremlin-server -DskipIntegrationTests=false`

          VOTE +1

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/apache/tinkerpop TINKERPOP-1354

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/tinkerpop/pull/354.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #354


          commit c37c3016bd05b3caeb09084f5469c606c439bfe5
          Author: Stephen Mallette <spmva@genoprime.com>
          Date: 2016-06-30T17:07:44Z

          Added more invalid binding keys to Gremlin Server OpProcessor validation.

          These "invalid" keys are reserved terms for Gremlin Server as they are statically imported enums and shouldn't be used as binding keys. You get some less than easy to understand error messages if those keys are used.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/tinkerpop/pull/354 TINKERPOP-1354 Added more invalid binding keys to Gremlin Server validation https://issues.apache.org/jira/browse/TINKERPOP-1354 These "invalid" keys are reserved terms for Gremlin Server as they are statically imported enums and shouldn't be used as binding keys. You get some less than easy to understand error messages if those keys are used. I would have CTR'd but wanted to see if anyone had other suggestions for additional validations at play. Also, this change is "breaking" in the sense that users who were somehow successfully using some of these newly reserved keys on previous versions (not fully sure if that was even possible) will have to update their code. I don't think this is a massive problem for someone to fix, so while "breaking" it doesn't seem massively detrimental and shouldn't be widely problematic. Builds with `mvn clean install -DskipTests && mvn verify -pl gremlin-server -DskipIntegrationTests=false` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1354 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/354.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #354 commit c37c3016bd05b3caeb09084f5469c606c439bfe5 Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-06-30T17:07:44Z Added more invalid binding keys to Gremlin Server OpProcessor validation. These "invalid" keys are reserved terms for Gremlin Server as they are statically imported enums and shouldn't be used as binding keys. You get some less than easy to understand error messages if those keys are used.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user robertdale commented on the issue:

          https://github.com/apache/tinkerpop/pull/354

          BUILD SUCCESS

          Show
          githubbot ASF GitHub Bot added a comment - Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/354 BUILD SUCCESS
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dkuppitz commented on the issue:

          https://github.com/apache/tinkerpop/pull/354

          VOTE: +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/354 VOTE: +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user okram commented on the issue:

          https://github.com/apache/tinkerpop/pull/354

          VOTE +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/354 VOTE +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tinkerpop/pull/354

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/354

            People

            • Assignee:
              spmallette stephen mallette
              Reporter:
              spmallette stephen mallette
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development