#23 Load parameter for log module's path (with patch)

closed
nobody
None
5
2011-02-25
2010-06-12
MentalFS
No

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.

Discussion

  • Psychon

    Psychon - 2010-06-13
    • labels: 658400 -->
     
  • Psychon

    Psychon - 2010-06-13

    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()....

     
  • MentalFS

    MentalFS - 2010-06-13

    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.

     
  • Psychon

    Psychon - 2010-06-25

    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.

     
  • MentalFS

    MentalFS - 2010-06-25

    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.

     
  • MentalFS

    MentalFS - 2010-07-12

    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.

     
  • MentalFS

    MentalFS - 2010-09-21

    Patch for log.cpp

     
  • MentalFS

    MentalFS - 2010-09-21

    log.cpp

     
  • MentalFS

    MentalFS - 2010-09-21

    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.

     
  • Psychon

    Psychon - 2010-12-11

    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.

     
  • Psychon

    Psychon - 2011-02-25
    • status: open --> closed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks