esotalk / esoTalk

Fat-free forum software.
GNU General Public License v2.0
1.47k stars 239 forks source link

Installation on nginx always falls back to non-rewrite URLs #427

Closed jsonnull closed 7 years ago

jsonnull commented 9 years ago

Currently the esoTalk installer checks for apache rewrite functions when setting the rewrite configuration, meaning that an esoTalk install on nginx will always fall back to non-rewrite URLs following the installation process.

This makes it hard for me to configure an nginx-based esoTalk server using Docker, because no matter how I configure the container's starting state, URL rewrites will be off after the user performs the installation.

Currently, I'm using sed after the repo is cloned to patch the offending line by removing the apache checks: sed -i.bak 's/and function_exists("apache_get_modules") and in_array("mod_rewrite", apache_get_modules())//g' core/controllers/ETInstallController.class.php

Obviously this is very ugly! :P

Is there something we can change with that line?

Felli commented 9 years ago

Kind of looks a little similar to this problem here (Which is a lot uglier, which is why I don't use Nginx.) http://esotalk.org/forum/1130-nginx-php-fpm-friendly-url

jsonnull commented 9 years ago

@Felli This is not an issue with nginx configuration at all, it's a problem where the PHP code for the installation assumes it's running on an apache server and an installation on non-apache servers will never have rewrite enabled unless you modify configuration post-install. You would have the same issue with lighttpd or any other server.

inliquid commented 9 years ago

@jsonnull exactly. i have mentioned it in my topic.

jgknight commented 9 years ago

Yeah it was written to work with Apache out of the box. The goal of this piece is to seamlessly setup friendly URLs out of the box for most users. Other web servers were a bonus :)

I should probably check with @tobscure however I think we need to actually fix the use of this config key, not just replace that line in the installer. As far as I can tell, that variable (urls.rewrite) should only be used to generate the .htaccess file. Throughout the codebase we're usually checking the condition of both keys, and I don't know why. It may just be a byproduct of developing originally on Apache and it's just stuck around since then.

At this point of the installer it makes sense to use that config key to know if we need to create the .htaccess file. However I think if somebody turns on friendly URLs, the logic throughout esoTalk should not also check the rewrite key.

jsonnull commented 9 years ago

@jgknight Well there's the possibility of having friendly URLs without rewrites, so that config key has to be used in several places to determine what URL to route the client to.

jgknight commented 9 years ago

@jsonnull ah oops, I forgot there are 3 different URL options, I was thinking just 2.

So yes perhaps pulling out that Apache check would be the easiest option.

jgknight commented 9 years ago

This would keep it simple for those on Apache (which most shared hosting is.) And also wouldn't get in the way of those with other webservers incompatible with the .htaccess file. @jsonnull would this work for your situation?

jsonnull commented 9 years ago

@jgknight I like this but I see one potential problem: if a user mistakenly enables rewrites on a server which doesn't support it, could this make Administration > Forum Settings inaccessible? The user should still have a way to disable rewrites. I'm not sure giving the user an option that could potentially make all of their forum inaccessible is an acceptable option.

jgknight commented 9 years ago

That's true. What could be done is similar to how Drupal and other projects handle rewriting. With javascript, make a request to a rewritten url (/rewrite/test or something) and if it returns a 404, then don't enable rewrites. If it returns a 200, enable rewrites. This would make it universal. You'd then use this to enable or disable the rewrite checkbox on the admin/settings page. This also becomes a lot more work than just running sed :)

jsonnull commented 9 years ago

That's great! It is more work than running sed, but I like that there's a solution that eliminates this problem for future users. I can work on this and send a pull request soon.

On May 9, 2015, at 3:07 PM, Josh Knight notifications@github.com wrote:

That's true. What could be done is similar to how Drupal and other projects handle rewriting. With javascript, make a request to a rewritten url (/rewrite/test or something) and if it returns a 404, then don't enable rewrites. If it returns a 200, enable rewrites. This would make it universal. You'd then use this to enable or disable the rewrite checkbox on the admin/settings page. This also becomes a lot more work than just running sed :)

— Reply to this email directly or view it on GitHub.