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

FIX Correct trailing slash for subsite in another domain #590

Merged

Conversation

GuySartorelli
Copy link
Member

Fixes https://github.com/silverstripe/silverstripe-subsites/actions/runs/10278313787/job/28441800184 which consists of failures like the below:

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'http://two.mysite.com/'
+'http://two.mysite.com/'

As of silverstripe/framework 5.2.20, Controller::normaliseTrailingSlash() won't normalise trailing slash for absolute URLs where the domain doesn't match the current site domain.

This means if getting a link to a page on Subsite A from Subsite B, if the domains don't match, the trailing slash won't be normalised.

It's possible that it would be better to implement the updateIsSiteUrl() extension hook in an extension for Director and saying "yes that's a site URL" if it belongs to a domain of any subsite. I didn't attempt that because I don't know what sort of flow-on effects that might have. I'm especially hesitant to do that in a patch release.

Note that the same problem happened on Fluent, see tractorcow-farm/silverstripe-fluent#872
Most likely we'll want to implement the updateIsSiteUrl() extension hook for both modules in a minor release, but I want at least one other opinion before I do that. Either way, IMO this PR should be merged for a patch fix.

Issue

@GuySartorelli
Copy link
Member Author

@silverstripe/core-team if anyone has a sec to look at this and give an opinion about the updateIsSiteUrl() option for a more robust solution in a minor release, I'd be grateful.

@emteknetnz emteknetnz merged commit 6536e8f into silverstripe:3.2 Aug 13, 2024
12 checks passed
@emteknetnz emteknetnz deleted the pulls/3.2/trailing-slash branch August 13, 2024 05:03
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.

2 participants