[ZNC-Users] [ znc-Patches-3015289 ] Load parameter for log module's path (with patch)

[SourceForge.net <no...@so...>]

Patches item #3015289, was opened at 2010-06-12 23:22
Message generated for change (Comment added) made by psychon
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=672823&aid=3015289&group_id=115828

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: MentalFS (mentalfs)
Assigned to: Nobody/Anonymous (nobody)
Summary: Load parameter for log module's path (with patch)

Initial Comment:
A load parameter that lets you set the path for the log files (preferably including file name).

I'm adding this patch I've been using for a while now and finally took the time to make it usable for everyone. It uses the strftime variables to expand time information. I added %s for the window name and %u for the user. I no parameter is given, it behaves like the current version.

----------------------------------------------------------------------

>Comment By: Psychon (psychon)
Date: 2010-12-11 17:04

Message:
Totally forgot this and only now remembered to look into this again,
sorry.

CLogMod::ContainsDateInfo(): Why aren't I allowed to just use "%D"? What I
want to say is that you can't check for all the possible characters and so
instead should IMHO not check at all. If all the logging goes to one big
file, then that's how it is. Perhaps someone really meant to do that.
Also, there is no reason for this function to be virtual. Mark it static
instead ;)

You change more code in PutLog() than necessary. Just leave the old
handling of writing to the file in there, you just have to change the stuff
before the CFile LogFile is created IMHO.
With that change, there isn't much use of RETURN_ERROR any more, does this
really need a macro? :/

Then comes documentation. This should explain somewhere (perhaps /msg *log
help) how it can be told to log elsewhere and perhaps also where it's
logging to right now.

Remind me to fix this up myself and commit when I'm bored ;)

Thanks for the patch, I think this is definitely sth we'd want in.

----------------------------------------------------------------------

Comment By: MentalFS (mentalfs)
Date: 2010-09-21 23:17

Message:
It's me again. I wasn't satisfied with the possibility of logging
everything into one big file - especially because that could happen without
purpose. So in this version, there are checks to make sure there is one
file per day and channel.

Also, there are path checks with CheckPathPrefix.

----------------------------------------------------------------------

Comment By: MentalFS (mentalfs)
Date: 2010-07-12 23:43

Message:
Another update. The last version was essentially as unsecure as the first
ones.

I'm now using CheckPathPrefix. It's used in OnLoad, but also in PutLog, so
that noone can use funky strftime formatting to circumvent it.

----------------------------------------------------------------------

Comment By: MentalFS (mentalfs)
Date: 2010-06-26 00:10

Message:
I always keep forgetting that.

Well, I think the only secure thing would be ignoring a leading / and
always prepending GetSavePath(). The rest could be done with symlinks like
already described in the wiki.

Adding a new patch that does this.

----------------------------------------------------------------------

Comment By: Psychon (psychon)
Date: 2010-06-25 23:27

Message:
cnu mentioned that he'd love to log to ~/.ssh/authorized_keys. He has a
valid point there, we can't allow arbitrary paths. That's a security issue.

----------------------------------------------------------------------

Comment By: MentalFS (mentalfs)
Date: 2010-06-13 20:49

Message:
Okay, i've added the checks for strftime.

Also, I changed %s and %u to $WINDOW and $USER - it's not only better to
read, using % would have caused potential problems with strftime.

I'm trying to catch every possibility in OnloadMod. This includes someone
setting a path that doesn't contain $WINDOW or date information. If that
happens, it will be written into the log file as part of the timestamp.

----------------------------------------------------------------------

Comment By: Psychon (psychon)
Date: 2010-06-13 10:08

Message:
It might be a good idea to check strftime()'s return value.

Besides that, patch seems like a good idea, although I don't really get
some of the parts from OnLoad()....

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=672823&aid=3015289&group_id=115828