sympa-community / sympa

Sympa, Mailing List Management Software
https://www.sympa.community/sympa
GNU General Public License v2.0
237 stars 94 forks source link

Cross-robot include_sympa_list tries to include the wrong list, possibly failing with an imagined "inclusion loop" #1797

Open dpoon opened 4 months ago

dpoon commented 4 months ago

If a mailing list code><i>target-list</i>@<itarget-robot.example.edu is configured with

include_sympa_list
name blah
listname source-list@source-robot.example.edu

… then when Sympa performs the sync, it tries to include code><i>source-list</i>@<itarget-robot.example.edu instead.

Version

6.2.66, but I believe the problematic code is the same in 6.2.72 and main.

Installation method

Universe repo for Ubuntu 22.04: https://mirror.it.ubc.ca/ubuntu/pool/universe/s/sympa/sympa_6.2.66~dfsg-2_amd64.deb

Steps to reproduce

Configure mailing list data sources as described in the summary above (Admin → Edit List Config → Data Sources Setup → List inclusion). Trigger a sync (sympa.pl --sync_include=i>target-list</i>@<itarget-robot.example.edu).

Actual behavior

Sympa tries to load i>source-list</i>@<itarget-robot.example.edu instead of i>source-list</i>@<isource-robot.example.edu.

Furthermore, if source-list and target-list share the same listname in different robots, then Sympa ends up trying to include i>target-list</i>@<itarget-robot.example.edu, causing the inclusion request to fail with

err main::#851 > Sympa::Spindle::spin#95 > Sympa::Request::Handler::include::_twist#202 > Sympa::Request::Handler::include::_update_users#370 > Sympa::DataSource::open#139 > Sympa::DataSource::List::_open#71 Loop detection in list inclusion: could not include again Sympa::DataSource::List <context=listname@target-robot.example.edu;id=4e750bd4;role=member;name=include_list listname@source-robot.example.edu> in list Sympa::List <listname@target-robot.example.edu>

… even though no such loop exists.

Expected behavior

This used to work: Sympa would load i>source-list</i>@<isource-robot.example.edu.

Additional information

I believe, from reading the code, that it broke in Sympa 6.2.45b.1. Commit https://github.com/sympa-community/sympa/commit/c1b7e324d5d8d3443a68658fd20c33902187218e, part of pull request https://github.com/sympa-community/sympa/pull/516, is suspect.

Compare this code in 6.2.44, in which the source list's robot is inferred to be the target list's robot only if the source list's name does not specify a robot: https://github.com/sympa-community/sympa/blob/313a004ef456e6a0b6455b46d630fff5102c2cff/src/lib/Sympa/List.pm#L7095-L7096

… with this code in 6.2.45b1, in which the source list's robot is forcibly set to be the inclusion context's domain (i.e. the target list's robot):

https://github.com/sympa-community/sympa/blob/c1b7e324d5d8d3443a68658fd20c33902187218e/src/lib/Sympa/DataSource/List.pm#L46-L51

Keep in mind that https://github.com/sympa-community/sympa/blob/f74d34cc76aba0ede74ff4b6ffea51bdfea8f86f/src/List.pm#L980 makes it so that specifying the optional $robot parameter when calling the Sympa::List->new($name, $robot) constructor causes the $robot parameter override any code>@<idomain that may appear in the $name.