WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 7 months ago

#25098 closed enhancement (fixed)

Support URL rewrites on Nginx

Reported by: johnbillion Owned by: nacin
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Rewrite Rules Keywords: has-patch
Focuses: Cc:

Description

Splitting this out from #18852.

In order to get URL rewrites without the index.php prefix on Nginx you need to use the got_rewrite filter. Less than ideal. Let's correctly detect Nginx and allow it to use nice URL rewrites.

Patch coming up.

Attachments (1)

25098.diff (2.3 KB) - added by johnbillion 8 months ago.

Download all attachments as: .zip

Change History (13)

johnbillion8 months ago

comment:1 johnbillion8 months ago

25098.diff adds support for detection of Nginx and adds a got_url_rewrite() function as a wrapper for detecting any kind of rewrite support.

What this patch ultimately does is allow WordPress to generate URL rewrites which don't get index.php prepended.

comment:2 SergeyBiryukov8 months ago

  • Milestone changed from Awaiting Review to 3.7

comment:3 jeremyfelt8 months ago

  • Keywords has-patch added

+1, this is great.

comment:4 follow-ups: dd328 months ago

The one thing that this patch is lacking, is the fact that by default nginx isn't configured for url rewriting, but we do configure Apache/IIS as best we can for rewrites.

I wish we could detect this via a local HTTP callback + transient cache or something..

comment:5 in reply to: ↑ 4 c3mdigital8 months ago

Replying to dd32:

The one thing that this patch is lacking, is the fact that by default nginx isn't configured for url rewriting, but we do configure Apache/IIS as best we can for rewrites.

I wish we could detect this via a local HTTP callback + transient cache or something..

The HttpRewriteModule is one of the standard modules and the only way for it not to be installed is if Nginx is compiled with the --without-http_rewrite_module flag. That still doesn't mean that the user has rewrite rules properly configured.

One thing we could do is write the configuration in the root web directory and update the codex examples to include this file in their server configurations. This is how W3 Total Cache writes to the nginx config.

One option would be if Nginx is detected create a test rewrite rule then send a HTTP request to see the response code. If we get back a 404 then got_rewrite returns false.

The other problem with writing to the config is the server has to be restarted for the changes to take. Kind of interesting how Drupal does it, http://drupalcontrib.org/api/drupal/contributions!provision!http!Provision!Service!http!nginx.php/6

comment:6 in reply to: ↑ 4 johnbillion8 months ago

Replying to dd32:

The one thing that this patch is lacking, is the fact that by default nginx isn't configured for url rewriting, but we do configure Apache/IIS as best we can for rewrites.

This is being addressed in #18852. The user will be shown the recommended Nginx configuration on the Permalinks screen, in the same way the recommended .htaccess rules are shown if it's not writable, or the recommended IIS web.config if it's not writable.

Effectively, we're treating the Nginx config as not writable. This mirrors the behaviour of WordPress when the server is running Apache and the .htaccess file is not writable.


Replying to c3mdigital:

One thing we could do is write the configuration in the root web directory and update the codex examples to include this file in their server configurations. This is how W3 Total Cache writes to the nginx config.

Note that W3TC doesn't write to the actual Nginx config file. It writes to a dummy file called nginx.conf in the server's document root, and it's up to the user to copy or merge those rules into their actual Nginx config file.

Writing the config rules to a dummy nginx.conf file in the document root serves no purpose; we may as well just show the rules on the Permalinks screen.

comment:7 in reply to: ↑ 4 ; follow-up: rmccue8 months ago

Replying to dd32:

I wish we could detect this via a local HTTP callback + transient cache or something..

That'd be a great feature for all the setups, not just nginx. Something like /wp-internal-test, then just check that we don't get an error.

comment:8 in reply to: ↑ 7 johnbillion8 months ago

+1 to that. Separate ticket anyone?

comment:9 rmccue8 months ago

Split into #25109 for that.

comment:10 desrosj8 months ago

  • Cc desrosj@… added

comment:11 nacin7 months ago

There were some adjustments here that needed to be made for when $home_path is not writable. Handling that in a commit.

comment:12 nacin7 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 25456:

Add nginx detection to the Permalink Settings screen.

Introduces got_url_rewrite() and a corresponding filter, which should now be used in lieu of the got_rewrite filter in got_mod_rewrite().

This does not write or even suggest nginx configuration; rather, it prevents nginx from being considered as either Apache or as an unrecognized server.

props johnbillion.
fixes #25098.

Note: See TracTickets for help on using tickets.