View Issue Details

IDProjectCategoryView StatusLast Update
0002542unrealdocumentationpublic2015-07-19 18:54
Reportertabrisnet Assigned Tosyzop  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.2.3 
Fixed in Version3.4-beta1 
Summary0002542: m_rehash() and oper-flags/U:lines
Description[Taken from a mailing list post that attracted zero responses]

Should m_rehash() differentiate btwn a /rehash vs /rehash -PARM ?

I found an oddity while coding a remote rehash in OperServ for SurrealServices.

basically, I can only do a remote rehash if I have IsNetAdmin(), and to do a /rehash -motd I must have IsAdmin() or IsCoAdmin() (IsNetAdmin() is not enough).

Even odder is that it appears that with IsULine() i should be able to do a standard rehash, but from empirical testing, it requires IsNetAdmin().

Would it not make sense for m_rehash() to always allow rehash if IsULine() ?
Is this a bug worth fixing, or do I just have to stick one of my Services Agents with umodes +AN and live with it?
TagsNo tags attached.
Attached Files
3rd party modules

Relationships

has duplicate 0002564 resolvedstskeeps No access for local ops with /rehash -motd 

Activities

aquanight

2005-05-31 19:04

reporter   ~0010016

Last edited: 2005-05-31 19:05

I think it'd be useful if U:Lines had the ability to remote rehash... allowing services to do things like mass-rehash (useful with remote-includes). Of course you could always do something like:

:OperServ MODE OperServ +AN
:OperServ REHASH server1
:OperServ REHASH server2
...etc
:OperServ MODE OperServ -AN

... but that's two commands (namely, the MODEs) that shouldn't be necessary :P .

*edit* I should point out - it is documented in /helpop ?rehash that use of a server name requires NetAdmin privileges... but... */edit*

tabrisnet

2005-06-01 02:02

reporter   ~0010024

Sure, it specifies that remote-rehash requires NetAdmin. otoh I think that IsUline() should trump IsNetAdmin.

And it's really rather stupid to have to give my services agents +A and +N, just to do this.

w00t

2005-09-29 06:49

reporter   ~0010534

Did a small patch off 3.2.3 changing a few things:
 -ULines are now allowed to rehash without being a netadmin client too
 -Merge rehash_motdrules with reread_motdsandrules, as they were virtually the same bar TLDs (besides, why have a function that only gets called in one place in the source..?)
 -Made some of the logic a little simpler/less duplicated [ala checking loop.ircd_rehashing]
 -Removed permissions check for /rehash -flags (if they are OPCanRehash, why check additional permissions for no reason?)

aquanight

2005-09-30 12:52

reporter   ~0010537

[quote] -Merge rehash_motdrules with reread_motdsandrules, as they were virtually the same bar TLDs (besides, why have a function that only gets called in one place in the source..?)[/quote]

Because having seperate functions can sometimes make life easier?

syzop

2005-09-30 21:26

administrator   ~0010541

[quote]Because having seperate functions can sometimes make life easier?[/quote]

Indeed.

Or even more often: make things more clear and easier to understand.

Code with stuff like:

if ... -motd ....
{
   rehashmotdandrules();
   return rehash(....

is just nicer than having X lines of stuff which can be in a nice subfunction :).
Sometimes it is, sometimes it isn't.. depends on the circumstances.. like: if the main function is only 5 lines, then the extra 5-10 lines wouldn't hurt.. but if it's already lengthy, then splitting up some things in subfunctions make things look clean.

Will take a look at the patch in more detail later, but yeah, I get the idea.. the function is kinda ugly.

w00t

2005-10-01 06:42

reporter   ~0010543

Well, perhaps they should still be merged, but accepting a parameter then or something? Given that function was only called in m_rehash anyway, there was really no need for it </imo>.

That said syzop, if you have any other suggestions on how you'd want it done, feel free to let me know and I'll go over it again. (yeah, it's uh, not the prettiest)

syzop

2005-10-01 11:13

administrator   ~0010548

[quote]Well, perhaps they should still be merged, but accepting a parameter then or something? Given that function was only called in m_rehash anyway, there was really no need for it </imo>.[/quote]
Did you actually read my point??
You make me quote myself again:
[quote]is just nicer than having X lines of stuff which can be in a nice subfunction :).
Sometimes it is, sometimes it isn't.. depends on the circumstances.. like: if the main function is only 5 lines, then the extra 5-10 lines wouldn't hurt.. but if it's already lengthy, then splitting up some things in subfunctions make things look clean.[/quote]
So it has nothing to do with only being called from 1 location.

[quote]That said syzop, if you have any other suggestions on how you'd want it done, feel free to let me know and I'll go over it again. (yeah, it's uh, not the prettiest)[/quote]
I think you kept the function intact (just merged it with another one, which fixes confusion bug #<whatever> which said that /rehash -motd does not rehash opermotd/botmotd), so that's good. I didn't look at it in-detail though, but probably most of it is ok :P.

w00t

2005-10-02 09:55

reporter   ~0010550

There were two 'sub' /rehash functions, one that did all MOTDs, bar TLD blocks - the other that did TLD blocks too. I merged them together, as I really see no point seperating them (particularly given how infrequent rehashes really are..)

I (think) I solved the -motd's stuff, yeah..
Also ulines not being able to rehash (this whole issue) and everyone NOT a server/coadmin not being able to do /rehash -motd or another -flag.

I think I get your point now? Basically, it's only good split out if it's a larger function, or frequently used.. my point was more I saw no point splitting MOTD/TLD rehashing into two essentially identical functions, my reply was that 'perhaps' the was something I'd missed, and a suggestion to re-merge them with a parameter indicating whether the TLD blocks should be reloaded in addition to the regular MOTD files (oper/bot/motd/short).

Sorry if I confused ;)

tabrisnet

2006-02-03 20:49

reporter   ~0011125

I assume that nothing on this has been done with 3.2.4 ? Or should I just test it when I have the time?

RandomNumber

2006-02-05 00:04

reporter   ~0011137

Nothing was stated about this topic in the release notes.

syzop

2015-07-19 18:54

administrator   ~0018529

All the permission checks have been moved to just one single check in 3.4-beta1.
Should be fine now. Function looks better than before too I think, only glanced at it.

Issue History

Date Modified Username Field Change
2005-05-30 18:01 tabrisnet New Issue
2005-05-31 19:04 aquanight Note Added: 0010016
2005-05-31 19:05 aquanight Note Edited: 0010016
2005-06-01 02:02 tabrisnet Note Added: 0010024
2005-08-19 14:49 syzop Status new => acknowledged
2005-09-29 06:45 w00t File Added: rehash_logic_and_perms-patch
2005-09-29 06:49 w00t Note Added: 0010534
2005-09-30 12:52 aquanight Note Added: 0010537
2005-09-30 21:26 syzop Note Added: 0010541
2005-10-01 06:42 w00t Note Added: 0010543
2005-10-01 11:13 syzop Note Added: 0010548
2005-10-02 09:55 w00t Note Added: 0010550
2006-02-03 20:49 tabrisnet Note Added: 0011125
2006-02-05 00:04 RandomNumber Note Added: 0011137
2007-04-27 05:16 stskeeps Relationship added has duplicate 0002564
2015-07-19 18:54 syzop Note Added: 0018529
2015-07-19 18:54 syzop Status acknowledged => resolved
2015-07-19 18:54 syzop Fixed in Version => 3.4-beta1
2015-07-19 18:54 syzop Resolution open => fixed
2015-07-19 18:54 syzop Assigned To => syzop