Fix mutable default arg in read_library_xml()

[misaochan]

Fix item 5 from the v1.0 release checklist (#650).

read_library_xml() used a list as a default arg for kiwix_exclude_attr. Default arg values are evaluated once at function definition time, so the same list object was shared across all calls. Each call appended "id" and "favicon" to it, causing the list to grow every call. Fixed by using None as the default and initializing a fresh list inside the function when no arg is passed.

• Confirmed bug with a test script calling the function twice and inspecting __defaults__

--- BEFORE FIX ---
kiwix_exclude_attr default before any calls: ([''],)
kiwix_exclude_attr default after call 1:     (['', 'id', 'favicon'],)
kiwix_exclude_attr default after call 2:     (['', 'id', 'favicon', 'id', 'favicon'],)

--- AFTER FIX ---
kiwix_exclude_attr default before any calls: (None,)
kiwix_exclude_attr default after call 1:     (None,)
kiwix_exclude_attr default after call 2:     (None,)

• Confirmed "Get ZIM Files from Kiwix" catalog loads correctly after fix

Tested on: Ubuntu 26.04, Python 3.14

[tim-moody]

I wasn't aware of this problem, so it could occur elsewhere in the code. Your solution looks like

https://www.geeksforgeeks.org/python/default-arguments-in-python/

However, there are a couple of problems.

First there are multiple definitions of this function across iiab-admin-console and iiab itself and they differ intentionally.

Second, the original intention of this code was to ensure that id and favicon are always excluded from the properties and that other properties can also be excluded. In your modification this will only be true if no other property is passed for exclusion.

So if property xxx is passed for exclusion then id and favicon will be included.

As near as I can tell, we never pass anything in the exclusion list to any of the 3 functions. Please verify if iiab_lib is same as cmdsrv.

Following iiab_lib:

def read_library_xml(lib_xml_file, kiwix_exclude_attr=["favicon"]): # duplicated from iiab-cmdsrv but changed [? true]

    excluded_attr = {'id'} # use a set and never include the key
    excluded_attr.update(kiwix_exclude_attr)
...

[misaochan]

Ah okay, thanks for the review. So I checked across this repo as well as iiab and iiab-factory.

So if property xxx is passed for exclusion then id and favicon will be included.

It can, but it doesn't actually change anything compared to the old code. The only place that passes a custom list is mk-zim-cat-item.py, and that call already kept favicon in both the old and new versions. I tested both versions side by side and the output is the same. And id is never affected: it's always excluded, because the fix puts it in the set directly rather than relying on the default.

As near as I can tell, we never pass anything in the exclusion list to any of the 3 functions.

Mostly correct with one exception. mk-zim-cat-item.py passes kiwix_exclude_attr=[''] to iiab_lib deliberately, presumably to override the ["favicon"] default so favicon is kept in the catalog item JSON. All other callers pass only the file path.

Please verify if iiab_lib is same as cmdsrv.

Seems similar except that iiab_lib also returns path_to_id_map which cmdsrv doesn't. Also iiab_lib doesn't seem to include your set pattern fix, same for iiab-factory/content/kiwix/upgrade-zims. So they end up with:

• iiab_lib (default ["favicon"], appends only id): ['favicon', 'id', 'id']
• iiab-factory (default [""], appends id and favicon): ['', 'id', 'favicon', 'id', 'favicon']

At any rate, I've applied your set pattern to the two admin console copies of the function (cmdsrv and upgrade-zims.py).