New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nginx staticfiles.conf interferes with rewrites of *.html urls #35

Closed
jeffwidman opened this Issue Dec 17, 2014 · 7 comments

Comments

Projects
None yet
2 participants
@jeffwidman

jeffwidman commented Dec 17, 2014

I recently migrated a forum from Simple Machines to Xenforo and needed to rewrite the SMF urls to Xenforo urls.

Old url: domain.com/forum/index.php/topic,101126.msg38.html
Rewritten url: domain.com/forum/index.php?posts/38

Here's my Nginx rewrite rule:
rewrite /forum/index\.php/topic,[0-9]+\.\bmsg([0-9]+)\.html /forum/index.php?posts/$1 redirect;

However, when I apply this to my Centmin Vhost file, it doesn't work. After much trial and error, I realized
the issue is in the global file include /usr/local/nginx/conf/staticfiles.conf;

The rewrite works perfectly if I do either of the following:
a) strip .html from both the old url and the rewrite regex.
b) comment out this include.

Option A is impossible since I've got thousands of inbound links that include the .html
After looking at what's in staticfiles.conf, I'd prefer to avoid option B because there's a lot of useful stuff in there...

Is there any way this global file can be tweaked to allow rewriting links in the vhost config files that point to .html files?

I suspect I could just edit staticfiles.conf and put the rewrite in there, but I'd prefer to avoid that to allow my vhost files to stay localized and not touch global centminmod configs so that it's easy to upgrade later...

I'm on Centminmod .008 beta because I wanted to use Centos 7.

@jeffwidman

This comment has been minimized.

Show comment
Hide comment
@jeffwidman

jeffwidman Dec 17, 2014

Figured out another way around it--I had my rewrite nested within my forum location, so that it would only be parsed when the url was within the forum.

However, Nginx was skipping the /forum/ prefix location since the static files regex matched. Nginx always picks a regex over a location unless you use the ^~ modifier on the location, in which case it immediately ends.

Solution: either add ^~ modifer to the forum prefix location, or move the rewrites to the same location context as the static file includes. On the surface it seems better to add the ^~ modifier, as that way the regex is only evaluated if the url is within the forum, but this potentially prevents the optimizations in the staticfiles.conf from being applied to any static urls within the /forum/ location--even those that aren't rewritten. So it's actually far simpler to just move the rewrites to the parent context...

Closing, as this isn't actually a bug, just me not thinking through how nginx was parsing the locations...

jeffwidman commented Dec 17, 2014

Figured out another way around it--I had my rewrite nested within my forum location, so that it would only be parsed when the url was within the forum.

However, Nginx was skipping the /forum/ prefix location since the static files regex matched. Nginx always picks a regex over a location unless you use the ^~ modifier on the location, in which case it immediately ends.

Solution: either add ^~ modifer to the forum prefix location, or move the rewrites to the same location context as the static file includes. On the surface it seems better to add the ^~ modifier, as that way the regex is only evaluated if the url is within the forum, but this potentially prevents the optimizations in the staticfiles.conf from being applied to any static urls within the /forum/ location--even those that aren't rewritten. So it's actually far simpler to just move the rewrites to the parent context...

Closing, as this isn't actually a bug, just me not thinking through how nginx was parsing the locations...

@jeffwidman jeffwidman closed this Dec 17, 2014

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod

centminmod Dec 18, 2014

Owner

Actually, the usual fix is to remove the htm/html location match from staticfiles.conf or not using rewrites with .html extensions and let Nginx server html files rather than forcing PHP-FPM to server .html files (better performance).

Owner

centminmod commented Dec 18, 2014

Actually, the usual fix is to remove the htm/html location match from staticfiles.conf or not using rewrites with .html extensions and let Nginx server html files rather than forcing PHP-FPM to server .html files (better performance).

@jeffwidman

This comment has been minimized.

Show comment
Hide comment
@jeffwidman

jeffwidman Dec 18, 2014

Hmm... I don't understand why what you're suggesting is the usual fix.

If I understand you correctly, you're saying that normally the fix is:

a) remove _.html location match from staticfiles.conf, thereby causing Nginx to send all _.html matches to PHP-FPM

b) don't rewrite *.html urls so that Nginx serves them.

Neither option seems like a good idea.

Option a, as you observed, is less performant since PHP-FPM now handles all *.html files, not just the rewritten ones.

Option b defeats the entire purpose of what I'm trying to accomplish, which is 301 redirect the thousands of *.html links scattered around the internet pointing to my forum site. They need to be 301'd to Xenforo urls.

Why isn't the usual fix just to put a few very specific regexes at the same level of staticfiles.conf?

Yes, those regexes are evaluated on every inbound request, but only a small percentage of those requests are rewritten and passed onto PHP-FPM... the rest are quickly discarded as not matching the regex. Seems far more performant, while still accomplishing the rewrites.

jeffwidman commented Dec 18, 2014

Hmm... I don't understand why what you're suggesting is the usual fix.

If I understand you correctly, you're saying that normally the fix is:

a) remove _.html location match from staticfiles.conf, thereby causing Nginx to send all _.html matches to PHP-FPM

b) don't rewrite *.html urls so that Nginx serves them.

Neither option seems like a good idea.

Option a, as you observed, is less performant since PHP-FPM now handles all *.html files, not just the rewritten ones.

Option b defeats the entire purpose of what I'm trying to accomplish, which is 301 redirect the thousands of *.html links scattered around the internet pointing to my forum site. They need to be 301'd to Xenforo urls.

Why isn't the usual fix just to put a few very specific regexes at the same level of staticfiles.conf?

Yes, those regexes are evaluated on every inbound request, but only a small percentage of those requests are rewritten and passed onto PHP-FPM... the rest are quickly discarded as not matching the regex. Seems far more performant, while still accomplishing the rewrites.

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod

centminmod Dec 18, 2014

Owner

Guess it's context relevant as the usual fix for Centmin Mod is only for folks using Wordpress and choosing to use permalinks with .html extension at the end. It isn't really relevant or applicable for your Simple Machines forums rewrite urls. It's where wordpress with permalink .html extension force PHP-FPM to serve html files.

Removing .html/html match in staticfiles.conf will still be served via Nginx, just without the expires headers custom configured so they do get served by Nginx. Which actually in your case should be the easiest way of resolving the issue.

The staticfiles.conf is called from all Nginx vhosts via include so has to be as general as possible for all or nearly all usage scenarios. The limitations of an auto installer setup and configuration that tries to cater to the masses

Owner

centminmod commented Dec 18, 2014

Guess it's context relevant as the usual fix for Centmin Mod is only for folks using Wordpress and choosing to use permalinks with .html extension at the end. It isn't really relevant or applicable for your Simple Machines forums rewrite urls. It's where wordpress with permalink .html extension force PHP-FPM to serve html files.

Removing .html/html match in staticfiles.conf will still be served via Nginx, just without the expires headers custom configured so they do get served by Nginx. Which actually in your case should be the easiest way of resolving the issue.

The staticfiles.conf is called from all Nginx vhosts via include so has to be as general as possible for all or nearly all usage scenarios. The limitations of an auto installer setup and configuration that tries to cater to the masses

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod

centminmod Dec 18, 2014

Owner

also other option is make a copy of staticfiles.conf i.e. staticfiles_custom.conf and use that include for just your specific Nginx vhost with what you need and remove the staticfile.conf include from this specific Nginx vhost

as to specific regexes, i do not provide support for custom regexes for specific apps, so end user would be responsible for such and 66% of Centmin Mod users wouldn't even know how to write regexes for such, so easier to just remove the .html location context matches in staticfiles.conf :)

Owner

centminmod commented Dec 18, 2014

also other option is make a copy of staticfiles.conf i.e. staticfiles_custom.conf and use that include for just your specific Nginx vhost with what you need and remove the staticfile.conf include from this specific Nginx vhost

as to specific regexes, i do not provide support for custom regexes for specific apps, so end user would be responsible for such and 66% of Centmin Mod users wouldn't even know how to write regexes for such, so easier to just remove the .html location context matches in staticfiles.conf :)

@jeffwidman

This comment has been minimized.

Show comment
Hide comment
@jeffwidman

jeffwidman Dec 18, 2014

Yeah, that all makes sense. It's definitely not a bug, I just didn't realize there were regexes buried in the include...

Might consider adding a comment to the Nginx page on centminmod saying:

Note: Beware of staticfiles.conf interfering with rewrites of static files, especially if you're using wordpress permalinks that end in .html. For more see here: #35

And then just leave it at that... most folks would glaze right over it, but the devs who are searching for answers to why their permalinks/rewrites aren't working as expected would hopefully stumble across it...

Regardless, thanks again for your hard work on Centminmod. Massive time saver for getting up and running quickly with an optimized VPS.

jeffwidman commented Dec 18, 2014

Yeah, that all makes sense. It's definitely not a bug, I just didn't realize there were regexes buried in the include...

Might consider adding a comment to the Nginx page on centminmod saying:

Note: Beware of staticfiles.conf interfering with rewrites of static files, especially if you're using wordpress permalinks that end in .html. For more see here: #35

And then just leave it at that... most folks would glaze right over it, but the devs who are searching for answers to why their permalinks/rewrites aren't working as expected would hopefully stumble across it...

Regardless, thanks again for your hard work on Centminmod. Massive time saver for getting up and running quickly with an optimized VPS.

@centminmod

This comment has been minimized.

Show comment
Hide comment
@centminmod
Owner

centminmod commented Dec 19, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment