View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001317 | unreal | ircd | public | 2003-10-24 13:59 | 2019-01-27 14:47 |
| Reporter | thilo | Assigned To | syzop | ||
| Priority | normal | Severity | feature | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Fixed in Version | 3.2.7-RC2 | ||||
| Summary | 0001317: Inclusion of topic_setter in topic message when set by services really necessary? | ||||
| Description | Whenever services enforce an old topic, be it the locking of the topic, be it a services restart, the topic_setter is included in braces at the end of the topic message, although the daemon stores the topic without the nick internally. This does cause a client/server desync. Furthermore, if TOPIC_NICK_IS_NUHOST was enabled in the config.h, the host of a user is also included in the topic message .. it looks particularly nasty on long hosts. I have coded a small "fix" in channel.c in the function m_topic that resends the "topic has been set by ..." numeric to all users on the channel once a topic has been reset. FYI: The patch below patches against the channel.c in the package 3.2beta-18 | ||||
| Additional Information | --- channel.c 2003-10-12 01:17:06.000000000 +0200 +++ channel.c.orig 2003-10-12 01:21:18.000000000 +0200 @@ -4149,9 +4149,6 @@ * some services do this in their topic enforcement -- codemastr */ { - /* we need this to search through the chained list and send the topic setter message to each client. */ - Member *lp; - /* setting a topic */ topiClen = strlen(topic); nicKlen = strlen(tnick); @@ -4182,16 +4179,9 @@ chptr->chname, chptr->topic_nick, chptr->topic_time, chptr->topic); sendto_channel_butserv(chptr, sptr, - ":%s TOPIC %s :%s", parv[0], - chptr->chname, chptr->topic); - - for(lp = chptr->members; lp; lp = lp->next) - { - sendto_one(lp->cptr, - rpl_str(RPL_TOPICWHOTIME), me.name, - lp->cptr->name, chptr->chname, - chptr->topic_nick, chptr->topic_time); - } + ":%s TOPIC %s :%s (%s)", parv[0], + chptr->chname, chptr->topic, + chptr->topic_nick); } } else if (((chptr->mode.mode & MODE_TOPICLIMIT) == 0 || | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | |||||
|
|
it gets embarassing if i do the diff the wrong way round .. correct patch: --- channel.c.orig 2003-10-12 01:21:18.000000000 +0200 +++ channel.c 2003-10-12 01:17:06.000000000 +0200 @@ -4149,6 +4149,9 @@ * some services do this in their topic enforcement -- codemastr */ { + /* we need this to search through the chained list and send the topic setter message to each client. */ + Member *lp; + /* setting a topic */ topiClen = strlen(topic); nicKlen = strlen(tnick); @@ -4179,9 +4182,16 @@ chptr->chname, chptr->topic_nick, chptr->topic_time, chptr->topic); sendto_channel_butserv(chptr, sptr, - ":%s TOPIC %s :%s (%s)", parv[0], - chptr->chname, chptr->topic, - chptr->topic_nick); + ":%s TOPIC %s :%s", parv[0], + chptr->chname, chptr->topic); + + for(lp = chptr->members; lp; lp = lp->next) + { + sendto_one(lp->cptr, + rpl_str(RPL_TOPICWHOTIME), me.name, + lp->cptr->name, chptr->chname, + chptr->topic_nick, chptr->topic_time); + } } } else if (((chptr->mode.mode & MODE_TOPICLIMIT) == 0 || |
|
|
I agree that the nick shouldn't be set in brackets after the end eg. *** service changes topic to 'weee (praetorian)' but i also disagree that the setby line should be sent when it does happen. meaning *** Set by praetorian at 11:11:11 1/1/1111 In my view i would think just set the topic. if anyone cares who set it, they can /topic #chan and get the setby line manually.. my 2 cents ;) |
|
|
Praetorian: You still have a server/client desync, and that is to be avoided in my humble opinion, on the other hand, the topic_setter version seems to work quite well here, and it does only happen when the services enforce a topic lock, which is probably not too often. Regarding the patch: I have also prepared a link where to get the patch, as I noticed now, that the web version here does not preserve spaces and tabs. You can download the patch from http://home.bawue.de/~arny/unreal_topicsetter.patch |
|
|
Well, suddenly sending something at an unexpected stage is usually considered wrong and might lead to problems (that's a general thing, not specific for this although this is unlikely to be accepted)... I agree that this current topic thing isn't that nice and that it causes this "topic desynch", so I can understand that you are searching for a solution to it. I personally don't have any problem with it because I know how it works but I can understand that inexperienced (or even pretty experienced) users will be confused by this. Let's see what other coders have to say about the general issue :p. |
|
|
Yes. maybe it is not very nice to send a numeric reply, when no query was sent. (although, when joining a channel, you get a bunch of numerics for which there were no specific questions) If it is not feasable, you should still consider completely removing the topic_setter in the topic message completely. |
|
|
How about instead just put the setter in the 'source' prefix when you send the topic? No numeric, no nick in brackets... but the client gets the right info. They don't need to know who really set the topic (ChanServ/BotServ-bot), do they? Instead of: * ChanServ sets topic to 'Blahblahblah (aquanight)' It will show as: * aquanight sets topic to 'Blahblahblah' Clients should handle it fine whether 'aquanight' is actually in the channel or opped with +t or not. |
|
|
Bad suggestion aquanight ... |
|
|
How do you know clients will handle that fine? I think it would screw them up rather badly! First off, it could screw Unreal up. Unreal has an option to show nick!user@host. Well if the user isn't online, where does the user@host part come from? Services doesn't store that, and neither does Unreal. Second, clients could very well become confused from seeing a topic change from someone who is not in the channel. I don't understand how you could assume they wouldn't. It's illegal, so how can we expect clients to accept and understand something that is illegal? |
|
|
Wouldn't the user@host come from the TOPIC message that is coming from the remote server? And yes, it most likely would be absent for services or offline users, in which case, there's no real need to pass it on to clients. (No sense trying to tell them something you don't know ;) .) And if it is a problem, then you could just pull it from the whowas data (since you already store the user@host there, one of them has to be correct ;) ). Also, how many clients have you seen that actually display the user@host in the "blablah sets topic to" message (though mIRC, for example, could be scripted to do so)? An oper using operoverride to op himself or banwalk is "illegal" and so is ChanServ opping people when it's not even in the channel, but have you seen a client have problems with that? And then there's +u... Have you ever been in a +u-t channel where one of the normals set the topic? Did the client have problems then? Personally, I don't think it's right for a client to be "validating" what's coming from the server. If the server says it's right, then by golly, it's right. If the client has problems with that, then it should just /quit... because clients have no business telling the server something isn't right ;) . Also, as it is now, a topic enforced by services still comes from an "invalid" user (ChanServ). This kind of topic change isn't illegal? |
|
|
Well I don't like this 'spoofing' either, it's not a good idea and very confusing. Anyway, back to the point... why do we do include the topic setter? If you do '/topic #chan' you can see who really set the last topic (since that info is updated correctly). So we are only talking about letting the user know who set it without requiring any effort (by appending the name after the topic etc)... Either way ("don't show who set it when updating the topic" or "add the nick in brackets after the topic") you get a little 'desynch', in the 2nd case even a topic-text-desynch which is IMO a lot more annoying. I think if someone really wonders who set the topic they could just do /topic #chan again, in most cases people won't care. Just my thoughts... [hm basically that's just what Praetorian_ said ;p] |
|
|
Yes, the reason I think, why this was reported is: *** ChanServ changes topic to 'blah (Rocko)' and I want to change the topic, mIRC shows the topic 'blah (Rocko)' in the editbox, instead of 'blah', so when I forget to make a /topic #channel prior to editing the topic, the (Rocko) will be added into the topic. When I make a /topic #channel first, the (Rocko) isn't there anymore. Thats the originally problem. And ChanServ sets a topic only in the case of a topic_lock (enforce), or if a new channel is created (topic Retention). So, thats right, when someone really cares about, who set the topic, he should make a /topic #channel. bearbeitet am: 2004-06-04 14:46 |
|
|
"How do you know clients will handle that fine? I think it would screw them up rather badly!" aquanight raised a valid point, that then something like ChanServ topic enforcement would screw up the client on those networks, that do not join ChanServ when a channel has been registered. Also, at the moment IRCops can change topics even when they are not on the channel. Anyways, I don't like his way of topic change either, the topic setter time is not being preserved :P "Unreal has an option to show nick!user@host. Well if the user isn't online, where does the user@host part come from? Services doesn't store that, and neither does Unreal." The whole part is stored in the topic_setter space. This is independent from whether the user is online or not ... in fact I have built in support for the TOPIC_NICK_IS_NUHOST option in my services. Like I said, it looks really nasty when the whole host is also being shown in the topic. "Either way ("don't show who set it when updating the topic" or "add the nick in brackets after the topic") you get a little 'desynch'" My solution suggested above has worked on my server without problems. Whenever a u:lined server enforces a topic with a foreign topic_setter, the server can send out the topic_setter numeric. This way, you ensure that there is no desynch. I don't know whether this violates any rules, even if the client hasn't explicitly requested for a topic. But then again, I can't find the topic_setter numeric in the RFCs anyways, and the numeric is also being sent to the client automatically when it joins a channel. Thinking about client architectures, I would actually be too lazy to build something in that would check whether really some query was sent that would justify receiving a numeric ... actually, I can't see why anyone would want to check this. My patch above basically works, you still need to check though, whether the cptr is a client on the current server before sending the numeric to that client, then it's perfect. |
|
|
'My solution suggested above has worked on my server without problems.' Yes, but that solution was rejected already. Just because you tested it and it seems to works fine does not mean it's good, there a lot of such patches that are just horrible ;P. |
|
|
as far as i can see this, noone rejected anything yet, not even you. My point is: per definition in RFC the client also does not expect a topic_setter message when it joins the channel, because there numeric 333 simply does not exist. If you were strict, you would have to at least limit the topic_setter numeric reply to when someone issues a topic command without a parameter, not on Join. edited on: 2004-06-04 16:37 |
|
|
'as far as i can see this, noone rejected anything yet, not even you.' 8 months ago already, although no.. not in strong terms ['unlikely to be accepted'].. sometimes I'm nice :P. The keyword is 'unexpected' here.. no client expects to suddenly receive a topicsetby message after the user is idling for 2 hours in a channel. It's ugly, bad practice / not good, and might even break things. It's a bit like the guys shouting a NAMES list should be sent when you do -u or +u... God.. ;p |
|
|
Yeah, whatever, pls pls just get rid of the topic setter in the topic text ;-D |
|
|
Mh, I found out, bahamut doesn't send that "topic_setter" too. (My nice examples with bahamut ;P) There is only this: *** ChanServ changes topic to 'test' Bahamut has this (little) "desynch", that clients could think, ChanServ sets the topic too, but mIRC doesn't show this anyway. After a /topic #channel, everythings ok, and you see the topic_setter. |
|
|
Well, personally, I could care less who it's coming from, only what it says (and if I do need to change it, I usually will notice the nick at the end, if it doesn't get removed ;) ). |
|
|
Bump. Still a issue? |
|
|
Yes it is. Actually I solve this "problem" by editing the source manually, because you aren't changing it in the main source code. It's only a change in src/modules/m_topic.c. Change :%s TOPIC %s :%s (%s)", parv[0], to :%s TOPIC %s :%s", parv[0], For people who don't want this topic setter. Because it is really anoying, if chanserv enforces a topic, and you see "Muuh..Topic.. (chanserv). Than I want to change the topic from my script and it gives me "Muuh..Topic.. (chanserv)" as the actual topic, which it isn't, bahamut doesn't set it too (topic setter), so why should unreal? In advance, I also modified the IRCd, so that when I delete the topic, it's really deleted, and not like "'". We/I also mentioned that, that this should be changed too, but I think there was an other bug report !? |
|
|
just to resay my comments: I don't like suddenly sending a numeric (this is unlikely to happen, sts will agree with me on that), but I do dislike the 'xx (setby)' thingy, so I agree it should be changed (dropped).. or otherwise be configurable at least. |
|
|
I dont like it "Topic (mynickhere)". I like the idea of removal. |
|
|
+1 to vonitsanet |
|
|
Removed in .2426 |
|
|
re-port this patch, don't go send numerics ;) |
|
|
Doing some archeology here. This seems to be fixed already. I am unable to reproduce this on irc.unrealircd.org (runs on Unreal3.2.10.2 as I speak). |
|
|
DBoyz is correct, this was fixed in f3a2d7f6a45c007a68397b620163febbf0257366 in UnrealIRCd 3.2.7, about 11,5 years ago. commit f3a2d7f6a45c007a68397b620163febbf0257366 Author: stskeeps <devnull@localhost> Date: Fri Jun 15 19:47:26 2007 +0000 - 0001317 reported by thilo regarding removal of (username) being appended to topics set by U:Lined servers. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2003-10-24 13:59 | thilo | New Issue | |
| 2003-10-24 14:01 | thilo | Note Added: 0003851 | |
| 2003-10-24 14:09 | Praetorian_ | Note Added: 0003852 | |
| 2003-10-24 14:24 | thilo | Note Added: 0003856 | |
| 2003-10-24 15:12 | syzop | Note Added: 0003859 | |
| 2003-10-24 17:34 | thilo | Note Added: 0003860 | |
| 2004-06-04 02:06 | aquanight | Note Added: 0006573 | |
| 2004-06-04 07:41 | Rocko | Note Added: 0006575 | |
| 2004-06-04 13:15 |
|
Note Added: 0006582 | |
| 2004-06-04 13:31 | aquanight | Note Added: 0006583 | |
| 2004-06-04 14:01 | syzop | Note Added: 0006588 | |
| 2004-06-04 14:38 | Rocko | Note Added: 0006591 | |
| 2004-06-04 14:42 | Rocko | Note Edited: 0006591 | |
| 2004-06-04 14:46 | Rocko | Note Edited: 0006591 | |
| 2004-06-04 15:04 | thilo | Note Added: 0006593 | |
| 2004-06-04 15:16 | syzop | Note Added: 0006596 | |
| 2004-06-04 16:19 | thilo | Note Added: 0006599 | |
| 2004-06-04 16:36 | thilo | Note Edited: 0006599 | |
| 2004-06-04 16:37 | thilo | Note Edited: 0006599 | |
| 2004-06-04 17:07 | syzop | Note Added: 0006600 | |
| 2004-06-04 17:26 | thilo | Note Added: 0006601 | |
| 2004-06-04 17:59 | Rocko | Note Added: 0006602 | |
| 2004-06-05 22:23 | aquanight | Note Added: 0006603 | |
| 2007-04-27 06:10 |
|
Note Added: 0013859 | |
| 2007-04-27 06:10 |
|
Status | new => feedback |
| 2007-04-27 10:45 | Rocko | Note Added: 0013881 | |
| 2007-04-27 10:56 | syzop | Note Added: 0013882 | |
| 2007-04-27 11:31 | vonitsanet | Note Added: 0013883 | |
| 2007-04-27 11:33 | Bock | Note Added: 0013884 | |
| 2007-06-09 18:38 | WolfSage | Note Added: 0014292 | |
| 2007-06-09 18:38 | WolfSage | Assigned To | => WolfSage |
| 2007-06-09 18:38 | WolfSage | Status | feedback => resolved |
| 2007-06-09 18:38 | WolfSage | Resolution | open => fixed |
| 2007-06-09 18:38 | WolfSage | Fixed in Version | => 3.3-alpha0 |
| 2007-06-09 18:38 | WolfSage | Description Updated | |
| 2007-06-09 18:38 | WolfSage | Additional Information Updated | |
| 2007-06-09 20:47 | WolfSage | Relationship added | related to 0003385 |
| 2011-07-19 17:54 | syzop | Note Added: 0016712 | |
| 2011-07-19 17:54 | syzop | Assigned To | WolfSage => |
| 2011-07-19 17:54 | syzop | Status | resolved => needs re porting |
| 2015-08-08 17:54 | syzop | Severity | tweak => feature |
| 2015-10-02 06:33 | dboyz | Note Added: 0018730 | |
| 2019-01-27 14:47 | syzop | Assigned To | => syzop |
| 2019-01-27 14:47 | syzop | Status | needs re porting => resolved |
| 2019-01-27 14:47 | syzop | Fixed in Version | 3.3-alpha0 => 3.2.7-RC2 |
| 2019-01-27 14:47 | syzop | Note Added: 0020464 |
