silverstripe / cwp

Common Web Platform (CWP) features module. We strongly recommend using it for all new CWP projects. Future features will be delivered here.
https://www.cwp.govt.nz
BSD 3-Clause "New" or "Revised" License
10 stars 27 forks source link

Usage of /public/.htaccess and /.htaccess #267

Closed amolswnz closed 3 years ago

amolswnz commented 4 years ago

The instruction for usage of public/.htaccess is unclear and confusing.

Updated explanation and usage of public/.htaccess and root .htaccess.

dhensby commented 4 years ago

Please provide a description to explain what you're changing and why.

dhensby commented 4 years ago

OK - I'm not sure that this change is quite right and I also think it has the potential to be more confusing. There are a few details that are just "dropped in" (like security headers - what are they doing in the redirection howto?) and need some proper feedback from those that know CWP better than I do

emteknetnz commented 4 years ago

@amolswnz could you retarget this PR to 2 instead of master

michalkleiner commented 4 years ago

Can we add some clarification on the reason why redirection rules would go into the root .htaccess on CWP when the site is using the /public folder, which is where the document root of the webserver should be pointed to on any other hosting? How would it then pick up the root .htaccess rules if that wasn't the case?

emteknetnz commented 4 years ago

@amolswnz two things I think we need here before we can merge this in:

a) better clarification around "on CWP you almost always need to redirect to the /public folder" - this probably needs to be in a section that's not in the same block as domain redirection. I'd say create a new section at the top of this file

b) target this PR at 2 instead of master which effectively targets "CWP 3". You'll needs to rebase your work on top or 2, or start a new pull-request branched off 2 and link to that PR from this one and close this if that's easier

emteknetnz commented 3 years ago

It seems like there's going to be no further activity on this pull request, so we've closed it for now. Please open a new pull-request if you want to re-approach this work, or for anything else you think could be fixed or improved.

michalkleiner commented 3 years ago

I think this was addressed through https://github.com/silverstripe/cwp/pull/284, so ok to close.