Exceeding maximum number of `id`s results in `Invalid id` because of unconsidered PHP `explode` function

[Benjamin-Loison]

If limit is set and positive, the returned array will contain a maximum of limit elements with the last element containing the rest of string.

import requests
import json

url = 'http://localhost/YouTube-operational-API/videos'

params = {
    'part': 'snippet',
    'id': ','.join(['MX5GkDRIdno' for _ in range(51)])
}

text = requests.get(url, params = params).text
data = json.loads(text)
print(json.dumps(data, indent = 4))

The last entry of $realIds then contain VIDEO_ID_50,VIDEO_ID_51.

[Benjamin-Loison]

The following does not work (actually expected behavior):

diff --git a/videos.php b/videos.php
index de1bfa4..91a9d43 100644
--- a/videos.php
+++ b/videos.php
@@ -53,10 +53,16 @@
         $isClip = isset($_GET['clipId']);
         $field = $isClip ? 'clipId' : 'id';
         $ids = $_GET[$field];
-        $realIds = str_contains($ids, ',') ? explode(',', $ids, 50) : [$ids];
-        if (count($realIds) == 0) {
+        $realIdsCountMax = 50;
+        $realIds = explode(',', $ids, $realIdsCountMax);
+        $realIdsCount = count($realIds);
+        if ($realIdsCount == 0) {
             dieWithJsonMessage('Invalid id');
         }
+        if ($realIdsCount > $realIdsCountMax)
+        {
+            dieWithJsonMessage('Too many id');
+        }
         foreach ($realIds as $realId) {
             if ((!$isClip && !isVideoId($realId)) && !isClipId($realId)) {
                 dieWithJsonMessage("Invalid $field");

However, the following works:

diff --git a/videos.php b/videos.php
index de1bfa4..f1b5261 100644
--- a/videos.php
+++ b/videos.php
@@ -53,9 +53,9 @@
         $isClip = isset($_GET['clipId']);
         $field = $isClip ? 'clipId' : 'id';
         $ids = $_GET[$field];
-        $realIds = str_contains($ids, ',') ? explode(',', $ids, 50) : [$ids];
-        if (count($realIds) == 0) {
-            dieWithJsonMessage('Invalid id');
+        $realIds = explode(',', $ids);
+        if (count($realIds) > 50) {
+            dieWithJsonMessage("Too many $field");
         }
         foreach ($realIds as $realId) {
             if ((!$isClip && !isVideoId($realId)) && !isClipId($realId)) {

[Benjamin-Loison]

I have doubts that the following line can even be triggered.

https://github.com/Benjamin-Loison/YouTube-operational-API/blob/c0baf2e81671f8d46aeed1ae478dc7a84d5d784b/videos.php#L58

Let us investigate when it was introduced. Well it was there since the beginning:

https://github.com/Benjamin-Loison/YouTube-operational-API/commit/20acaee08a42f965174349ea71f1707a54a5356b#diff-515c409ddb619da28c52829262e517f3d2ab4910aaa2ab60dd88e9064e0cfd4fR26

Without id I get:

Required parameters not provided

With an empty id I get:

Invalid id from this line

This check seems to have always been unnecessary, as there was already the isset on $_GET['id'].

[Benjamin-Loison]

Could limit $ids considered characters for the complexity but as a whole limiting previously at apache2 level in a more general way seem more appropriate if I ever do so.