Page MenuHomePhorge

fix PHP8 array_slice($results, null) which cause diffusion locate file broken
ClosedPublic

Authored by gemini133 on Jul 28 2024, 01:45.
Tags
None
Referenced Files
F2689517: D25750.1734735446.diff
Thu, Dec 19, 22:57
F2689085: D25750.1734728101.diff
Thu, Dec 19, 20:55
F2686436: D25750.1734715059.diff
Thu, Dec 19, 17:17
F2686300: D25750.1734712140.diff
Thu, Dec 19, 16:29
F2333140: image.png
Jul 29 2024, 01:10
F2332521: image.png
Jul 28 2024, 14:12
F2332519: image.png
Jul 28 2024, 14:12
F2332517: image.png
Jul 28 2024, 14:12

Details

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I skiped the linting and unit testing because they take time to process. too much for this tiny change

Thanks. Tested 👍

Please evaluate the same change also here to fix the "Pattern Search"

diff --git a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php
index ba4c824061..5f40aa06bb 100644
--- a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php
@@ -40,7 +40,7 @@ final class DiffusionSearchQueryConduitAPIMethod
       throw $ex;
     }
 
-    $offset = $request->getValue('offset');
+    $offset = $request->getValue('offset', 0);
     $results = array_slice($results, $offset);
 
     return $results;

I skiped the linting and unit testing because they take time to process. too much for this tiny change

Well, tests took 1 second on my laptop :D But yes: setting this up took 1 hour to me the first time... so I completely understand. Don't worry.

Tested! thanks 🌈


To land, run arc land


P.S.

Do you also have this error now?

strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [DiffusionQueryPathsConduitAPIMethod.php:94]

To fix, probably we need to replace strlen() with phutil_nonempty_str in a couple of places, or, just cast to string. Something like this:

diff --git a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php
index e511492fc4..7f48133c30 100644
--- a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php
@@ -33,13 +33,13 @@ final class DiffusionQueryPathsConduitAPIMethod
 
   protected function getGitResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
-    $path = $drequest->getPath();
+    $path = phutil_string_cast($drequest->getPath());
     $commit = $request->getValue('commit');
     $repository = $drequest->getRepository();
 
     // Recent versions of Git don't work if you pass the empty string, and
     // require "." to list everything.
-    if (!strlen($path)) {
+    if ($path === '') {
       $path = '.';
     }
 
@@ -78,11 +78,11 @@ final class DiffusionQueryPathsConduitAPIMethod
   }
 
   protected function filterResults($lines, ConduitAPIRequest $request) {
-    $pattern = $request->getValue('pattern');
+    $pattern = phutil_string_cast($request->getValue('pattern'));
     $limit = (int)$request->getValue('limit');
     $offset = (int)$request->getValue('offset');
 
-    if (strlen($pattern)) {
+    if ($pattern !== '') {
       // Add delimiters to the regex pattern.
       $pattern = '('.$pattern.')';
     }
@@ -90,7 +90,7 @@ final class DiffusionQueryPathsConduitAPIMethod
     $results = array();
     $count = 0;
     foreach ($lines as $line) {
-      if (strlen($pattern) && !preg_match($pattern, $line)) {
+      if ($pattern !== '' && !preg_match($pattern, $line)) {
         continue;
       }

Thanks again. Feel free to land as-is.

This revision is now accepted and ready to land.Jul 28 2024, 08:23

Many thanks for your patience. you're very kind!
but I do have trouble landing this diff.
not sure what I'm doing wrong.

image.png (188×710 px, 65 KB)

image.png (326×1 px, 117 KB)

image.png (138×1 px, 73 KB)

I think the troubleshooting could be inconvenient for both of us. could you make this diff on your own first?
even though I really want to make the contribution on my own. :[

Thanks. Tested 👍

Could you try arc land instead of git push?

Could you try arc land instead of git push?

it's actually arc land. but just with GIT_TRACE env variable to see what's going on deep down

image.png (1×1 px, 878 KB)

You didn't have Trusted Contributors membership, which provides access to the repo. Try now