View Issue Details

IDProjectCategoryView StatusLast Update
0005162unrealmodule apipublic2018-12-08 15:30
ReporterGottemAssigned Tosyzop 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version4.2.0 
Target VersionFixed in Version4.2.1 
Summary0005162: Cmdoverride priorities might be funky
DescriptionI figured it was finally time to update some modules to work with command override priorities, but it seems they're not properly adhered to. I have 2 modules overriding INVITE and one of them is always processed first for some reason.

I modified the override functions to print just their name and return right away, to make sure it's not something else down the function screwing with it. The one that's always called first (m_fixhop) also includes a loop to print the linked list of command overrides.

// Module 1 (m_fixhop)
inviteovr = CmdoverrideAddEx(modinfo->handle, "INVITE", -10, fixhop_inviteoverride); // In MOD_LOAD
static int fixhop_inviteoverride(Cmdoverride *ovr, aClient *cptr, aClient *sptr, int parc, char *parv[]) {
    sendto_realops("fixhop_inviteoverride");
    aCommand *debug_cmd;
    Cmdoverride *debug_ovr;
    if((debug_cmd = find_Command_simple("INVITE"))) {
        for(debug_ovr = debug_cmd->overriders; debug_ovr; debug_ovr = debug_ovr->next)
            sendto_realops("Override: %s => %d", debug_ovr->owner->header->name, debug_ovr->priority);
    }
    return CallCmdoverride(ovr, cptr, sptr, parc, parv);
}

// Module 2 (m_repeatprot)
inviteovr = CmdoverrideAddEx(modinfo->handle, "INVITE", -20, repeatprot_override);
static int repeatprot_override(Cmdoverride *ovr, aClient *cptr, aClient *sptr, int parc, char *parv[]) {
    sendto_realops("repeatprot_override");
    return CallCmdoverride(ovr, cptr, sptr, parc, parv);
}

Console output:
*** fixhop_inviteoverride
*** Override: m_repeatprot => -20
*** Override: m_fixhop => -10
*** repeatprot_override

But I expected:
*** repeatprot_override
*** fixhop_inviteoverride
*** Override: m_repeatprot => -20
*** Override: m_fixhop => -10

As you can see the for loop prints the list in proper order, yet fixhop still runs first. I'm assuming the priorities work the same as for hooks, i.e. a lower number has more priority and you can go into negative. I also checked these values and the same happens for them:
* fixhop -20 and repeatprot -10 (so reversed from above code)
* fixhop 9999999 and repeatprot 10
* fixhop 0 and repeatprot 0 (which is exceptionally odd since this is the default priority and is pretty much how it worked in <= v4.0.18

In my unrealircd.conf fixhop is loaded after repeatprot, but if I switch them then the fixhop override doesn't even run at all. O.o Maybe I'm just having a major brainfart but something doesn't seem right (anymore) when multiple modules override the same command. :D I actually do need repeatprot to run first because there's a little conflict in very specific cases if it doesn't (which actually happens a lot on my network and is why I requested priorities in the first place =]).
TagsNo tags attached.
3rd party modules

Activities

syzop

2018-12-08 15:21

administrator   ~0020393

Right, so it has the order reversed? I see this in CallCmdoverride:

int CallCmdoverride(Cmdoverride *ovr, aClient *cptr, aClient *sptr, int parc, char *parv[])
{
        if (ovr->prev)
                return ovr->prev->func(ovr->prev, cptr, sptr, parc, parv);
        return ovr->command->func(cptr, sptr, parc, parv);
}

So it's doing ->prev each time, instead of ->next. Not sure why.

syzop

2018-12-08 15:29

administrator   ~0020394

Fixed, thanks for the report Gottem :)

commit 9cfff2d07dd0c6717527236ff9391affeb76a004 (HEAD -> unreal42, origin/unreal42)
Author: Bram Matthys <syzop@vulnscan.org>
Date: Sat Dec 8 15:23:42 2018 +0100

    In 4.2.0 we added support for priorities in CmdoverrideAddEx(),
    however it turns out they were accidentally reversed.
    This is now corrected: highest number = highest prioty.
    Reported by Gottem in https://bugs.unrealircd.org/view.php?id=5162

syzop

2018-12-08 15:30

administrator   ~0020395

By the way I didn't double check or test this, but I'm assuming this was the fix. I know.. bad practice :D

Issue History

Date Modified Username Field Change
2018-11-05 00:48 Gottem New Issue
2018-12-08 15:21 syzop Note Added: 0020393
2018-12-08 15:29 syzop Assigned To => syzop
2018-12-08 15:29 syzop Status new => resolved
2018-12-08 15:29 syzop Resolution open => fixed
2018-12-08 15:29 syzop Fixed in Version => 4.2.1
2018-12-08 15:29 syzop Note Added: 0020394
2018-12-08 15:30 syzop Note Added: 0020395