Problem/Motivatio

Generating images gives error
"hash_equals(): Expected known_string to be a string, null given i Drupal\webp\Controller\ImageStyleDownloadController->deliver()", this hits when image-style returns a non-webp and the sanity check then fixes it.

Steps to reproduce

Upload new image to some type of content
Regenerate image styles and empty caches
Reload content

Proposed resolution

The solution I found so far has been to check that the "IMAGE_DERIVATIVE_TOKEN" is not NULL. However not sure this is fully optimal..

Comments

VAnnergard created an issue. See original summary.

vannergard’s picture

Issue summary: View changes
vannergard’s picture

Issue summary: View changes
vannergard’s picture

StatusFileSize
new1.08 KB

And here is the patch I used to work around the warning. As mentioned not sure how secure it is, but it runs the same code as before but without warning.

vannergard’s picture

To be clear this issue seems to not have any functional effect, but it makes logs harder to read and is an general annoyance

jedgar1mx’s picture

StatusFileSize
new1.22 KB

I see this error too. In some of our cases it looks like the image style that the module it's trying to serve is missing. So maybe just having a more friendly way to exit out in case of missing file. I edited the first patch a bit and now I get Error generating image, missing source file. message instead of the site Temporarily Unavailable message.

avpaderno’s picture

Version: 8.x-1.0-beta6 » 8.x-1.x-dev
Status: Active » Needs review
Issue tags: -PHP Warning hash_equals
zcht’s picture

Since the switch to PHP 8.1 I had the same error, patch #6 fixed it successfully in my case.

gaele’s picture

Status: Needs review » Reviewed & tested by the community

RTBC for #6.

basvredeling’s picture

Status: Reviewed & tested by the community » Needs work

Patch #6 doesn't adhere to the Drupal code style. Needs a slight update.

gaele’s picture

arccoss’s picture

Status: Needs work » Needs review
StatusFileSize
new2.48 KB
new1.91 KB

Made some fixes on the formatting of #6. Now it should be ok.

basvredeling’s picture

Status: Needs review » Reviewed & tested by the community

#12 looks good, status back to RTBC

basvredeling’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.04 KB
new92 bytes

Patch #12 didn't apply via composer patching. New patch also fixes bad code style (tabs) in next few lines.

avpaderno’s picture

Title: hash_equals warning » hash_equals(): Expected $known_string to be a string, null given in Drupal\webp\Controller\ImageStyleDownloadController::deliver()
Priority: Minor » Normal
Status: Needs review » Needs work
+      try {
+        if (isset($image_token)) {
+          $valid &= hash_equals($request->query->get(IMAGE_DERIVATIVE_TOKEN), $image_style->getPathToken($image_uri));
+        }
+        else {
+          $valid = false;
+        }
+      }
+      catch (\Throwable $th) {
+        $valid = false;
+      }

There is no need to use a try block. It's sufficient to avoid to pass NULL to hash_equals().

For changing the code to follow the coding standards, a new issue is required.

basvredeling’s picture

Status: Needs work » Needs review
StatusFileSize
new1.31 KB

New patch with feedback from #15 included.

godotislate’s picture

StatusFileSize
new989 bytes

The logic in the controller can probably otherwise be streamlined a bit, but I think this patch addresses the issue and has the smallest diff.

berliner’s picture

Patch in #17 applies properly and solves the error.

odai atieh’s picture

Patch #17 solved the issue

ahmad abbad’s picture

+1 patch#17

gaele’s picture

Status: Needs review » Reviewed & tested by the community
idiaz.roncero’s picture

+1, Patch 17 solves the issue.

eduardo morales alberti’s picture

Patch #17 solved the issue

klemendev’s picture

Looks good (patch #17 fixes the issue), hope this gets into a release soon :)

angelgarciaco’s picture

Well .. patch #17 what do we could have done without you? :D

nicxvan’s picture

17 is required for me or the site fails, adding 17 works though!

klemendev’s picture

@mandclu asked for confirmations of open issues on #3290669 so we can get a new stable release, so here is another confirmation this is RTBC and working on our websites :)

klemendev’s picture

I can confirm again #17 works fine on multiple production sites and this issue seems RTBC and for release

  • mandclu committed 52f7d07d on 8.x-1.x
    Issue #3267138 by basvredeling, Arccoss, VAnnergard, jedgar1mx,...
mandclu’s picture

Status: Reviewed & tested by the community » Fixed

Thanks to all for the various patches, as well as feedback on the patch in #17. Merged in, and will roll a release shortly.

Status: Fixed » Closed (fixed)

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