field_get is used to test the, in my opinion, bad practice of encoding arrays in relation Db as strings (personally, I'd rather a more normalized system where arrays are actual tables and joins are used, if necessary, to retrieve data. Makes maintenance easier, too).
At the moment, 

ereg(",?$name=([^,]+)", ", $string", $regs);

Will match against any value ending in $name.
So, say, uid 1 will also be matched against uid 11,21,31 ...
Simple fix was to remove the ? after the , making it non-optional.
If the concern is about whitespace or other separators being used, the regex needs to be made more complex, but in its current state is definitely a bad thing.

Comments

kyber’s picture

kyber commented

Oh, my bad. One more tweak.
The first value needs to be tested against too, that's why the , was optional.
How ugly.
Anyway, 

 ereg(",$name=([^,]+)|^$name=([^,]+)", ", $string", $regs);

Works better.

kyber’s picture

kyber commented

BTW, from checking my own setup, this is a rather uncommon common.inc method.
Only seems to be used in queue.module in my setup, where it was simply preventing people from voting.
Not, say, giving them priveleges they didn't deserve.
No idea if used in other projects.

kyber’s picture

kyber commented

And to further beat to death this small method.
", $string" is rather silly.
If it was ",$string" it would have eliminated the extra test in the regex. with the space after the comma though, it doesn't seem to do anything.
Changed it to just $string without the quotes and comma.

kyber’s picture

kyber commented

*sigh* Terribly sorry to flood the mailing list with comments on this minor thing..
I thought the | match was exclusive in regex. Clearly is not.
Doing that match meant $reg[1] no longer was guaranteed to contain anything. Rather then test against $reg[1] or $reg[3] I just changed it to:

function field_get($string, $name) {
  ereg(",$name=([^,]+)", ",$string", $regs);
  return $regs[1];
}

Which seems to work, with the space after the , removed.
PHP is still a learning experience for me, 'tis clear.

dries’s picture

dries commented
Assigned: Unassigned » dries

Committed to HEAD. Thanks. I agree that serialized data is a bad idea and that normalized SQL tables are preferred, however, it is not always that easy. Patches that move data to normalized SQL tables without introducing performance issues are happily accepted.

Anonymous’s picture

(not verified) commented