Azure / azure-cosmos-dotnet-v2

Contains samples and utilities relating to the Azure Cosmos DB .NET SDK
MIT License
577 stars 838 forks source link

graph - unable to use bind variables in gremlin statements #300

Open ghost opened 7 years ago

ghost commented 7 years ago

Hi,

unable to use bind variables in gremlin statements

this makes

Regards,

Bart,

ScottHolden commented 7 years ago

This is also quickly becoming a blocker for using CosmosDB in a production environment, any updates?

ashwini-ms commented 7 years ago

Hi Bart,

Which client are you using, gremlin console or a client app?

Thanks Ashwini

ghost commented 7 years ago

Hi, I am using

It is my experience that in this problem is rather common in the Graph API and applications themselves must implement data-sanitization.

Regards, Bart,

ghost commented 7 years ago

I recommend to keep onder the category of "nice to have" in a product backlog - in relative perspective and would be ok to close

Regards, Bart,

ashwini-ms commented 7 years ago

Hi Bart,

Cosmos-DB graph service does supports variable binding as part of gremlin requests. And, some of the clients does support that.

Here are some references to the documentation:

Gremlin.Net:

You can provide the bindings as part of request arguments. https://florianhockmann.github.io/Gremlin.Net/api/Gremlin.Net.Messages.ScriptRequestArguments.html

Nodejs:

We can use optional parameter to provide variable bindings. buildMessage(rawScript, rawBindings = {}, baseMessage = {}) https://github.com/jbmusso/gremlin-javascript/blob/master/src/GremlinClient.js

Java:

public ResultSet submit(String gremlin, Map<String,Object> parameters) http://tinkerpop.apache.org/javadocs/current/full/org/apache/tinkerpop/gremlin/driver/Client.html#submit-java.lang.String-java.util.Map

We understand that binding parameters is not support on Microsoft.Azure.Graph. One of the backlog items and we are working on this.

Hope this helps :)

ghost commented 7 years ago

Hi Ashwini,

Thank you for the feedback. I tried Gremlin.Net.

I found an example at https://stackoverflow.com/questions/44473303/how-to-prevent-gremlin-injection-in-c

I conclude that variables are not prefixed (ie with ":").

I tested several strings and found some that failed.

string strGremlin = @"
                g.addV('Organization').property('id', 'I').property('@id', 'http://www.patentsview.org/api/assignee/I').property('@type', 'Organization').property(list, 'assignee_organization', 'N.V.').property(list, 'assignee_id', 'I').property(list, 'assignee_type', '3').property('@context', Context)
            ";

string strContext = @"{this is ""quoted"" text} ";

            Dictionary<string, object> dictBinds = new Dictionary<string, object>
            {
                {"Context", strContext}
            };
// OK
            string strGremlin = @"
                g.addV('Organization').property('id', 'I').property('@id', 'http://www.patentsview.org/api/assignee/I').property('@type', 'Organization').property(list, 'assignee_organization', 'N.V.').property(list, 'assignee_id', 'I').property(list, 'assignee_type', '3').property('@context', Context)
            ";

            string strContext = @"           {\""xsd\"":\""http://www.w3.org/2001/XMLSchema#\"",\""Organization\"":\""http://schema.org/Organization\"",\""assignee_type\"":{\""@id\"":\""http://www.patentsview.org/api/assignee/assignee_type\"",\""@type\"":\""xsd:integer\""},\""assignee_organization\"":\""http://schema.org/name\"",\""assignee_id\"":{\""@id\"":\""http://www.patentsview.org/api/assignee/assignee_id\""}}

            Dictionary<string, object> dictBinds = new Dictionary<string, object>
            {
                {"Context", strContext}
            };
            // NOT OK
            string strGremlin = @"
                g.addV('Organization').property('id', 'I').property('@id', 'http://www.patentsview.org/api/assignee/I').property('@type', 'Organization').property(list, 'assignee_organization', 'N.V.').property(list, 'assignee_id', 'I').property(list, 'assignee_type', '3').property('@context', Context)
            ";

            string strContext = @"           {""xsd"":""http://www.w3.org/2001/XMLSchema#"",""Organization"":""http://schema.org/Organization"",""assignee_type"":{""@id"":""http://www.patentsview.org/api/assignee/assignee_type"",""@type"":""xsd:integer""},""assignee_organization"":""http://schema.org/name"",""assignee_id"":{""@id"":""http://www.patentsview.org/api/assignee/assignee_id""}}

            Dictionary<string, object> dictBinds = new Dictionary<string, object>
            {
                {"Context", strContext}
            };
            // NOT OK

normally the data is to be injected as is.

this may be related to https://github.com/Azure/azure-documentdb-dotnet/issues/315

although by definition (bind variables) it should not.

Regards,

Bart,

ashwini-ms commented 7 years ago

@bartdotnet Thanks for reporting the issue with strings containing ":". We are working on this. We will update the thread once we have more details.

ashwini-ms commented 7 years ago

@bartdotnet, closing this issue since we have a way to bind variables.

We will track the string with ":" issue along with #315 . Please feel free to reach out if you have any concerns.

@ScottHolden , Let us know if you have issues with binding variables and we will be happy to help.

ghost commented 7 years ago

@ashwini-ms

In theory using bind variables would have allowed me to avoid the issue explained in https://github.com/Azure/azure-documentdb-dotnet/issues/315 but rather astonishingly it does not because it has other issues.

My point is that using bind variables does not solve the issue, because I think its implementation has an issue (see Examples marked "NOT OK"). What bind variables mean is that data should be stored as-is and non-parsed. It should allow me - within reasonable limitations - to put whatever I want in a property.

Also, bind variables are there to allow the above and also safeguard against malicious content Assuming the client - here Gremlin.Net - just passes on the message but then via a bind variable. I assume the problem is the server-side code specifically to managing bind variables.

Hence I would suggest to treat this issue as a related but not just a similar issue in regards to the potential security perspective alone.

Hence also, I suggest to re-open the issue for use in an OLTP context but to worry less from a semantic graph database perspective. The Use cases may have other needs. It depends what the product team is aiming for.

Regards, Bart,

ashwini-ms commented 7 years ago

Hi @bartdotnet, I agree binding variables should allow us to replace the string as-is. But, the issue is not related to binding variables. This is the known issue in our parser to handle strings with characters like quotes, colon, etc. We are working on the fix and it should be resolved soon.

The reason I wanted to track this with #315 because both are related to the underline gremlin parser. I am happy to reopen if you want :).

Thanks Ashwini

ashwini-ms commented 7 years ago

We are looking into the issue. ETA: end of this year.

syedhassaanahmed commented 6 years ago

@ashwini-ms We have other customers facing this issue. Any updates on this?

ephraimlambarte commented 3 years ago

any updates?

friyantismile commented 3 years ago

any updates?