Undefined array key warning in UrlHelper::parse()

Created on 23 April 2024, 10 months ago
Updated 30 April 2024, 10 months ago

Problem/Motivation

Fuzzing reveals that UrlHelper::parse() throws a warning when called with some malformed strings:

   WARNING  Undefined array key 1 in core/lib/Drupal/Component/Utility/UrlHelper.php on line 200.

Steps to reproduce

\Drupal\Component\Utility\UrlHelper::parse('#/://#')

Proposed resolution

One solution could be to check if $parts[0] is empty before trying to use it in explode().

๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Baseย  โ†’

Last updated 44 minutes ago

Created by

๐Ÿ‡ซ๐Ÿ‡ทFrance prudloff Lille

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @prudloff
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Vivek Panicker Kolkata

    Vivek Panicker โ†’ made their first commit to this issueโ€™s fork.

  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Vivek Panicker Kolkata
  • Pipeline finished with Failed
    10 months ago
    Total: 1687s
    #155064
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Vivek Panicker Kolkata

    Tests are failing because of media_library test https://git.drupalcode.org/issue/drupal-3442833/-/jobs/1413833#L79.
    Not sure how to resolve it.

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Current development branch is 11.x

    Can't just add this though it will have to have backward compatibility coverage for the typehint being added. With a deprecation.

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance prudloff Lille

    The type hint would not fix the issue.
    You can see in the steps to reproduce that I am passing a string.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Binoli Lalani Gujarat

    Binoli Lalani โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Canceled
    10 months ago
    Total: 176s
    #156152
  • Pipeline finished with Canceled
    10 months ago
    Total: 427s
    #156154
  • Pipeline finished with Failed
    10 months ago
    Total: 477s
    #156169
  • Pipeline finished with Success
    10 months ago
    Total: 1080s
    #156163
  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Binoli Lalani Gujarat

    Hello,

    I can reproduce the error in drupal 11.x and raised MR against 11.x branch. Please review.

    Thank you!

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Since this is a component class believe we will need test coverage

  • Pipeline finished with Success
    10 months ago
    Total: 1081s
    #157152
  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Binoli Lalani Gujarat

    Hello @smustgrave,

    Thank you for reviewing the code. I have added test coverage for the warning. Please review.

    Thank you!

  • Status changed to RTBC 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Not 100% the use case but the test does show the failure

    1) Drupal\Tests\Component\Utility\UrlHelperTest::testParse with data set #5 ('#/://#', array(null, array(), '/://#'))
    Undefined array key 1
    /builds/issue/drupal-3442833/core/lib/Drupal/Component/Utility/UrlHelper.php:213
    /builds/issue/drupal-3442833/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php:272
    /builds/issue/drupal-3442833/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
    /builds/issue/drupal-3442833/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
    /builds/issue/drupal-3442833/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
    /builds/issue/drupal-3442833/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
    /builds/issue/drupal-3442833/vendor/phpunit/phpunit/src/TextUI/Command.php:146
    /builds/issue/drupal-3442833/vendor/phpunit/phpunit/src/TextUI/Command.php:99
    ERRORS!
    Tests: 161, Assertions: 165, Errors: 1.
    

    And checking if empty doesn't seem to hurt. LGTM.

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    Left a suggestion on the MR to preserve the logic of which bit of logic processes external urls and which processes internal.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia pradhumanjainOSL

    pradhumanjain2311 โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Success
    10 months ago
    Total: 1037s
    #160834
Production build 0.71.5 2024