[MDL-39023] Circular dependency in parentlanguage definition may easily lead to infinitive loops - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Critical
Resolution: Fixed
Affects Version/s: 2.1.10, 2.2.9, 2.3.6, 2.4.3, 2.5
Fix Version/s: 2.3.7, 2.4.4
Component/s: Language
Labels:
- triaged

Testing Instructions:
Hide

Testing difficulty: medium (requires direct access to moodledata)

1. Install some language pack (let's call it 'xx')
2. Edit $CFG->dataroot/lang/xx/langconfig.php
3. Change/add the parent language declaration and point it to the language itself:

$string['parentlanguage'] = 'xx';

4. TEST: Make sure the site (e.g. the front page) is still available (it died in infinitive loop before).
Show
Testing difficulty: medium (requires direct access to moodledata) 1. Install some language pack (let's call it 'xx') 2. Edit $CFG->dataroot/lang/xx/langconfig.php 3. Change/add the parent language declaration and point it to the language itself: $string['parentlanguage'] = 'xx'; 4. TEST: Make sure the site (e.g. the front page) is still available (it died in infinitive loop before).
Affected Branches:
MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
Fixed Branches:
MOODLE_23_STABLE, MOODLE_24_STABLE
Pull from Repository:
git://github.com/mudrd8mz/moodle.git
Pull Master Branch:
~~MDL-39023~~-parentlang
Pull Master Diff URL:
https://github.com/mudrd8mz/moodle/compare/master...MDL-39023-parentlang

Description

This was just observed at lang.moodle.org - the maintainer of he_kids language pack set the parentlanguage of it to the he_kids itself (instead of expected he). This creates circular dependency problem in string_manager::get_language_dependecies() and the site with the lang installed dies in infinitive loop hell.

Gliffy Diagrams

Attachments

Issue Links

has been marked as being related by

MDLSITE-2194 html error 500 on lang.moodle.org

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Koen Roggemans added a comment - 08/Apr/13 8:41 AM

Wow - that's a silly mistake. Probably one of the milion possibilities in how to break a Moodle site by making errors in a language pack.

Show

Koen Roggemans added a comment - 08/Apr/13 8:41 AM Wow - that's a silly mistake. Probably one of the milion possibilities in how to break a Moodle site by making errors in a language pack.

Hide

Permalink

David Mudrák added a comment - 08/Apr/13 9:09 AM

Yeah, especially langconfig.php seems to be tricky. The other day, we were talking about how ISO codes are broken across lang packs (see MDLSITE-2191). Maybe it's time to actually start managing langconfig.php independently on other strings and manage it centrally (an by AMOS admins only, not by maintainers themselves).

Show

David Mudrák added a comment - 08/Apr/13 9:09 AM Yeah, especially langconfig.php seems to be tricky. The other day, we were talking about how ISO codes are broken across lang packs (see MDLSITE-2191 ). Maybe it's time to actually start managing langconfig.php independently on other strings and manage it centrally (an by AMOS admins only, not by maintainers themselves).

Hide

Permalink

Koen Roggemans added a comment - 08/Apr/13 9:14 AM

Not all can be done by AMOS admins I'm affraid (alphabet, thousend separator, ...), but I aggree that the technical ones could do with protection and be our responsability that they are correct.

Show

Koen Roggemans added a comment - 08/Apr/13 9:14 AM Not all can be done by AMOS admins I'm affraid (alphabet, thousend separator, ...), but I aggree that the technical ones could do with protection and be our responsability that they are correct.

Hide

Permalink

Ralf Krause added a comment - 08/Apr/13 10:32 AM - edited

There seems to be a very important problem.

What should a language maintainer write into the field "parentlanguage" within the component "core_langconfig"? I think that the field should stay empty in the german language pack "de". If I would write de into this field than the son would be the same person as his father and the father would be the same as the grandfather.

Or should I write "en" into this field because the fallback is "en" ... every maintainer tries to get 100%

Show

Ralf Krause added a comment - 08/Apr/13 10:32 AM - edited There seems to be a very important problem. What should a language maintainer write into the field "parentlanguage" within the component "core_langconfig"? I think that the field should stay empty in the german language pack "de". If I would write de into this field than the son would be the same person as his father and the father would be the same as the grandfather. Or should I write "en" into this field because the fallback is "en" ... every maintainer tries to get 100%

Hide

Permalink

David Mudrák added a comment - 08/Apr/13 12:25 PM

@Ralf Krause: The correct value for the parentlanguage of 'de' is empty value as the language does not have any explicit parent language. IIRC, this string is excluded from missing strings stats in AMOS so it won't block you from reaching 100% of translated strings. Having explicit 'en' there is tolerated, too. Having 'de' there would cause the infinite loop problem (and that is what this issue is about).

Show

David Mudrák added a comment - 08/Apr/13 12:25 PM @ Ralf Krause : The correct value for the parentlanguage of 'de' is empty value as the language does not have any explicit parent language. IIRC, this string is excluded from missing strings stats in AMOS so it won't block you from reaching 100% of translated strings. Having explicit 'en' there is tolerated, too. Having 'de' there would cause the infinite loop problem (and that is what this issue is about).

Hide

Permalink

David Mudrák added a comment - 08/Apr/13 1:10 PM

Submitting a patch for integration. The patch reimplements the core_string_manager::get_language_dependencies() method so that potentially mis-configured language packs with circular dependencies or self dependency do not make the site unavailable. Unit tests for the expected behaviour are added.

Show

David Mudrák added a comment - 08/Apr/13 1:10 PM Submitting a patch for integration. The patch reimplements the core_string_manager::get_language_dependencies() method so that potentially mis-configured language packs with circular dependencies or self dependency do not make the site unavailable. Unit tests for the expected behaviour are added.

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 08/Apr/13 2:58 PM

Integrated (23, 24 & master), thanks!

Show

Eloy Lafuente (stronk7) added a comment - 08/Apr/13 2:58 PM Integrated (23, 24 & master), thanks!

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 08/Apr/13 3:37 PM - edited

unit tests passed @ CI servers

Edited, grrr, typo.

Show

Eloy Lafuente (stronk7) added a comment - 08/Apr/13 3:37 PM - edited unit tests passed @ CI servers Edited, grrr, typo.

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 08/Apr/13 6:35 PM

Passing, I've reproduced the problem without the patch and works with it applied.

Ciao

Show

Eloy Lafuente (stronk7) added a comment - 08/Apr/13 6:35 PM Passing, I've reproduced the problem without the patch and works with it applied. Ciao

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 11/Apr/13 12:29 PM

Your awesome contributions are now part of Moodle, your fav LMS out there.

Closing this as fixed.

Many thanks for all the hard work, ciao

Show

Eloy Lafuente (stronk7) added a comment - 11/Apr/13 12:29 PM Your awesome contributions are now part of Moodle, your fav LMS out there. Closing this as fixed. Many thanks for all the hard work, ciao

People

Assignee:

David Mudrák

Reporter:

David Mudrák

Integrator:

Eloy Lafuente (stronk7)

Tester:

Eloy Lafuente (stronk7)

Participants:

David Mudrák, Eloy Lafuente (stronk7), Koen Roggemans, Ralf Krause

Votes:

0 Vote for this issue

Watchers:

4 Start watching this issue

Dates

Created:

08/Apr/13 8:29 AM

Updated:

11/Apr/13 12:29 PM

Resolved:

11/Apr/13 12:29 PM

Fix Release Date:

13/May/13