Internal Server Error

Netsparker Cloud identified an internal server error.
The server responded with an HTTP status 500, indicating there is a server-side error. Reasons may vary, and the behavior should be analyzed carefully. If Netsparker Cloud is able to find a security issue in the same resource, it will report this as a separate vulnerability.

When hitting https://domain.com/en/sites/default/files/styles/thumbnail" URL, error 500 is getting displayed. This is happening for all the folders inside styles.
The exact error message is "ArgumentCountError: Too few arguments to function image_style_deliver(), 1 passed in /var/www/html/domain.com/docs/includes/menu.inc on line 527 and exactly 2 expected in image_style_deliver() (line 807 of /var/www/html/domain.com/docs/modules/image/image.module)."

At least 404 page not found should be displayed instead of 500.

CommentFileSizeAuthor
#14 3063048-14_test_only.patch880 bytesmcdruid
#13 3063048-13.patch2.11 KBmcdruid
#13 interdiff-3063048-6-13.txt806 bytesmcdruid
#6 drupal-fatal-in-styles-folder-3063048-D7.patch1.25 KBdarkodev
#3 drupal-fatal-in-styles-folder-3063048-3-D7.patch533 bytesasvira
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

prabha.venkat created an issue. See original summary.

oranges13’s picture

oranges13
she/her
Location Michigan
CreditAttribution: oranges13 commented

We received this error as well and it was because a crawl bot was hitting invalid urls on our installation, such as `oururl.com/sites/default/files/styles/large/` with no actual image information.

I agree a more graceful error recovery should probably occur.

asvira’s picture

asvira CreditAttribution: asvira at DrupalSquad commented
Status: Active » Needs review
FileSize
drupal-fatal-in-styles-folder-3063048-3-D7.patch533 bytes

This patch makes .../styles/%image_style request to behave in the same way as .../styles/%image_style/public request (403 Access denied).

prabha.venkatesan’s picture

prabha.venkatesan CreditAttribution: prabha.venkatesan commented

Thanks for the patch asvira , but the patch is breaking the site. It is throwing 500 error on all the pages at the bottom of the page. Even on the https://domain.com/en/sites/default/files/styles/thumbnail page it is throwing a 500 error along with the 404.
Any other solution would be helpful.

darkodev’s picture

darkodev CreditAttribution: darkodev commented

Adding to the list of users seeing this error. Netsparker also ran recently on our sites. Will report our findings as we debug . . .

/sites/default/files/styles/large/

results in

ArgumentCountError: Too few arguments to function image_style_deliver(), 1 passed in [...]/includes/menu.inc on line 527 and exactly 2 expected in image_style_deliver() (line 807 of [...]/modules/image/image.module).

On my local dev environment, this throws a generic 404. Perhaps an Apache configuration issue, since we're not seeing more people reporting this issue?

darkodev’s picture

darkodev CreditAttribution: darkodev commented
FileSize
drupal-fatal-in-styles-folder-3063048-D7.patch1.25 KB

Adding !empty($scheme) to check for $valid as well as $scheme = NULL works for me. I feel that it can't hurt to check !empty($scheme) since we're already checking !empty($style).

https://www.php.net/manual/en/migration71.incompatible.php

Our site that runs an older version of PHP only emits the warning (so all versions of 7.x core suffer this issue).

So, this warning in PHP < 7.1:
Warning: Missing argument 2 for image_style_deliver(), called in /includes/menu.inc on line 527 and defined in image_style_deliver() (line 807 of /modules/image/image.module).

Becomes this fatal error in PHP >= 7.1:
ArgumentCountError: Too few arguments to function image_style_deliver(), 1 passed in /includes/menu.inc on line 527 and exactly 2 expected in image_style_deliver() (line 807 of /modules/image/image.module).

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Status: Needs review » Reviewed & tested by the community

That seems like a reasonable fix, thanks @darkodev, @asvira, and @prabha.venkat

Steven Jones’s picture

Steven Jones CreditAttribution: Steven Jones at ComputerMinds commented

This fix works for me, does it need a test though?

sjerdo’s picture

sjerdo
Primary language Dutch
Location Haarlem
CreditAttribution: sjerdo at Synetic commented
Issue tags: +Drupal 7.74 target

Patch #6 LGTM +1

malcolm_p’s picture

malcolm_p CreditAttribution: malcolm_p commented

Working well for me, preventing these server errors with no issue.

webservant316’s picture

webservant316 CreditAttribution: webservant316 commented

I am seeing this error. Why isn't this patch committed?

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
Issue tags: -Drupal 7.74 target

Per #8 it would be good to add a (very basic) test for this.

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
FileSize
interdiff-3063048-6-13.txt806 bytes
3063048-13.patch2.11 KB

Hopefully this'll do.

As we're adding to the helper function which several different tests call, this new code will be run a handful of times. On the one hand perhaps that's overkill, but at least it'll mean more coverage...

I've only manually tested this:

ImageStylesPathAndUrlTestCase::testImageStyleUrlAndPathPublic

...so we'll see if all the other test that call \ImageStylesPathAndUrlTestCase::_testImageStyleUrlAndPath() also behave as expected.

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
FileSize
3063048-14_test_only.patch880 bytes

Oops forgot the test only patch, which might make things a bit messy.

Also noticed a comment typo (mine) which can be fixed on commit:

// Check that requesting a partial image style path return access denied.

s/return/returns/

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: 3063048-14_test_only.patch, failed testing. View results

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented

Let's see if hiding the test only patch helps show the testbot that we want #13

darkodev’s picture

darkodev CreditAttribution: darkodev commented

So sorry I didn't pay more attention to the need for a test, and thanks very much to @mcdruid for jumping on it. Respect!

  • mcdruid committed e470533 on 7.x 
    Issue #3063048 by mcdruid, darkodev, asvira: Error 500 when requesting...
mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
Status: Needs work » Fixed

Thank you!

Status: Fixed » Closed (fixed)

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