Skip to content
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

Create .htaccess and updated public/.htaccess #213

Merged
merged 3 commits into from
Oct 5, 2021
Merged

Create .htaccess and updated public/.htaccess #213

merged 3 commits into from
Oct 5, 2021

Conversation

AntoninoM90
Copy link
Contributor

No description provided.

@l0gicgate
Copy link
Member

I don't use Apache, I just NginX + PHP-FPM so I don't know if I can approve this PR. @akrabat @tupola feel free to chime in.

RewriteRule ^$ public/ [L]
RewriteRule (.*) public/$1 [L]

# Redirect to HTTPS

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's enough to have that just in public/.htaccess

@@ -1,6 +1,18 @@
+Options All –Indexes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess Options and <Files> directives could come in another PR

@@ -16,6 +28,7 @@
# absolute physical path to the directory that contains this htaccess file.
# RewriteBase /

RewriteCond %{REQUEST_FILENAME} !-d

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not necessary, as it allows directory listing (some hosts don't support Options All –Indexes)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This directive checks for a directory in the file system. These are the usual recommendations for configuring Apache Server. Here is an example of recommendations from the Slim documentation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thanks

@l0gicgate
Copy link
Member

@piotr-cz is this PR good to go? I'm not good with Apache configs as I don't use it.

@piotr-cz
Copy link

piotr-cz commented Oct 5, 2021

@l0gicgate
This PR handles case when document root may be set to project root instead of public directory.
In my opinion it's an exceptional case, but it may help some folks.

@l0gicgate
Copy link
Member

@piotr-cz thanks for your response

@l0gicgate l0gicgate merged commit c76b91f into slimphp:master Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants