WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#8974 closed enhancement (fixed)

WordPress permalinks settings page should work with IIS 7.0 URL Rewrite Module

Reported by: ruslany Owned by: westi
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.8
Component: Permalinks Keywords: close
Focuses: Cc:

Description

WordPress is able to determine if it is running on Apache and then it offers "Pretty Permalink" options and also auto-generates mod_rewrite rules necessary to support pretty permalinks. However, when WordPress is running on IIS then WordPress always uses "Almost pretty permalinks" with index.php in the URL.

Now, that IIS has a built-in support for URL rewriting, it would be benefitical for WordPress users if WordPress offered "Pretty Permalinks" option when running on IIS 7.0 with url rewriter installed. Also WordPress can auto-generate IIS rewrite rules necessary to support "Pretty Permalinks". This capability will greatly simplify the permalink configuration task for users who host WordPress on IIS.

I already have a patch ready for this enhancement. See attached file for an example how it works.

If you decide to accept this enhancement feel free to assign bug to me to provide the patch.

Attachments (7)

Wordpress-permalinks2.png (61.8 KB) - added by ruslany 5 years ago.
Permalinks with IIS URL Rewriting
WordPressURLRewrite-misc(11027).patch (6.9 KB) - added by peaceablewhale 5 years ago.
New patch to resolve conflict between WordPressURLRewrite.patch and the current revision of misc.php in trunk
WordPressURLRewrite.patch (14.9 KB) - added by ruslany 5 years ago.
Removed svn:ingore and fixed the conflicts in misc.php
8974.patch (14.8 KB) - added by hakre 5 years ago.
Small benefit in putting vars in another order and resuse an evaluation.
8974.diff (14.8 KB) - added by Denis-de-Bernardy 5 years ago.
per ryan's comment
8974.2.diff (14.8 KB) - added by Denis-de-Bernardy 5 years ago.
against 11256
8974.3.diff (15.0 KB) - added by sivel 5 years ago.

Download all attachments as: .zip

Change History (41)

ruslany5 years ago

Permalinks with IIS URL Rewriting

comment:1 DD325 years ago

  • Version changed from 2.7 to 2.8

You can attach a Diff of your changes to this Trac ticket.

comment:2 westi5 years ago

  • Owner changed from ryan to westi
  • Status changed from new to assigned

comment:3 ruslany5 years ago

I am working on preparing the diff file: adding comments, etc. Should provide it shortly.

comment:4 westi5 years ago

  • Keywords reporter-feedback added

It is getting late to include this into version 2.8

Any news on when you might be able to share a patch file?

comment:5 ruslany5 years ago

The patch with proposed fix is attached. Sorry it took so long. Let me know if you have any questions about the patch or if you need help with testing of patch.

comment:6 ruslany5 years ago

The patch contains modifications to the following files:

  1. /wp-admin/includes/misc.php – added functions to add and remove rewrite rules to/from IIS configuration file
  2. /wp-admin/options-permalinks.php – changed the page rendering logic so that it displays “pretty permalinks” options if IIS has URL rewriter installed. Also added logic to render different messages when user saves permalinks configuration when IIS with URL rewriter is used.
  3. /wp-includes/functions.php – added function to determine if URL rewriter is installed and loaded in IIS
  4. /wp-includes/rewrite.php – added function that generates IIS rewrite rules
  5. /wp-includes/vars.php – added a variable that is used to check if IIS 7 is used.

I have tested the patch on Windows Server 2008 with IIS 7 and URL rewrite module installed. I tested it with PHP 5.2.8 and PHP 5.2.9.

comment:7 westi5 years ago

Thank you for the patch.

I will work through and review and possibly make a few changes to better match the coding style and then get the code into trunk for wider testing.

comment:8 ruslany5 years ago

Cool! Once it is in the trunk I will test it more on various platforms.

comment:9 peaceablewhale5 years ago

  • Keywords has-patch needs-testing added; permalinks reporter-feedback removed

comment:10 peaceablewhale5 years ago

Changed the keywords in order to let people notice and test it during the Marathon (http://wordpress.org/development/2009/04/the-super-awesome-wordpress-24-hour-has-patch-marathon/)

comment:11 peaceablewhale5 years ago

The patch doesn't seem to work with IIS 7.5 on Windows 7 Beta x86 with URL Rewrite Module 1.1 installed. PHP/5.2.9-2 Non-Thread Safe. The permalink options are still in the format of ${ServerHost}/wordpress/index.php/${permalink}. Saving "/archives/%post_id%" in Custom Structure does not generate any rule in web.config and no warning about web.config is displayed.

On the other hand, the misc.php part of WordPressURLRewrite.patch has a conflict with the current trunk revision. I will attach a patch for that (not replacing WordPressURLRewrite.patch).

peaceablewhale5 years ago

New patch to resolve conflict between WordPressURLRewrite.patch and the current revision of misc.php in trunk

comment:12 ruslany5 years ago

peaceablewhale, thanks for testing the patch and reporting the bug. That problem was caused by an incorrect detection logic which failed to detect URL rewriter when fastcgi impersonation was turned on. I've attached new patch that has a fix for this bug, plus it is also has all the conflicts in misc.php resolved.

comment:13 peaceablewhale5 years ago

Thanks Ruslan. The new patch works now. I have tested the followings:

  1. Enable permalink when the web.config file does not exist and the folder is writable

=> Permalink was enabled successfully and a web.config file with the "wordpress" rule was successfully generated

  1. Enable permalink when the web.config file does not exist and the folder is non-writable

=> The patch generated a message asking the user to "update" the web.config file
=> Question: Should more instructions be given to guide the user how to create the web.config file and add the necessary rule?

  1. Enable permalink when the web.config file exists with the "wordpress" rule and the file is writable

=> Permalink was enabled successfully

  1. Enable permalink when the web.config file exists without the "wordpress" rule and the file is writable

=> Permalink was enabled successfully and the "wordpress" rule was added to web.config successfully

5 Enable permalink when the web.config file exists with the "wordpress" rule and the file is non-writable
=> Permalink was enabled successfully and the user was asked to update the web.config file

  1. Enable permalink when the web.config file exists without the "wordpress" rule and the file is non-writable

=> Permalink was enabled successfully and the user was asked to update the web.config file

  1. Disable permalink the web.config file exists with the "wordpress" rule and the file is writable

=> Permalink was disabled successfully and the "wordpress" rule in web.config was removed

  1. Disable permalink the web.config file exists with the "wordpress" rule and the file is non-writable

=> Permalink was disabled successfully BUT the web.config file was not updated and there is no notice to the user
=> Suggestion: Add a notice asking the user to remove the "wordpress" rule in web.config manually

By the way, an extra question, does this patch work with URL Rewrite Module 1.0?

comment:14 ruslany5 years ago

Cool. Thanks a lot for testing this!

With URL rewrite 1.0 the patch will not be able to reliably determine the presence of the URL rewrite module, so the permalinks settings will work as if no url rewriting is enabled. Only with URL rewrite 1.1 it is possible to reliably determine the presence of the module because v1.1 sets a server variable indicating that URL rewriting is enabled.

comment:15 peaceablewhale5 years ago

As Ruslan is looking into adding the notice to remove the rewrite rule if web.config is not writable, I am not going to add the "tested" keyword to this report until he has uploaded a new patch or confirmed that the notice is a "won't fix".

comment:16 ruslany5 years ago

The notice about removing the rule manually is "won't fix". This is how it works on Apache with .htacess file. Changing that would greatly complicate the logic in the options-permalinks.php page. Plus having this rewrite rule left over in web.config file will not do any harm.

I've attached a new version of the patch which contains minor modification to the information text for the user that explains where to place the rewrite rule inside of web.config file.

comment:17 ruslany5 years ago

  • Cc ruslany@… added

comment:18 peaceablewhale5 years ago

  • Keywords tested added

I've tested the new patch and all the 8 conditions tested before now produce expected results.

Regarding the new string, should a slash be added before <configuration> to indicate that it is the root element? In addition, I think that "in web.config file" should be changed to "in the <code>web.config<code> file" (added "the" before web.config and "web.config" should be inside a <code> element).

Since they are minor changes, I've marked the patch as tested.

comment:19 ruslany5 years ago

Thanks! Since this patch has been already tested and the proposed changes are minor, I am not going to attach another patch. Those changes can be easily made when the patch is checked in.

comment:20 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 2.8 to Future Release

broken patch

ruslany5 years ago

Removed svn:ingore and fixed the conflicts in misc.php

comment:21 ruslany5 years ago

  • Milestone changed from Future Release to 2.8

I've removed the svn:ignore and fixed the conflicts with misc.php. I've changed the milestone back to 2.8.

comment:22 ruslany5 years ago

  • Keywords has-patch added; needs-patch needs-testing removed

hakre5 years ago

Small benefit in putting vars in another order and resuse an evaluation.

comment:23 Denis-de-Bernardy5 years ago

  • Keywords commit added

commit? wontfix?

comment:24 peaceablewhale5 years ago

Tested the new patch. Nothing wrong.

comment:25 ryan5 years ago

Looks good to me although I would rename is__writable() to something like win_is_writable() because I thought it was a typo for is_writable() at first. :-)

I'll give this a test to make sure apache is still happy. I think westi is going to look at IIS. I'll try to get IIS setup as well. We should be able to get this into 2.8.

Last edited 19 months ago by dd32 (previous) (diff)

Denis-de-Bernardy5 years ago

per ryan's comment

comment:26 Denis-de-Bernardy5 years ago

there you go

Denis-de-Bernardy5 years ago

against 11256

comment:27 follow-up: Denis-de-Bernardy5 years ago

see also: #6631, related to the wp_is_writable function.

comment:28 in reply to: ↑ 27 sivel5 years ago

  • Cc matt@… added

From Ryan in #wordpress-dev:

Anyone want to cleanup this patch to better match our coding style (cuddled braces, etc.) and make sure it doesn't break apache rewrite rules?

Cleaned up patch coming in a few moments. My testing shows that this does not seem to interfere or break the apache rewrite rules.

sivel5 years ago

comment:29 ruslany5 years ago

Tried the cleaned up patch. Works as expected.

comment:30 ryan5 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [11350]) Support IIS 7.0 URL Rewrite Module. Props ruslany. Hat tips to peaceablewhale, hakre, Denis-de-Bernardy, sivel. fixes #8974

comment:31 frumph5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening:

  • function iis7_save_url_rewrite_rules()
  • function iis7_add_rewrite_rule()

These functions need to handle multiple rewrite rules in the rewrite.php for WPMU and future additions to the rewrite rules, right now it's only working for the single rewrite rule, if you add another one it errors the web.config file out on iis7.

Also the creation of the web.config file doesnt happen until you go to the settings - permalinks section itself so flush_rules() on upgrade isn't creating the web.config file - not sure if that's on purpose.

Last edited 19 months ago by dd32 (previous) (diff)

comment:32 peaceablewhale5 years ago

  • Keywords close added; has-patch tested commit removed

It seems that the Apache mod_rewrite rules don't support MU either. In addition, the flush_rules behavior appears by design.

comment:33 Denis-de-Bernardy5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Please open a WPMU ticket.

Note: See TracTickets for help on using tickets.