Domain based language detection always use the default language if the HTTP server listen to non 80 port.

$_SERVER['HTTP_HOST'] contains the port number if it is not 80.
$host does not contains port.

Files: 
CommentFileSizeAuthor
#48 i1572394-48.patch1.15 KBcrotown
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#37 i1572394-37.patch1.77 KBattiks
PASSED: [[SimpleTest]]: [MySQL] 39,142 pass(es).
[ View ]
#19 i1572394-19.patch701 bytesattiks
PASSED: [[SimpleTest]]: [MySQL] 36,682 pass(es).
[ View ]
#13 i1572394-13-test.patch712 bytesattiks
FAILED: [[SimpleTest]]: [MySQL] 39,110 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#13 i1572394-13.patch1.73 KBattiks
PASSED: [[SimpleTest]]: [MySQL] 39,111 pass(es).
[ View ]
#8 i1572394-6.patch1.83 KBattiks
PASSED: [[SimpleTest]]: [MySQL] 36,605 pass(es).
[ View ]
#7 i1572394-6-test.patch723 bytesattiks
FAILED: [[SimpleTest]]: [MySQL] 36,592 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#7 i1572394-6.patch723 bytesattiks
FAILED: [[SimpleTest]]: [MySQL] 36,594 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#1 drupal-1572394.patch670 bytesSweetchuck
PASSED: [[SimpleTest]]: [MySQL] 39,053 pass(es).
[ View ]

Comments

Version:8.x-dev» 7.x-dev
Issue tags:-D8MI, -sprint, -language-base
StatusFileSize
new670 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,053 pass(es).
[ View ]

quick fix
Problem with this patch: language detection cannot be port number based.

Version:7.x-dev» 8.x-dev
Status:Active» Needs review
Issue tags:+D8MI, +sprint, +language-base

To me it sounds like it would make more sense to remove the port from $_SERVER['HTTP_HOST'] and then use the resulting value to compare. I assume this applies to Crupal 8 as well, and needs to be fixed there first in that case.

Version:7.x-dev» 8.x-dev
Status:Needs review» Needs work
Issue tags:+D8MI, +sprint, +language-base

BTW the same code in Drupal 8 is in language_from_url() and is in core/modules/language/language.negotiation.inc.

Title:language detection on port non 80Language detection by domain only works on port 80

I've verified that indeed HTTP_HOST woud contain the port number (despite its name). This bug was relatively recently introduced to Drupal 7 I think with #1250800: Language domain should work regardless of ports or protocols, so it was only part of Drupal 7 for some time. Looks like people are not running (multilingual) Drupal sites on non-standard ports too often. Posted there asking for more people interested, but we really need you taking this further. Thanks!

Assigned:Unassigned» attiks

I checked RFC2616 and it states: "The Host request-header field specifies the Internet host and port number of the resource being requested", Host = "Host" ":" host [ ":" port ] and no port means use the default of the requested service.

I'll make a patch removing the port before doing the check.

Status:Needs work» Needs review
StatusFileSize
new723 bytes
FAILED: [[SimpleTest]]: [MySQL] 36,594 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
new723 bytes
FAILED: [[SimpleTest]]: [MySQL] 36,592 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

I don't know if this is the fastest code: $http_host = current(explode(':', $http_host));

StatusFileSize
new1.83 KB
PASSED: [[SimpleTest]]: [MySQL] 36,605 pass(es).
[ View ]

Correct version of i1572394-6.patch

Status:Needs review» Needs work
Issue tags:-D8MI, -sprint, -language-base

The last submitted patch, i1572394-6.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+D8MI, +sprint, +language-base

#8: i1572394-6.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Looks good, should then be backported to D7 too. Let's get this in D8 and 7, so it can be part of the next D7 release too.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)
Issue tags:+needs backport to D7

Thanks. Committed/pushed to 8.x, moving to 7.x for backport.

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.73 KB
PASSED: [[SimpleTest]]: [MySQL] 39,111 pass(es).
[ View ]
new712 bytes
FAILED: [[SimpleTest]]: [MySQL] 39,110 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Looks good to go to me if it passes tests :)

Let's hope the first one fails and only the second passes ;p

+        $http_host = current(explode(':', $http_host));

According to #1517654: Strict warning by field_ui_table_pre_render(), this kind of thing can cause E_STRICT warnings.

Note that I've never been able to reproduce that myself, and don't understand why it would; I can reproduce it with reset(), end(), prev(), etc. because those change the passed-in array, but current() doesn't so I have no idea why PHP would take the array by reference.

However, there are a whole lot of people over on that other issue claiming that it does and that for them, gobs of warnings are being triggered. Maybe it depends on PHP version?

Version:7.x-dev» 8.x-dev
Status:Needs review» Needs work

In that case, the D8 one would need to be fixed first (again). Doh.

I'll reroll both, but I'm also not able to reproduce this. The array is taken by reference for performance reasons.

FYI PHP 5.3.5 & PHP 5.3.10

Status:Needs work» Needs review
StatusFileSize
new701 bytes
PASSED: [[SimpleTest]]: [MySQL] 36,682 pass(es).
[ View ]

Instead of this

<?php
$http_host
= current(explode(':', $http_host));
?>

I generally use
<?php
list($http_host) = explode(':', $http_host);
?>

Robin, i don't mind using your code, but will it make a difference?

I never used current(), so I'm not aware of its implications and changes over the years. But list() is quite safe in my experience, no secondary effects.

From the other thread, these are the settings that triggered the warning:

Linux www.example.com 2.6.18-028stab085.3-ent #1 SMP Mon Mar 21 19:57:43 MSK 2011 i686
PHP Version 5.2.17
error_reporting 6143

@regarding #20
This is fine too as you will always get at least one value.

<?php
list($http_host) = explode(':', $http_host);
?>

This can throw warnings if there were no ":" in the $http_host string.

<?php
list($http_host,$port) = explode(':', $http_host);
?>

@catch which one do you prefer?

Why would list() = explode() throw warnings if there was no :, and how was that avoided then with current(explode()) earlier?

@Gábor: I think the last example from #23 can throw warnings if there's no ':'. Do you prefer #19 or #20?

@attiks: yeah, I asked why? Why would that throw a warning and current(explode()) would not?

current(explode()) can throw a warning because the array passed to current() is passed by reference, but apparently it depends on the PHP version if you get a warning or not. AFAIK list($http_host) = explode(':', $http_host); will never throw a warning. And list($http_host,$port) = explode(':', $http_host); will complain ("Undefined offset: 1") if there's no ':' inside $http_host.

I hope it answers your question.

Oh, shoot. There are two examples in #23, and "This can throw warnings" was inbetween the two. It was supposed to be meant for the second example, not the first I guess. I don't get why are we concerned about the second example though as we don't need that code (to extract the port separately). Why are we wasting our time discussing it? :)

My idea exactly :) , but since I have your attention: Do you prefer #19 or #20?

I don't really care, either works.

RTBC for #19?

Status:Needs review» Reviewed & tested by the community

Sorry guys, I should have made that clearer! It was in response to "list() is quite safe in my experience, no secondary effects".

#19: i1572394-19.patch queued for re-testing.

Version:8.x-dev» 7.x-dev

Committed to 8.x. Thanks! Moving to 7.x.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new1.77 KB
PASSED: [[SimpleTest]]: [MySQL] 39,142 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Straight backport.

Version:7.x-dev» 8.x-dev

This and several other issues look like they weren't actually committed/pushed. So, #19 is still RTBC for Drupal 8.

Pushed it now, I hope. :)

Very big thank you for every body :-)

Status:Reviewed & tested by the community» Fixed
Issue tags:+7.15 release notes

Issue tags:-sprint

Not on the sprint anymore than. Thanks.

Assigned:attiks» Unassigned

Status:Fixed» Closed (fixed)

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

Version:7.x-dev» 6.x-dev
Status:Closed (fixed)» Needs review
Issue tags:-needs backport to D7, -D8MI, -7.15 release notes+6.27 release notes
StatusFileSize
new1.15 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Here is a patch for Drupal 6.x rolled against the current 6.x branch. I did this for for a Acquia client. I want to contribute it back to the community. It is essentially the same as i1572394-19.path but backported to D6.

Issue tags:+D8MI, +7.15 release notes

Putting back tags I removed.

The patch itself looks good. This was a set of issues for Drupal 7. Are you sure this is the only part of the set of issues that applies to Drupal 6? Did you or the customer successfully test/use the patch? Did anybody else test the patch?

I am not sure that this is the only part of larger set of Drupal 7 set of issues that applies to Drupal 6. I thought that my patch covered everything that the previous patch did except for testing code -- but it looks like I did not fully understand the code that I did not backport. Here is what I did...

I tested the patch by making a multilingual local clone of the client's site and verifying that a URL like fr.example.local:8080 is now seen as equivalent to fr.example.local and selects the French version of example.local.

The client issue was that they wanted to set up a local site that worked the same way as their live site example.com, where picking "French" out of a dropdown menu redirects to "fr.example.com" and results in a French version of the site be served using the same database that serves all the different languages. This worked for the local site after applying my patch, and not before since the ":8080"s caused ALL of the comparisons to fail when checking for the versions of the URL mapping to different language version of the site. I performed the test by writing the strings that were compared into a temporary file. I made the patch by removing that one line after the code was working (and testing once again).

I haven't heard back from the customer yet, to see if it is working for them, as well. I will check with them an report back here.

The customer has not tried the patch yet.

They have just switched to MAMP Pro for a local environment. It seems they have not grappled with the fact that this issue will recur as long as they need to include ":xxxx" to get a local site. Or perhaps they have configured around it.

A developer there is interested trying the patch and getting involved in this issue.

Related: #1645156: URL generation only works on port 80 (that is about generation not detection).