Conversation
p/api/fever.php
Outdated
| $values[':id'] = $max_id; | ||
| $order = ' ORDER BY id DESC'; | ||
| } else { | ||
| } elseif ($since_id !== null) { |
There was a problem hiding this comment.
This was wrong before, so just the question if we want to make it fail safe:
Looks like the calling part makes sure that either since_id or max_id is not null. But if we consider that someone is trying to use the API programmatically, then we should move both the WHERE and the ORDER (or at least the WHERE) clause into the if-else. Otherwise the SQL would be broken if someone calls this function with max_id=null and since_id=null.
Or probably use an array to collect all WHERE parts and append it with implode(' AND ', $where) before $order is added to $sql.
There was a problem hiding this comment.
The old 1=1 trick is also fine :-D
p/api/fever.php
Outdated
| // use the since_id argument to request the next $item_limit items | ||
| $since_id = isset($_REQUEST['since_id']) && is_numeric($_REQUEST['since_id']) ? intval($_REQUEST['since_id']) : 0; | ||
| $since_id = '' . $_REQUEST['since_id']; | ||
| if (!ctype_digit($since_id) || $since_id < 0) { |
There was a problem hiding this comment.
If I read the docu correct, a negative sign would already not be accepted by ctype_digit as it is a non numeric value. So I guess the <0 check is not required.
Also from the documentation: the string 000000001 would be accepted, but I don't know what the database makes out of it in that case id > 000000001. Thats why I originally used intval().
But to be honest, I didn't test these edge cases.
|
So you pushed while I was still reviewing and typing. LOL, seems you catched my remarks before I even submitted them ;-) |
* FeverAPI 32-bit fixes FreshRSS#1962 * Small fixes FreshRSS#1964 (comment)
#1962