Problem/Motivation

with #2343943: Language domain may not be left blank for default language we're using gethostname().
This points on the new testbot infrastructure not to localhost (as it does on the old one), it does point to the real hostname of the docker image (which is random).
So after setting the domain URL for english, drupal redirects to an url which is different from before and the cookie is not correct anymore, so we end up with an Access Denied.

Proposed resolution

Using parse_url($base_url, PHP_URL_HOST); which returns the same Host URI as it is used in WebTestBase::getAbsoluteUrl()

Remaining tasks

Review

User interface changes

none

API changes

none

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Schnitzel’s picture

Schnitzel’s picture

Assigned: Unassigned » Schnitzel
Schnitzel’s picture

Schnitzel’s picture

Status: Active » Needs review
Schnitzel’s picture

Title: #2343943 introduced gethostname() which breaks on new testbot » #2343943 introduced gethostname(), which breaks on new testbot
Schnitzel’s picture

screenshot and log of new testbot run, which proves it works :)

alimac’s picture

+++ b/core/modules/language/src/Tests/LanguageUrlRewritingTest.php
@@ -98,10 +98,13 @@ private function checkUrl($language, $message1, $message2) {
+    // Getting the current host URI we're running on.

Small nit: in the surrounding comments the verb is in command form (Add, Change, etc.) so I think this comment should use "Get" instead of "Getting".

Schnitzel’s picture

addressing #7.

thanks for review :)

alimac’s picture

Status: Needs review » Reviewed & tested by the community

I downloaded the new testbot containers (https://www.drupal.org/project/drupalci_testbot) and ran the tests on them, which passed. I read through the patch and verified that it replaces the previous method of obtaining the hostname (gethostname()) with a more reliable method using parse_url($base_url, PHP_URL_HOST).

Marking as RTBC.

Schnitzel’s picture

Priority: Normal » Critical
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I think we should have a replacement for global $base_url however our concept and what is available on the Symfony request is different. Going forward with this patch to unblock testbot improvements.

Committed 853741a and pushed to 8.0.x. Thanks!

  • alexpott committed 853741a on 8.0.x
    Issue #2350543 by Schnitzel: Fixed #2343943 introduced gethostname(),...
webchick’s picture

Honestly, I don't think we even have to be all that fancy. You can see that the domain below is completely made up (it.example.com) so in this case, we should probably just make it "example.com."

Here's Yet Another Follow-Up where we can try and do that: #2350937: Remove fanciness from various language tests

However, gethostname() is really just a standard PHP function, not anything overly fancy, so it's weird we would not be able to call it and instead have to do workarounds. Filed #2350935: Testbots should not break when gethostname() is called for that.

alimac’s picture

@webchick in this particular test, after submitting the form it redirects to the default language domain URL, so the test would fail unless the host was configured to use example.com (one of my earlier patches used en.example.com: https://www.drupal.org/node/2343943#comment-9192485).

webchick’s picture

Yep, just figured that out in the first sub-issue. :( The point about testbot NG brittleness I think is still worthy of the second issue though, since the workaround here is quite advanced.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

YesCT’s picture

Issue tags: -Amsterdam 2014 +Amsterdam2014

fixing the tag, so I can delete the least used one.