Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-13868

Adding multiple language files for acp/mcp/ucp modules is incorrectly handled for extensions

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.1.5-RC1
    • Component/s: Extensions, Other
    • Labels:
      None

      Description

      function add_mod_info() in includes/functions_module.php incorrectly handles modules lang files for extensions, namely (see line https://github.com/phpbb/phpbb/blob/3.1.x/phpBB/includes/functions_module.php#L1089):
      1) array_unique() doesn't allow using multiple info_Xcp_xxx.php language files per extension.
      2) the order in which language files are loaded is incorrect/suboptimal as $user_lang_files values are being overriden with $english_lang_files and $default_lang_files values in certain circumstances.

      IRC discussion:

      [01:09]	rxu: the thing is that if you have, say, 2 acp files, 2nd one will be filtered out
      [01:09]	rxu: for the 1 ext
      [01:10]	rxu: because values are the same (which are equal to the ext name)
      [01:11]	bantu: yeah
      [01:12]	bantu: array_merge() already does what is required
      [01:13]	bantu: even if multiple arrays contain the same map entry, the map entry will only be there once in the resulting array
      [01:13]	bantu: due to array_merge using keys
      [01:13]	rxu: indeed
      [01:18]	rxu: but getting rid of array_unique() is not enough as it leaads to overriding user lang values with the english or default (if any) ones
      [01:19]	bantu: but that is also the case currently?
      [01:20]	bantu: actually I don't know what finder returns
      [01:20]	bantu: absolute paths?
      [01:21]	rxu: not quite, it returns pairs like that
      [01:21]	rxu: 'ext/rxu/PostsMerging/language/en/info_acp_posts_merging.php' => string 'rxu/PostsMerging'
      [01:21]	rxu: 'string' part is a leftover of var_dump() output :P
      [01:21]	rxu: 'ext/rxu/PostsMerging/language/en/info_acp_posts_merging.php' => 'rxu/PostsMerging'
      [01:22]	bantu: hum
      [01:24]	bantu: i think these are different issues in any case
      [01:24]	bantu: the array_unique is clearly wrong
      [01:24]	bantu: consindering what array values are
      [01:25]	bantu: or hmm
      [01:25]	bantu: now I am confused
      [01:26]	bantu: maybe this was just intentional?
      [01:26]	bantu: as this is the legacy info file?
      [01:26]	bantu: but then we wouldn't be using finder
      [01:28]	rxu: yeah
      [01:29]	bantu: next I think you're saying that the order in which language files are loaded is incorrect/suboptimal
      [01:29]	bantu: I would probably agree with that.
      [01:30]	bantu: English, Default, User is probably better order
      [01:30]	bantu: array_merge should preserve the order
      [01:30]	rxu: exactly
      [01:31]	bantu: actually, array_merge never overwrites any keys as they are distinct.
      [01:33]	bantu: thus the array merge isn't actually necessary and we could just iterate over all three arrays seperately
      [01:37]	rxu: true, again
      [01:38]	rxu: although 1 foreach is more handy than 3 :P
      [01:39]	bantu: foreach with level 2 of course
      [01:39]	bantu: the array_merge is probably fine, though
      

        Activity

        Hide
        bpetty Bryan Petty added a comment -

        I just wanted to confirm that I've run into the same issue with my extension. I've inspected a request with my language set to Russian with my extension enabled (both the board and my extension have Russian language files installed):

        Taking a look at add_mod_info(), this is the method that automatically loads the extension "info_acp_*.php" language files (copied here for posterity and convenience):

        $lang_files = array_unique(array_merge($user_lang_files, $english_lang_files, $default_lang_files));
        foreach ($lang_files as $lang_file => $ext_name)
        {
        	$user->add_lang_ext($ext_name, $lang_file);
        }
        

        Here's what comes in with my extension:

          'user_lang_files' => 
            array (size=2)
              'ext/tierra/topicsolved/language/ru/info_acp_forums.php' => string 'tierra/topicsolved' (length=18)
              'ext/tierra/topicsolved/language/ru/info_acp_styles.php' => string 'tierra/topicsolved' (length=18)
              
          'english_lang_files' => 
            array (size=0)
              empty
              
          'default_lang_files' => 
            array (size=2)
              'ext/tierra/topicsolved/language/en/info_acp_forums.php' => string 'tierra/topicsolved' (length=18)
              'ext/tierra/topicsolved/language/en/info_acp_styles.php' => string 'tierra/topicsolved' (length=18)
        

        Note that both the "forums" and "styles" files were found in both the default English, and the user's Russian language files. Here's what happens in the call to array_merge:

          'merged_files' => 
            array (size=4)
              'ext/tierra/topicsolved/language/ru/info_acp_forums.php' => string 'tierra/topicsolved' (length=18)
              'ext/tierra/topicsolved/language/ru/info_acp_styles.php' => string 'tierra/topicsolved' (length=18)
              'ext/tierra/topicsolved/language/en/info_acp_forums.php' => string 'tierra/topicsolved' (length=18)
              'ext/tierra/topicsolved/language/en/info_acp_styles.php' => string 'tierra/topicsolved' (length=18)
        

        And here's the final result after the call to array_unique:

          'lang_files' => 
            array (size=1)
              'ext/tierra/topicsolved/language/ru/info_acp_forums.php' => string 'tierra/topicsolved' (length=18)
        

        Note that "info_acp_styles.php" is no longer included from any language.

        Show
        bpetty Bryan Petty added a comment - I just wanted to confirm that I've run into the same issue with my extension. I've inspected a request with my language set to Russian with my extension enabled (both the board and my extension have Russian language files installed): Taking a look at add_mod_info(), this is the method that automatically loads the extension "info_acp_*.php" language files (copied here for posterity and convenience): $lang_files = array_unique ( array_merge ( $user_lang_files , $english_lang_files , $default_lang_files )); foreach ( $lang_files as $lang_file => $ext_name ) { $user ->add_lang_ext( $ext_name , $lang_file ); } Here's what comes in with my extension: 'user_lang_files' => array (size=2) 'ext/tierra/topicsolved/language/ru/info_acp_forums.php' => string 'tierra/topicsolved' (length=18) 'ext/tierra/topicsolved/language/ru/info_acp_styles.php' => string 'tierra/topicsolved' (length=18) 'english_lang_files' => array (size=0) empty 'default_lang_files' => array (size=2) 'ext/tierra/topicsolved/language/en/info_acp_forums.php' => string 'tierra/topicsolved' (length=18) 'ext/tierra/topicsolved/language/en/info_acp_styles.php' => string 'tierra/topicsolved' (length=18) Note that both the "forums" and "styles" files were found in both the default English, and the user's Russian language files. Here's what happens in the call to array_merge: 'merged_files' => array (size=4) 'ext/tierra/topicsolved/language/ru/info_acp_forums.php' => string 'tierra/topicsolved' (length=18) 'ext/tierra/topicsolved/language/ru/info_acp_styles.php' => string 'tierra/topicsolved' (length=18) 'ext/tierra/topicsolved/language/en/info_acp_forums.php' => string 'tierra/topicsolved' (length=18) 'ext/tierra/topicsolved/language/en/info_acp_styles.php' => string 'tierra/topicsolved' (length=18) And here's the final result after the call to array_unique: 'lang_files' => array (size=1) 'ext/tierra/topicsolved/language/ru/info_acp_forums.php' => string 'tierra/topicsolved' (length=18) Note that "info_acp_styles.php" is no longer included from any language.

          People

          • Assignee:
            Marc Marc
            Reporter:
            rxu Ruslan Uzdenov
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development