neo4j / neo4j-javascript-driver

Neo4j Bolt driver for JavaScript
https://neo4j.com/docs/javascript-manual/current/
Apache License 2.0
853 stars 148 forks source link

Cannot run a query looking for a dynamic label #249

Closed danilovergara closed 7 years ago

danilovergara commented 7 years ago

I'm trying to make a query with a dynamic label as follows:

let startQuery = 'MATCH (p)-[r]->(n) \
    WHERE n:{elementType} \
    RETURN n';
let startParams = {
    elementType: elementType
};
session.run(startQuery, startParams)
.then((result) => {
    session.close();
    let node = result.records[0].get(0);
    resolve(node);
}).catch((err) => {
    console.log(err);
});

But when I execute this code, it throws the next error:

{ Error: Invalid input '{': expected whitespace or a label name (line 1, column 31 (offset: 30))
"MATCH (p)-[r]->(n)             WHERE n:{elementType}           RETURN n"
                               ^
    at new Neo4jError (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/error.js:76:132)
    at newError (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/error.js:66:10)
    at Connection._handleMessage (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/connector.js:354:56)
    at Dechunker.Connection._dechunker.onmessage (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/connector.js:285:12)
    at Dechunker._onHeader (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/chunking.js:246:14)
    at Dechunker.AWAITING_CHUNK (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/chunking.js:199:21)
    at Dechunker.write (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/chunking.js:257:28)
    at NodeChannel.self._ch.onmessage (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/connector.js:258:27)
    at TLSSocket.<anonymous> (/var/www/bpmnengine/node_modules/neo4j-driver/lib/v1/internal/ch-node.js:308:16)
    at emitOne (events.js:96:13)
    at TLSSocket.emit (events.js:191:7)
    at readableAddChunk (_stream_readable.js:178:18)
    at TLSSocket.Readable.push (_stream_readable.js:136:10)
    at TLSWrap.onread (net.js:561:20) code: 'Neo.ClientError.Statement.SyntaxError' }

I've also tried replacing the variable on the node as follows:

let startQuery = 'MATCH (p)-[r]->(n:{elementType}) \
    RETURN n';

But as the first case, it throws the same error.

lutovich commented 7 years ago

Hi @danilovergara,

Labels can't be parameterized in Cypher. This is a limitation of the language not the driver itself. It is only possible to parameterize property values. So you could simply use string concatenation or template string.

See this issue for more details: https://github.com/neo4j/neo4j/issues/4334

pirate commented 3 years ago

Is there any chance of this design being reconsidered @lutovich?

Maybe this can be fixed within the JS adapter layer itself before the query gets sent to bolt, postponing the need to solve the much harder language-level limitation.

In particular for JS having to use template strings for the label names everywhere is both quite cumbersome and dangerous (it allows injection attacks) when all the rest of the parameters can be passed via auto-sanitized values object.

An example of our current flawed implementation with Cypher injection exposure:

const unsanitized_edges_sent_from_frontend = [
    {
        type: 'payout',
        src: 'user::someArtist456',
        dst: 'payoutMethod::someStripeCustomer324',
    },
    {
        type: 'subscription',
        src: 'user::someListener123',
        dst: 'channel::someChannel123',
    },
]

for (const edge of unsanitized_edges_sent_from_frontend) {
    const result = await session.run(
        `
            MATCH (src:${edge.src.split('::')[0]} {uuid: $src})
            MATCH (dst:${edge.dst.split('::')[0]} {uuid: $dst})
            MERGE (src)-[edge:${edge.type.toUpperCase()}]->(dst)
            RETURN edge
        `,
        {
            // would be great if src:type, dst:type, and edge:type could be passed here instead
            // to avoid injection attacks via frontend-provided node/edge type labels
            src: edge.src,
            dst: edge.dst,
        }
    )
}
lutovich commented 3 years ago

Hey @pirate, I no longer work on the Neo4j JavaScript driver. Perhaps @nigelsmall could provide an opinion about this? :)

pirate commented 3 years ago

Ah sorry to bother you then @lutovich and thanks for the referral. I think we will use https://github.com/adam-cowley/neode for now, but I believe it would still be useful to many people if this were implemented in neo4j-javascript-driver at some point :)

technige commented 3 years ago

@pirate This isn't a driver-level concern, and we would absolutely not add that logic to the driver, since it would require an entire Cypher lexer to be added on the client side. The right place to request this behaviour would be on the Neo4j repository itself, although I know this has already been under discussion several times to date.

FWIW I (personally) completely agree about the potential for injection attacks. This is a good justification for such a capability to be added to Cypher.