View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002542 | unreal | documentation | public | 2005-05-30 18:01 | 2015-07-19 18:54 |
| Reporter | tabrisnet | Assigned To | syzop | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.2.3 | ||||
| Fixed in Version | 3.4-beta1 | ||||
| Summary | 0002542: 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? | ||||
| Tags | No tags attached. | ||||
| Attached Files | |||||
| 3rd party modules | |||||
| has duplicate | 0002564 | resolved | No access for local ops with /rehash -motd |
|
|
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* |
|
|
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. |
|
|
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?) |
|
|
[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? |
|
|
[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. |
|
|
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) |
|
|
[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. |
|
|
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 ;) |
|
|
I assume that nothing on this has been done with 3.2.4 ? Or should I just test it when I have the time? |
|
|
Nothing was stated about this topic in the release notes. |
|
|
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. |
| 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 |
|
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 |
