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

dev/drupal#85 Drupal8: Fix bug with empty language prefix mangling https:// to http:/ #15912

Merged
merged 1 commit into from
Jan 26, 2020

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Nov 21, 2019

Overview

On a Drupal 8 multilingual site, if you are using URL "Path prefix" language negotiation, and set one of the prefixes to empty (so, for that language, there is no prefix), then CiviCRM resource URLs will get mangled to be "https:/" (one slash) instead of "https://" (two slashes)

Here's what the config would look like:

Selection_058

Before

On CiviCRM pages, it's trying to load CSS and JS from URLs like "https:/example.org" (one slash) rather than "https://example.org" (two slashes)

After

The right URL gets used!

Technical Details

The problem is in CRM_Utils_System_Drupal8::languageNegotiationURL():

            if ($removeLanguagePart) {
              $url = str_replace("/" . $config['prefixes'][$language] . "/", '/', $url);
            }

If $config['prefixes'][$language] is an empty string, then we end up with str_replace("//", "/", $url) which is taking the double slash in "https://" and turning it into "https:/"

@civibot
Copy link

civibot bot commented Nov 21, 2019

(Standard links)

@civibot civibot bot added the master label Nov 21, 2019
@seamuslee001
Copy link
Contributor

This looks fine to me @totten ?

@totten
Copy link
Member

totten commented Nov 21, 2019

Looks reasonable to me (though I don't use language prefixes, so I'm weak to the power of suggestion 🙃).

@mlutfy mlutfy changed the title [Drupal 8] Fix bug with empty language prefix mangling https:// to http:/ dev/drupal#85 Drupal8: Fix bug with empty language prefix mangling https:// to http:/ Nov 21, 2019
@mlutfy
Copy link
Member

mlutfy commented Nov 21, 2019

This seem to fix: https://lab.civicrm.org/dev/drupal/issues/85

I'm a bit behind in my upgrades and would need time to test, but the patch makes sense.

Could we backport to 5.20-rc? It's not a recent regression, but a few people had complained about this bug.

@seamuslee001
Copy link
Contributor

@mlutfy have you had a chance to test this?

@mlutfy mlutfy merged commit 05a99b6 into civicrm:master Jan 26, 2020
@mlutfy
Copy link
Member

mlutfy commented Jan 26, 2020

I couldn't replicate the issue, but the code makes sense. I did a bit more testing and didn't see any impact. Thanks @dsnopek !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants