WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 18 months ago

#25194 new defect (bug)

Filtered IIS rewrite rules not added to web.config

Reported by: pat@… Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.6
Component: Rewrite Rules Keywords: has-patch needs-testing
Focuses: Cc:

Description

If using IIS and web.config, appending rules via the filter 'iis7_url_rewrite_rules' will not add the filtered content if a rule for 'wordpress' already exists in the web.config file.

Expected behavior:

Even after WP has added its rule to web.config we should be able to influence the rules content in the web.config via the filter.

Steps to reproduce:

  1. Use a system running IIS that supports rewriting in the web.config
  2. Visit permalinks page so WP generates the 'wordpress' rule in the web.config
  3. Add a filter to 'iis7_url_rewrite_rules' and append the following rule: '<rule name="testrule" stopProcessing="true"><match url="readme.html"/><action type="Rewrite" url="foo.html"/></rule>'
  4. Visit the permalinks page
  5. Rule will not be appended to web.config file

Correct behavior:

  1. Repeat steps 1-3 above
  2. Remove the 'wordpress' rule from web.config file
  3. Visit permalinks page
  4. Filtered rule text will be present in this case

Attachments (2)

iis7-permalinks.patch (11.8 KB) - added by pat@… 18 months ago.
Patch File
iis7.php (1.1 KB) - added by pat@… 18 months ago.
Patch File Plugin Test

Download all attachments as: .zip

Change History (6)

comment:1 follow-up: @pat@…20 months ago

Any thoughts on this one? Issue is still present. I can look at patching it if no one else wants to move on this.

comment:2 in reply to: ↑ 1 @nacin18 months ago

  • Component changed from IIS to Rewrite Rules

Replying to pat@…:

Any thoughts on this one? Issue is still present. I can look at patching it if no one else wants to move on this.

Hi pat, if you're able to patch this, that would be great. IIS support in WordPress doesn't get a whole lot of attention.

@pat@…18 months ago

Patch File

@pat@…18 months ago

Patch File Plugin Test

comment:3 @pat@…18 months ago

  • Keywords has-patch needs-testing added

Okay, I've created checked out the trunk, made my changes, tested and have a document + plugin that illustrates the bug before the patch, and how it works after. There's no UI for the plugin, just edit the file to see what I'm doing, there's some comments in there as well. Here are some notes I took while patching.

Changes:

/wp-admin/includes/misc.php

  • changed function iis7_add_rewrite_rule to 'iis7_add_rewrite_rules'
    • allow filtering of rewrite rule text after 'wordpress' rule already in web.config
    • added $wp_rewrite->non_wp_rules to content generated by WP to be written to web.config
    • take $rewrite_rules text param (filtered by 'iis7_url_rewrite_rules'), parse it as a doc to get a collection of user filtered rules
    • check existing wp and non_wp_rules against filtered rules, determine which rules to include, which to remove from web.config


  • changed function iis7_delete_rewrite_rule to 'iis7_delete_rewrite_rules'
    • instead of deleting just the wordpress rule, also delete all non_wp_rules
    • this is only triggered when the filtered rules_text is empty (will clear all wp rules)


  • changed function iis7_save_url_rewrite_rules
    • changed function calls to pluralized versions.
    • removed un-neccessary params from call to iis7_url_rewrite_rules()



/wp-includes/rewrite.php

  • changed function iis7_url_rewrite_rules
    • dependencies: called by wp-admin/options-permalink.php and wp-admin/includes/misc.php
  • this was being called from options-permalink.php with additional empty string params that didn't seem to be used
  • made a 2nd param of $format, which helps output the content for the options-permalink.php
  • instead of just adding the single wordpress rule, nclude $wp_rewrite->non_wp_rules in generated rules content
    • Influencing the non_wp_rules array is the best way to get rules into the web.config
    • Filtering the text directly with will still work, but non_wp_rules are already written to the .htaccess, so makes sense to do it here as well


/wp-admin/options-permalink.php

  • line 263 (update call to pluralized function, modify params to include $format=true)
  • line 270 (update call to pluralized function, modify params to include $format=true)

To test:
1) Install Plugin
2) Visit permalinks page, plugin will attempt to add:

  • A rule: 'test-extra-rule' via the 'iis7_add_rewrite_rules' filter, if no 'wordpress' rule in web.config then it will be added, if there is it will not be.
  • A 'bar.php' rewrite through $wp_rewrite->add_external_rule (which doesn't have a chance of getting added to the web.config before patch)

3) Apply patch
4) Visit permalinks page

After patch and Step 4 the 'text-extra-rule' and bar.php rewrite will appear in the web.config

Again, the preferred way of adding rules would be to call $wp_rewrite->add_external_rule which would ensure it gets written to .htaccess or the web.config

Last edited 18 months ago by pat@… (previous) (diff)

comment:4 @pat@…18 months ago

One thing to note. Adding a rule via $wp_rewrite->add_external_rule() will push a rule into the $wp_rewrite->non_wp_rules array, which gets added to .htaccess and web.config. However, if the call to add_external_rule is removed, the corresponding rewrite rulet is not removed from the .htaccess file. However, in my patch I am prefixing the non_wp_rules names with 'wordpress-non-wp-' which I'm looking for when writing to the web.config. If there is a rule present with this prefix that is not in the $wp_rewrite->non_wp_rules array then I remove it from the web.config.

I don't know if this is desirable or not. It's the behavior I would want to see as a developer, otherwise your web.config (or .htaccess) could be populated by rules from a 3rd party plugin, that when removed, would leave orphaned rules that could point the web app in the direction of an error. Or, you're developing functionality and want to change the rules, so you would need to manually open up the web.config|.htaccess to hunt and peck for the rules to remove.

I like that the rules are added/removed based off the non_wp_rules array, however this behavior should be consident with the .htaccess as well. Thoughts?

Last edited 18 months ago by pat@… (previous) (diff)
Note: See TracTickets for help on using tickets.