statistics.php can exit early and prevent output errors

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Fixed failed commands on #86
Attached patch against Drupal 10.1.x

+++ b/core/modules/statistics/statistics.php
@@ -8,23 +8,38 @@
+    // Range set so that value of nid does not goes out of bound
+    // and PDOException is not triggered.
+    $nid = filter_input(INPUT_POST, 'nid', FILTER_VALIDATE_INT, [
+      'options' => [
+        'min_range' => 0,
+        'max_range' => 4294967295,
+      ],
+    ]);

In #61 we agreed to not validate the range. It's not possible to set a value for all DB types supported by core. The correct patch to be re-rolling is #68 - unfortunately #69 added this back in without explanation and after it was decided that filtering by number is incorrect.

Address #92 by removing extra range filtering which is database dependent and will need real controller to check further access (currently it using config setting) and re-titled

Interdiff vs #89 - we should not add theme to base test as child ones must care

Hope this kind of optimization fits

+++ b/core/modules/statistics/statistics.php
@@ -8,23 +8,34 @@
+  $request = Request::createFromGlobals();
+  $kernel = DrupalKernel::createFromRequest($request, $autoloader, 'prod');
...
-    $container->get('request_stack')->push(Request::createFromGlobals());
...
+    $container->get('request_stack')->push($request);

I see no reason to have 2 requests because of bug 🐛 Requests are pushed onto the request stack twice, popped once Needs work

Feel bad I was part of that carrying the bad approach forward.

Can we update the issue summary for the proposed solution and remaining tasks?

Updated to the last patch and #61

Thanks for the quick turnaround.

So reading the proposed solution compared to the patch I see it now.

Ran the test locally just to make sure it failed but it passes without the fix. So think they need to be tweaked.

Statistics is approved for removal. See 🌱 [Policy] Deprecate Statistics module in D10 and move to contrib in D11 Fixed

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project → and the Extensions approved for removal → policies.

It will be moved to a contributed Statistics project once the project is created and the Drupal 11 branch is open.

Reworded IS to account for changes since D8.

Pinging @catch since you were the original issue poster. @mallezie @stefank since you did the most patches before the split.

Daniel's suggestion in #5 is relevant in the "transitions tracking" part of the plan 🌱 Plan for Statistics 1.x Active .

Regarding the nid validation : the maximum check was removed for DB independence, but can we agree we should really keep the min_range => 0 check ? I do not think there is any case in which a nid can be negative, is there ?

Merged 9 years later. Thanks everyone.

Automatically closed - issue fixed for 2 weeks with no activity.