You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Alexey Kuznetsov <ak...@gridgain.com> on 2015/11/04 10:13:15 UTC

HTTP-REST sql query ID problem

Igniters,

I found that we have following problems with HTTP-REST sql query API.

After user execute sql query he will receive queryId to be able to fetch
next page.
See docs: https://apacheignite.readme.io/docs/rest-api#sql-query-execute

And current implementation of queryId is a long that simply incremented.

First problem:
  1. client1 execute query and get queryId = 1.
  2. node where query was executed is restarted (queryId generator
initialized to zero).
  3. client2 execute some query and also get queryId=1.
  4. client1 fetch next page for queryId=1 and GETS results of client2!!!!

Second problem:
  As queryId is generated sequentially it is very easy to brute force and
some client could get data of other clients too easy.

What we could do:
 1) Add nodeId to execute sql query response and fetch next page should
pass queryId + nodeId to get next page.
 2) Generate queryId as random long.

OR

Generate queryId as random UUID in this case it will be globally random, no
need for nodeId.

But I'm afraid this will break backward compatibility.

Thoughts?

-- 
Alexey Kuznetsov
GridGain Systems
www.gridgain.com

Re: HTTP-REST sql query ID problem

Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Wed, Nov 4, 2015 at 1:27 AM, Alexey Kuznetsov <ak...@gridgain.com>
wrote:

> Dmitriy,
>
> I like idea of passing clientID, how it should look? UUID?
>

I think Michael has suggested similar approach. I am not sure we need to
pass the clientID with the message, given that we should know who sent the
message anyway. ClientID can be the ID of the client node, which is UUID.

D.


>
> On Wed, Nov 4, 2015 at 4:16 PM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
> > I am assuming we are storing the query IDs per client on the server side,
> > right? How about storing client ID on the server side as well, together
> > with the query-id and returning an error whenever the clientID does not
> > match?
> >
> > On Wed, Nov 4, 2015 at 1:13 AM, Alexey Kuznetsov <
> akuznetsov@gridgain.com>
> > wrote:
> >
> > > Igniters,
> > >
> > > I found that we have following problems with HTTP-REST sql query API.
> > >
> > > After user execute sql query he will receive queryId to be able to
> fetch
> > > next page.
> > > See docs:
> https://apacheignite.readme.io/docs/rest-api#sql-query-execute
> > >
> > > And current implementation of queryId is a long that simply
> incremented.
> > >
> > > First problem:
> > >   1. client1 execute query and get queryId = 1.
> > >   2. node where query was executed is restarted (queryId generator
> > > initialized to zero).
> > >   3. client2 execute some query and also get queryId=1.
> > >   4. client1 fetch next page for queryId=1 and GETS results of
> > client2!!!!
> > >
> > > Second problem:
> > >   As queryId is generated sequentially it is very easy to brute force
> and
> > > some client could get data of other clients too easy.
> > >
> > > What we could do:
> > >  1) Add nodeId to execute sql query response and fetch next page should
> > > pass queryId + nodeId to get next page.
> > >  2) Generate queryId as random long.
> > >
> > > OR
> > >
> > > Generate queryId as random UUID in this case it will be globally
> random,
> > no
> > > need for nodeId.
> > >
> > > But I'm afraid this will break backward compatibility.
> > >
> > > Thoughts?
> > >
> > > --
> > > Alexey Kuznetsov
> > > GridGain Systems
> > > www.gridgain.com
> > >
> >
>
>
>
> --
> Alexey Kuznetsov
> GridGain Systems
> www.gridgain.com
>

Re: HTTP-REST sql query ID problem

Posted by Alexey Kuznetsov <ak...@gridgain.com>.
Dmitriy,

I like idea of passing clientID, how it should look? UUID?

On Wed, Nov 4, 2015 at 4:16 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:

> I am assuming we are storing the query IDs per client on the server side,
> right? How about storing client ID on the server side as well, together
> with the query-id and returning an error whenever the clientID does not
> match?
>
> On Wed, Nov 4, 2015 at 1:13 AM, Alexey Kuznetsov <ak...@gridgain.com>
> wrote:
>
> > Igniters,
> >
> > I found that we have following problems with HTTP-REST sql query API.
> >
> > After user execute sql query he will receive queryId to be able to fetch
> > next page.
> > See docs: https://apacheignite.readme.io/docs/rest-api#sql-query-execute
> >
> > And current implementation of queryId is a long that simply incremented.
> >
> > First problem:
> >   1. client1 execute query and get queryId = 1.
> >   2. node where query was executed is restarted (queryId generator
> > initialized to zero).
> >   3. client2 execute some query and also get queryId=1.
> >   4. client1 fetch next page for queryId=1 and GETS results of
> client2!!!!
> >
> > Second problem:
> >   As queryId is generated sequentially it is very easy to brute force and
> > some client could get data of other clients too easy.
> >
> > What we could do:
> >  1) Add nodeId to execute sql query response and fetch next page should
> > pass queryId + nodeId to get next page.
> >  2) Generate queryId as random long.
> >
> > OR
> >
> > Generate queryId as random UUID in this case it will be globally random,
> no
> > need for nodeId.
> >
> > But I'm afraid this will break backward compatibility.
> >
> > Thoughts?
> >
> > --
> > Alexey Kuznetsov
> > GridGain Systems
> > www.gridgain.com
> >
>



-- 
Alexey Kuznetsov
GridGain Systems
www.gridgain.com

Re: HTTP-REST sql query ID problem

Posted by Dmitriy Setrakyan <ds...@apache.org>.
I am assuming we are storing the query IDs per client on the server side,
right? How about storing client ID on the server side as well, together
with the query-id and returning an error whenever the clientID does not
match?

On Wed, Nov 4, 2015 at 1:13 AM, Alexey Kuznetsov <ak...@gridgain.com>
wrote:

> Igniters,
>
> I found that we have following problems with HTTP-REST sql query API.
>
> After user execute sql query he will receive queryId to be able to fetch
> next page.
> See docs: https://apacheignite.readme.io/docs/rest-api#sql-query-execute
>
> And current implementation of queryId is a long that simply incremented.
>
> First problem:
>   1. client1 execute query and get queryId = 1.
>   2. node where query was executed is restarted (queryId generator
> initialized to zero).
>   3. client2 execute some query and also get queryId=1.
>   4. client1 fetch next page for queryId=1 and GETS results of client2!!!!
>
> Second problem:
>   As queryId is generated sequentially it is very easy to brute force and
> some client could get data of other clients too easy.
>
> What we could do:
>  1) Add nodeId to execute sql query response and fetch next page should
> pass queryId + nodeId to get next page.
>  2) Generate queryId as random long.
>
> OR
>
> Generate queryId as random UUID in this case it will be globally random, no
> need for nodeId.
>
> But I'm afraid this will break backward compatibility.
>
> Thoughts?
>
> --
> Alexey Kuznetsov
> GridGain Systems
> www.gridgain.com
>

Re: HTTP-REST sql query ID problem

Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Wed, Nov 4, 2015 at 10:50 PM, Sergi Vladykin <se...@gmail.com>
wrote:

> Guys,
>
> It looks like that if we need state on server side it is not REST anymore.
> We really need user sessions.
> Everything else you will invent here is just a hack.
>

Sergi, we already support nodeLocal state per query with REST. Not ideal,
but it works. Alexey K. noticed a bug in this implementation. All we need
to do is pick one of many proposed solutions and fix it.


>
> Sergi
>
> 2015-11-05 1:31 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
>
> > On Wed, Nov 4, 2015 at 2:46 AM, Yakov Zhdanov <yz...@apache.org>
> wrote:
> >
> > > We can use IgniteUuid qryId = IgniteUuid.fromUuid(clientId);
> > >
> > > and restrict page requests for queries submitted from other clients.
> I.e.
> > > throw exception if qryId.globalId() != clientId
> > >
> >
> > Yakov, I think it is inconvenient to pass UUID in a query string. How
> about
> > we use node order, defined by ClusterNode.order() method? Essentially,
> > instead of passing just the queryID, REST client will also pass the
> > nodeOrder parameter.
> >
> > On the server side, we check that the received node order should be equal
> > to the local node order. If not, then error. This approach will have the
> > same behavior we do right now, and will also fix the bug mentioned by
> > Alexey.
> >
> >
> > > --Yakov
> > >
> > > 2015-11-04 12:21 GMT+03:00 endian675 <en...@gmail.com>:
> > >
> > > > Alexey,
> > > >
> > > > No problem, here is a link that is relatively simple to understand :
> > > >
> > > >
> > >
> >
> http://javarevisited.blogspot.co.uk/2011/02/fix-protocol-session-or-admin-messages.html
> > > >
> > > > However, a simplified approach of just adding a client ID seems
> > > sufficient
> > > > -
> > > > the sequence number reset functionality of FIX is overly complex for
> > this
> > > > requirement.
> > > >
> > > > Regards
> > > > Mike
> > > >
> > > >
> > > >
> > > > --
> > > > View this message in context:
> > > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/HTTP-REST-sql-query-ID-problem-tp4524p4531.html
> > > > Sent from the Apache Ignite Developers mailing list archive at
> > > Nabble.com.
> > > >
> > >
> >
>

Re: HTTP-REST sql query ID problem

Posted by Sergi Vladykin <se...@gmail.com>.
Guys,

It looks like that if we need state on server side it is not REST anymore.
We really need user sessions.
Everything else you will invent here is just a hack.

Sergi

2015-11-05 1:31 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:

> On Wed, Nov 4, 2015 at 2:46 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>
> > We can use IgniteUuid qryId = IgniteUuid.fromUuid(clientId);
> >
> > and restrict page requests for queries submitted from other clients. I.e.
> > throw exception if qryId.globalId() != clientId
> >
>
> Yakov, I think it is inconvenient to pass UUID in a query string. How about
> we use node order, defined by ClusterNode.order() method? Essentially,
> instead of passing just the queryID, REST client will also pass the
> nodeOrder parameter.
>
> On the server side, we check that the received node order should be equal
> to the local node order. If not, then error. This approach will have the
> same behavior we do right now, and will also fix the bug mentioned by
> Alexey.
>
>
> > --Yakov
> >
> > 2015-11-04 12:21 GMT+03:00 endian675 <en...@gmail.com>:
> >
> > > Alexey,
> > >
> > > No problem, here is a link that is relatively simple to understand :
> > >
> > >
> >
> http://javarevisited.blogspot.co.uk/2011/02/fix-protocol-session-or-admin-messages.html
> > >
> > > However, a simplified approach of just adding a client ID seems
> > sufficient
> > > -
> > > the sequence number reset functionality of FIX is overly complex for
> this
> > > requirement.
> > >
> > > Regards
> > > Mike
> > >
> > >
> > >
> > > --
> > > View this message in context:
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/HTTP-REST-sql-query-ID-problem-tp4524p4531.html
> > > Sent from the Apache Ignite Developers mailing list archive at
> > Nabble.com.
> > >
> >
>

Re: HTTP-REST sql query ID problem

Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Wed, Nov 4, 2015 at 2:46 AM, Yakov Zhdanov <yz...@apache.org> wrote:

> We can use IgniteUuid qryId = IgniteUuid.fromUuid(clientId);
>
> and restrict page requests for queries submitted from other clients. I.e.
> throw exception if qryId.globalId() != clientId
>

Yakov, I think it is inconvenient to pass UUID in a query string. How about
we use node order, defined by ClusterNode.order() method? Essentially,
instead of passing just the queryID, REST client will also pass the
nodeOrder parameter.

On the server side, we check that the received node order should be equal
to the local node order. If not, then error. This approach will have the
same behavior we do right now, and will also fix the bug mentioned by
Alexey.


> --Yakov
>
> 2015-11-04 12:21 GMT+03:00 endian675 <en...@gmail.com>:
>
> > Alexey,
> >
> > No problem, here is a link that is relatively simple to understand :
> >
> >
> http://javarevisited.blogspot.co.uk/2011/02/fix-protocol-session-or-admin-messages.html
> >
> > However, a simplified approach of just adding a client ID seems
> sufficient
> > -
> > the sequence number reset functionality of FIX is overly complex for this
> > requirement.
> >
> > Regards
> > Mike
> >
> >
> >
> > --
> > View this message in context:
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/HTTP-REST-sql-query-ID-problem-tp4524p4531.html
> > Sent from the Apache Ignite Developers mailing list archive at
> Nabble.com.
> >
>

Re: HTTP-REST sql query ID problem

Posted by Yakov Zhdanov <yz...@apache.org>.
We can use IgniteUuid qryId = IgniteUuid.fromUuid(clientId);

and restrict page requests for queries submitted from other clients. I.e.
throw exception if qryId.globalId() != clientId

--Yakov

2015-11-04 12:21 GMT+03:00 endian675 <en...@gmail.com>:

> Alexey,
>
> No problem, here is a link that is relatively simple to understand :
>
> http://javarevisited.blogspot.co.uk/2011/02/fix-protocol-session-or-admin-messages.html
>
> However, a simplified approach of just adding a client ID seems sufficient
> -
> the sequence number reset functionality of FIX is overly complex for this
> requirement.
>
> Regards
> Mike
>
>
>
> --
> View this message in context:
> http://apache-ignite-developers.2346864.n4.nabble.com/HTTP-REST-sql-query-ID-problem-tp4524p4531.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>

Re: HTTP-REST sql query ID problem

Posted by endian675 <en...@gmail.com>.
Alexey,

No problem, here is a link that is relatively simple to understand :
http://javarevisited.blogspot.co.uk/2011/02/fix-protocol-session-or-admin-messages.html

However, a simplified approach of just adding a client ID seems sufficient -
the sequence number reset functionality of FIX is overly complex for this
requirement.

Regards
Mike



--
View this message in context: http://apache-ignite-developers.2346864.n4.nabble.com/HTTP-REST-sql-query-ID-problem-tp4524p4531.html
Sent from the Apache Ignite Developers mailing list archive at Nabble.com.

Re: HTTP-REST sql query ID problem

Posted by Alexey Kuznetsov <ak...@gridgain.com>.
M G,

Could you please, give a link to FIX protocol description?
I would like to read about it to be on same page with you.

Thanks!

On Wed, Nov 4, 2015 at 4:17 PM, M G <en...@gmail.com> wrote:

> How about a sequence number pattern like in the FIX protocol? So the
> restarted node carries on from where it left off. To make that work you
> would also need a client id (equivalent to a CompId in FIX) to make each
> request unique.
> On 4 Nov 2015 09:13, "Alexey Kuznetsov" <ak...@gridgain.com> wrote:
>
> > Igniters,
> >
> > I found that we have following problems with HTTP-REST sql query API.
> >
> > After user execute sql query he will receive queryId to be able to fetch
> > next page.
> > See docs: https://apacheignite.readme.io/docs/rest-api#sql-query-execute
> >
> > And current implementation of queryId is a long that simply incremented.
> >
> > First problem:
> >   1. client1 execute query and get queryId = 1.
> >   2. node where query was executed is restarted (queryId generator
> > initialized to zero).
> >   3. client2 execute some query and also get queryId=1.
> >   4. client1 fetch next page for queryId=1 and GETS results of
> client2!!!!
> >
> > Second problem:
> >   As queryId is generated sequentially it is very easy to brute force and
> > some client could get data of other clients too easy.
> >
> > What we could do:
> >  1) Add nodeId to execute sql query response and fetch next page should
> > pass queryId + nodeId to get next page.
> >  2) Generate queryId as random long.
> >
> > OR
> >
> > Generate queryId as random UUID in this case it will be globally random,
> no
> > need for nodeId.
> >
> > But I'm afraid this will break backward compatibility.
> >
> > Thoughts?
> >
> > --
> > Alexey Kuznetsov
> > GridGain Systems
> > www.gridgain.com
> >
>



-- 
Alexey Kuznetsov
GridGain Systems
www.gridgain.com

Re: HTTP-REST sql query ID problem

Posted by M G <en...@gmail.com>.
How about a sequence number pattern like in the FIX protocol? So the
restarted node carries on from where it left off. To make that work you
would also need a client id (equivalent to a CompId in FIX) to make each
request unique.
On 4 Nov 2015 09:13, "Alexey Kuznetsov" <ak...@gridgain.com> wrote:

> Igniters,
>
> I found that we have following problems with HTTP-REST sql query API.
>
> After user execute sql query he will receive queryId to be able to fetch
> next page.
> See docs: https://apacheignite.readme.io/docs/rest-api#sql-query-execute
>
> And current implementation of queryId is a long that simply incremented.
>
> First problem:
>   1. client1 execute query and get queryId = 1.
>   2. node where query was executed is restarted (queryId generator
> initialized to zero).
>   3. client2 execute some query and also get queryId=1.
>   4. client1 fetch next page for queryId=1 and GETS results of client2!!!!
>
> Second problem:
>   As queryId is generated sequentially it is very easy to brute force and
> some client could get data of other clients too easy.
>
> What we could do:
>  1) Add nodeId to execute sql query response and fetch next page should
> pass queryId + nodeId to get next page.
>  2) Generate queryId as random long.
>
> OR
>
> Generate queryId as random UUID in this case it will be globally random, no
> need for nodeId.
>
> But I'm afraid this will break backward compatibility.
>
> Thoughts?
>
> --
> Alexey Kuznetsov
> GridGain Systems
> www.gridgain.com
>