Problem/Motivation

There’s not any consistency with when dots/periods are escaped in rewrite conditions/rules in the main .htaccess file.

Proposed resolution

Escape all dot/periods for consistency and clarity.

Release note snippet

Drupal's generated .htaccess files now consistently escape dots (the . character) in rewrite conditions and rules. (For example, statistics.php has been corrected to statistics\.php.) These changes make the rules slightly more strict (and therefore safer). Site owners should make a backup of customized .htaccess files before updating, and may wish to also escape dots in their own custom rules where appropriate.

Issue fork drupal-2989262

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

    Support from Acquia helps fund testing for Drupal Acquia logo

    Comments

    Daniel Korte created an issue. See original summary.

    Daniel Korte’s picture

    Assigned: Daniel Korte » Unassigned
    Status: Active » Needs review
    FileSize
    1.71 KB
    Daniel Korte’s picture

    Issue summary: View changes
    Daniel Korte’s picture

    Title: Escape .htaccess dots » Escape all RewriteCond/RewriteRule .htaccess dots

    Version: 8.6.x-dev » 8.8.x-dev

    Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

    Version: 8.8.x-dev » 8.9.x-dev

    Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

    Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

    Kristen Pol’s picture

    Version: 8.9.x-dev » 9.1.x-dev
    Issue tags: +Bug Smash Initiative

    Patch applies cleanly to 9.1.x.

    Kristen Pol’s picture

    Status: Needs review » Needs work

    Thanks for the issue and patch.

    1) If I'm following the changes properly, it looks like the comparison part should be escaped and the path to go to should not which I think makes sense. Example:

    +++ b/.htaccess
    @@ -123,13 +123,13 @@ AddEncoding gzip svgz
    -  RewriteRule ^core/install.php core/install.php?rewrite=ok [QSA,L]
    +  RewriteRule ^core/install\.php core/install.php?rewrite=ok [QSA,L]
    

    2) Kicking off tests for 9.1.x.

    Daniel Korte’s picture

    Status: Needs work » Needs review
    FileSize
    3.51 KB

    Thanks @Kristen Pol

    1) That's correct.

    2) Should be fixed by the attached patch.

    longwave’s picture

    Status: Needs review » Reviewed & tested by the community

    Thanks for catching this. I agree that this should be fixed, the unescaped dot means "any character" which means we have some false positives here which, while unlikely to be triggered on most sites, might be tricky to debug if you did accidentally find them.

    alexpott’s picture

    Status: Reviewed & tested by the community » Fixed

    This makes sense. Only commiting to 9.1.x because htaccess files are often customised so changing only in a minor release is nice for everyone. I can't work out any serious bugs or issued caused by this misconfiguration so that seems okay.

    Committed 952c086 and pushed to 9.1.x. Thanks!

    • alexpott committed 952c086 on 9.1.x
      Issue #2989262 by Daniel Korte, Kristen Pol: Escape all RewriteCond/...

    Status: Fixed » Closed (fixed)

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

    xjm’s picture

    Status: Closed (fixed) » Needs work
    Issue tags: +9.1.0 release notes, +Needs release note

    Changes to these files always get a release notes mention.

    xjm’s picture

    Issue summary: View changes
    Issue tags: -Needs release note +Needs change record

    I drafted a release note. It should also link a change record explaining the change in more detail, because explaining escaped-dot-literal versus dot-regex-match is beyond the scope of the release notes, but someone might need an explanation. For now I'm going to add a link to the issue itself (which is a no-no generally), but let's add a change record so we can explain it properly when 9.1.0 itself comes out. Thanks!

    Version: 9.1.x-dev » 9.3.x-dev

    Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

    xjm credited quietone.

    xjm’s picture

    Status: Needs work » Fixed

    Adding credit for @quietone who added the change record. Thanks!

    Status: Fixed » Closed (fixed)

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

    quietone’s picture

    Issue tags: -Needs change record

    This does have a CR, removing tag.