fix: by default CanonicalURLMiddleware should run in all environments

[wilr]

Description

Currently CanonicalURLMiddleware only runs in production, this leads to the classic example of 'It works on mine(tm)' but more dangerously 'It works on the test site (tm)'. Out of the box, we should have as close to production behaviour as possible.

While the benefits don't apply to dev / test environments the side effects of the behaviour definitely apply.

cc @madmatt

Manual testing steps

Issues

• CanonicalURLMiddleware should support trailing slash redirection #8882
• Run CanonicalURLMiddleware in dev / test. #11167

Pull request checklist

• The target branch is correct
  • See picking the right version
• All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
• The commit messages follow our commit message guidelines
• The PR follows our contribution guidelines
• Code changes follow our coding conventions
• This change is covered with tests (or tests aren't necessary for this change)
• Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• CI is green

[madmatt]

Pushing this to dev might be a bit over the top (though having TLS on localhost is possible and is much more common in these days of Docker etc), but I definitely think it should apply to test and live because these environments should always be considered identical (in as many ways as possible).

[wilr]

Forcing SSL is still not enabled by default in dev. You'll need to opt in via Injector as you would currently.

[madmatt]

Oh okay I did miss that part, nice _b

[mandrew]

I got caught out on this yesterday :( however it's a big change and might be best to wait to 6? Also any documentation that points to CanonicalURLMiddleware working only in live mode will need to be changed

[GuySartorelli]

Please use the pull request template, which I have added back for you. The following need to be done:

• open a new issue so that we have an issue to track. We track issues, not pull requests.
• add some steps to test this change.
• Tick all the checkboxes that are relevant - you may find there are things that the checkboxes will point out (e.g. for a bug fix 5 is not the correct branch to target)

[GuySartorelli]

We can't make this change outside of a major release without breaking some peoples' workflows.
Depending on what the issue actually is that you're trying to resolve, there might be another way to tackle it - but if this does turn out to be the correct way to resolve this problem, the PR will need to target the 6 branch and have an associated PR for the CMS 6 changelog.

[wilr]

Updated now!

[GuySartorelli]

Thanks! Can you please open a pull request to the 6 branch of developer docs repo to update the CMS 6 changelog (you may need to add a new file for it)?
Please also create an issue and link the two PRs to it so I can track this.

[wilr]

@GuySartorelli updated to v6 now. If you want to see the issue in place have a fetch('page.url') somewhere on your page. You'll find this works on development environment, once you're in production this will return a 301 redirect to page.url/

[GuySartorelli]

You'll find this works on development environment, once you're in production this will return a 301 redirect to page.url/

I assume you mean this happens from a fresh installation - there are multiple items of configuration that could change this result, including setting the enabled environments via injector config.

[GuySartorelli]

I'll be happy to merge this once there is a matching PR to the silverstripe/developer-docs as per #11154 (comment)

[wilr]

I'll be happy to merge this once there is a matching PR to the silverstripe/developer-docs as per #11154 (comment)

Won't it just be automated generated as it's a bug fix?

[GuySartorelli]

Won't it just be automated generated as it's a bug fix?

The only things that are automatically generated for changelogs are:

• commit references
• deleted or changes API signatures (classes, interfaces, methods, etc) in minor releases

Changes to default values in protected member properties are not automatically documented in changelogs - and the potential ramifications and how to change those settings yourself are definitely not automatically documented.

[wilr]

Done! Created at silverstripe/developer-docs#465

[GuySartorelli]

Fantastic. Can you please also create an issue and link both PRs to it as per #11154 (comment)?

It's harder to track PRs than issues, especially when there are multiple related PRs.

[GuySartorelli]

Thanks for making this change