Currently, the regex used by locale_language_from_browser() to match language strings will incorrectly match invalid language strings. As an example, the random 10-character string iecRswK4eh will incorrectly result in eh-oh-laa-laa because there is a numeral in the string, and the regex does not require that each string be preceeded by the beginning of the string or a comma (with optional whitespace).

CommentFileSizeAuthor
#8 core-failing_langage_test-1513520-8.patch735 bytespdrake
#7 core-fix_nonspecific_language_matching_regex-1513520-7.patch1.54 KBplach
#3 core-failing_langage_test-1513520-3.patch755 bytespdrake
#1 core-fix_nonspecific_language_matching_regex-1513520-1.patch1.58 KBpdrake

Comments

pdrake’s picture

pdrake commented
Status: Active » Needs review
StatusFileSize
new core-fix_nonspecific_language_matching_regex-1513520-1.patch1.58 KB
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +D8MI, +sprint, +language-base

Looks good. It just adds a look-behind pattern before the existing pattern. Can you also submit just the test change so we can demonstrate it failing? Thanks!

pdrake’s picture

pdrake commented
StatusFileSize
new core-failing_langage_test-1513520-3.patch755 bytes

This patch includes both an added test for a comma-space separated set of languages which should pass (and the change continues to pass) as well as the test for the bad string, which should fail.

Status: Needs review » Needs work

The last submitted patch, core-failing_langage_test-1513520-3.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Reviewed & tested by the community

Looks like this is RTBC. Patch to be committed in #1 (kudos ;)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Version: 8.x-dev » 7.x-dev
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new core-fix_nonspecific_language_matching_regex-1513520-7.patch1.54 KB

Just applied the patch in #1 with -p2 and rerolled, thus setting back to RTBC.

pdrake’s picture

pdrake commented
StatusFileSize
new core-failing_langage_test-1513520-8.patch735 bytes

And here is a re-roll of #3 which should show the failure in D7.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, core-failing_langage_test-1513520-8.patch, failed testing.

pdrake’s picture

pdrake commented
Status: Needs work » Reviewed & tested by the community

Since the patch in #8 was intended to fail - setting this back to RTBC (patch in #7).

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented
Title: locale_language_from_browser() incorrectly matches invalid strings » HEAD BROKEN: locale_language_from_browser() incorrectly matches invalid strings
Priority: Normal » Critical
Status: Fixed » Reviewed & tested by the community

The test-only patch from #8 was committed. We need a rollback, and/or just to commit the fix part from #7. :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Please commit the fix :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: HEAD BROKEN: locale_language_from_browser() incorrectly matches invalid strings » locale_language_from_browser() incorrectly matches invalid strings
Status: Reviewed & tested by the community » Fixed

Doh. :( Core commit sprees after 1 am--;

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Critical » Normal

Yay.

Tor Arne Thune’s picture

Tor Arne Thune commented
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -sprint

Thanks.

Status: Fixed » Closed (fixed)

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

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Issue tags: +Needs backport to D7

Updating tags per http://drupal.org/node/1517250