Undefined array key warning in UrlHelper::parse()

Created on 23 April 2024, 2 months ago
Updated 30 April 2024, about 2 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 about 2 hours 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 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Vivek Panicker Kolkata
  • Pipeline finished with Failed
    2 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 2 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
    2 months ago
    Total: 176s
    #156152
  • Pipeline finished with Canceled
    2 months ago
    Total: 427s
    #156154
  • Pipeline finished with Failed
    2 months ago
    Total: 477s
    #156169
  • Pipeline finished with Success
    2 months ago
    Total: 1080s
    #156163
  • Status changed to Needs review 2 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 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

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

  • Pipeline finished with Success
    2 months ago
    Total: 1081s
    #157152
  • Status changed to Needs review 2 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 2 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 about 2 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
    about 2 months ago
    Total: 1037s
    #160834
Production build 0.69.0 2024