View Issue Details

IDProjectCategoryView StatusLast Update
0002697unrealircdpublic2019-07-08 15:38
ReporterRobby22Assigned To 
PrioritynormalSeverityfeatureReproducibilityalways
Status acknowledgedResolutionopen 
Platformi586OSRed Hat LinuxOS Version7.3
Product Version3.2.4 
Target VersionFixed in Version 
Summary0002697: Unable to completely remove the topic
DescriptionWhen you first join a channel and you do: /TOPIC #Test
you get: #Test No topic is set.

When unsetting the topic, the ircd still says there is a topic set, even though there is nothing in it. It should normally say "No topic is set." again but it doesn't.
Steps To ReproduceSet a topic and remove it again, use /TOPIC #channel and you will see it still says that one is set eventhough it's empty.
Additional Information-> Server: TOPIC #Test
* Topic is 'test1 test2 test3'
* Set by Robby on Wed Nov 16 06:46:06
-> Server: TOPIC #Test :
* Robby changes topic to ''
-> Server: TOPIC #Test
* Topic is ''
* Set by Robby on Wed Nov 16 06:46:48
TagsNo tags attached.
3rd party modules

Relationships

child of 0005328 assignedargvx TODO list for 'i' 

Activities

w00t

2005-11-16 01:17

reporter   ~0010750

*theoretical* fix, I've not thought about this too hard..

        else if (ttime && topic && (IsServer(sptr)
            || IsULine(sptr)))
        {
            if (!chptr->topic_time || ttime > chptr->topic_time || IsULine(sptr))
            /* The IsUline is to allow services to use an old TS. Apparently
             * some services do this in their topic enforcement -- codemastr
             */
            {
                /* setting a topic */
                topiClen = strlen(topic);
                nicKlen = strlen(tnick);

                if (chptr->topic)
                    MyFree(chptr->topic);

around there, check for topiClen of 0 after MyFree() and return, as such:

        else if (ttime && topic && (IsServer(sptr)
            || IsULine(sptr)))
        {
            if (!chptr->topic_time || ttime > chptr->topic_time || IsULine(sptr))
            /* The IsUline is to allow services to use an old TS. Apparently
             * some services do this in their topic enforcement -- codemastr
             */
            {
                /* setting a topic */
                topiClen = strlen(topic);
                nicKlen = strlen(tnick);

                if (chptr->topic)
                    MyFree(chptr->topic);
                if (topiClen == 0)
                    return 0;

(though you might want to move:
                if (chptr->topic_nick)
                    MyFree(chptr->topic_nick);
up so as you remove that too, but looking at the topic send code, it's not necessary.)

syzop

2005-11-16 16:42

administrator   ~0010752

This has been discussed before.
In the end, we stuck with our current implemention, because you can then see who actually removed (unset) the topic (and when).

w00t

2005-11-16 22:52

reporter   ~0010758

That makes sense, but perhaps make an exception for U:lines in that? I've had instances of ChanServ restoring an empty topic, which just looks irritating.. your call.

Robby22

2005-11-17 00:27

reporter   ~0010760

Last edited: 2005-11-17 00:39

True, ChanServ also restores empty topics. (But I think you can't really avoid that as that is related to the services you are using... (You could maybe make the ircd ignore empty topics from U:lined services/ChanServ, but I don't believe that's good, I feel it's the services coders itself that must make services not reset a topic when it's empty in the first place.))

But back on topic:
Maybe it could be made an option or something, so that the behaviour is configurable between UnrealIRCd's implementation and the classic one (and default it to UnrealIRCd's implementation orso).

Like for example the same was done with TOPIC_NICK_IS_NUHOST... which is nice, because I always enable it.

syzop

2005-11-17 10:04

administrator   ~0010763

Probably a good idea @ ulines. Though I must admit I don't think I have seen that actually ;p.

Oh ehm w00t... Whenever you free (or MyFree) something, you have to set the pointer to NULL (chptr->topic = NULL;). Otherwise it points at just-freed-memory, which will cause a "read-after-free" which is very dangerous (read: crashy ;p).
(In the current code we don't set it to NULL of course, because we already assign a new value a few lines later to it)

syzop

2005-11-17 10:05

administrator   ~0010764

Robby22: I think we should consider that indeed.

w00t

2005-11-18 03:17

reporter   ~0010770

syzop: I realised that when I went home, changed my copy to test it, and got some really fun wild pointer fun :P.

Will keep it in mind from now on ;).

Issue History

Date Modified Username Field Change
2005-11-16 00:52 Robby22 New Issue
2005-11-16 01:17 w00t Note Added: 0010750
2005-11-16 16:42 syzop Note Added: 0010752
2005-11-16 22:52 w00t Note Added: 0010758
2005-11-17 00:27 Robby22 Note Added: 0010760
2005-11-17 00:39 Robby22 Note Edited: 0010760
2005-11-17 10:04 syzop Note Added: 0010763
2005-11-17 10:05 syzop Note Added: 0010764
2005-11-18 03:17 w00t Note Added: 0010770
2007-04-27 04:29 stskeeps Status new => acknowledged
2015-08-08 18:27 syzop Severity minor => feature
2019-07-08 08:46 syzop Relationship added child of 0005328