Closed GoogleCodeExporter closed 8 years ago
Original comment by fiedler....@gmail.com
on 16 Nov 2010 at 11:04
Any suggestions of a patch that won't break backwards compatibility? I can't
see it. Converting ocr_usa_id_ref or ocr_id to char/varchar doesn't seem to be
a good idea.
You know you can always create a table such as
CREATE TABLE oauth_to_uuid (
ocr_id int(11),
uuid char(16)
);
Original comment by brunobg%...@gtempaccount.com
on 17 Nov 2010 at 5:17
Such a table won´t work with an custom database wrapper made for a distributed
database (that´s what i´m trying now). In an distributed database you cant
have auto increment id´s. Except you implement an stand alone id-server.
I think it should work, if i change all queries in OAuthStroeSQL to use
something like oct_usa_id_ref = \'%s\' (instead of %d) and validate every
$user_id at the beginning of an method. Like:
$user_id = preg_replace('/[^a-z0-9\-]/i', '', $user_id);
+ changing the database from int(11) to varchar(36)
That wont break older db´s and still work with integers.
What do you think?
Original comment by fiedler....@gmail.com
on 17 Nov 2010 at 5:39
Btw, there are some "bugs" in OAuthStroeSQL... at some places there´s noted
oct_usa_id_ref = %s
That´s a bit risky. ;o)
Original comment by fiedler....@gmail.com
on 17 Nov 2010 at 5:41
[deleted comment]
e.g.
http://code.google.com/p/oauth-php/source/browse/trunk/library/store/OAuthStoreS
QL.php#213
$user_id can be a sql injection... you don´t know. Should be %d ... or
rewritten for use of UUIDs ;o)
Original comment by fiedler....@gmail.com
on 17 Nov 2010 at 5:59
I just fixed the missing quotes. I don't know how I could have missed that...
It's not a catastrophe because any sane application wouldn't get the user id
from a GET/POST, and the query is passed through a string escaper but it was
bad anyway -- I think it would lead to a bad query that wouldn't run, but not
an injection. Thanks a lot André :)
Now, about your actual request. Your proposal would work, but I'm a little
worried about backward compatibility. People would have to update their
database schemas and there's a penalty for the int->string conversion in joins.
If you can't have auto_increments, then don't you have a larger problem with
all the primary keys in the oauth_* tables? Notice that my oauth_to_uuid does
not have an auto increment id, so perhaps the ugly wrapper could work...
Original comment by brunobg%...@gtempaccount.com
on 17 Nov 2010 at 8:27
Hm, all id´s should work as UUID... let me think about it a bit more. My
actual thoughts are using MySQL only for OAuth handling (not much work), all
other data is stored in Cassandra on more than one node. So at the moment UUID
user_ids are enough. But if someone has 500.000 users, he may want to switch
the whole OAuth thing to a distributed database. He will definitively have a
problem.
Let me think about this a bit more...
greetings André
Original comment by fiedler....@gmail.com
on 17 Nov 2010 at 9:04
If OAuth is moved to a distributed database, you won't use the MySQL storage
system. So you could easily create a new storage system with a schema that uses
UUIDs instead. The migration would not be difficult if you have the (int id)
<-> UUID table... right?
Original comment by brunobg%...@gtempaccount.com
on 23 Nov 2010 at 5:29
Yes, my problem is, that i temporary mix mysql and cassandra. ;o) It works by
now and you´re right. If i write a storage system for use with cassandra it
will work fine. Thats what i want to do... so no problem... except a few small.
;o)
I don´t know why, but at some places in the library id´s (user id, osr id
aso.) get parsed to int. But not every time. :/ I think this should be removed.
If necessary, it should be parsed in storage system. But i think this isn´t
really necessary.
Original comment by fiedler....@gmail.com
on 23 Nov 2010 at 5:48
Hey Andre!
I couldn't find these places ids are parsed to int... But I agree, if they
exist, they should be removed and the storage system deal with it. Can you tell
me where they happen or, better, send a patch? ;)
Original comment by brunobg%...@gtempaccount.com
on 24 Nov 2010 at 7:58
Hi Bruno,
i´ve fetcht r175 and checked again. Seems someone from my team inserted these
intvar() calls in the library. He added xAuth support. :/ So the only thing
that should be changed are the docu-comments, like this:
* @param object usr_id
And the initializers, like this:
function doRequest ( $usr_id = null, ...)
i will close this, think it works fine. If time is ready, i will release a
cassandra wrapper based on https://github.com/thobbs/phpcassa but thats another
topic.
thx for your help, André :o)
Original comment by fiedler....@gmail.com
on 24 Nov 2010 at 9:44
Original issue reported on code.google.com by
fiedler....@gmail.com
on 16 Nov 2010 at 11:03