Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cannot create logfile in a subdirectory #4027

Open
axd1967 opened this issue Dec 26, 2024 · 8 comments
Open

cannot create logfile in a subdirectory #4027

axd1967 opened this issue Dec 26, 2024 · 8 comments
Labels
enhancement Improve existing functionality importance: low Small problem, rarely visible, no crash
Milestone

Comments

@axd1967
Copy link
Contributor

axd1967 commented Dec 26, 2024

in order to not clutter the user directory, logging should be sent to a user-selected subdirectory (if desired).

Despite having created a correct and acceptable subdirectory, following script does not work :

#!/usr/bin/env bash

VERSION=master
cd /home/alex/projects/stellarium/

USER_DIR=$PWD/configs/$VERSION
LOG_DIR=$USER_DIR/logs
mkdir $LOG_DIR
dtg=`date --utc --iso-8601='seconds'`	
LOGFILE=$LOG_DIR/log_$dtg.txt

OPTION=""
OPTION=$OPTION" --user-dir $USER_DIR"
OPTION=$OPTION" --log-file $LOGFILE"

./builds/$VERSION/bin/stellarium $OPTION

log output:


./builds/master/bin/stellarium --user-dir /home/alex/projects/stellarium/configs/master --log-file =/home/alex/projects/stellarium/configs/master/logs/log_2024-12-26T15:43:39+00:00.txt 
 ---------------------------------------------------------------------------------
[ This is Stellarium 24.3+ (v24.3.301-4ee5f20 [master]) - https://stellarium.org/ ]
[ Copyright (C) 2000-2024 Stellarium Developers                                   ]
 ---------------------------------------------------------------------------------
Writing log file to: /home/alex/projects/stellarium/configs/master/log_2024-12-26T15:43:39+00:00.txt

Solution

The argument to --log-file should only be sanitized (reduced to eg userdir/(basename logfile)) if it cannot be created (eg permission issues).

User guide currently explains this behaviour (but not why).

See a541fc7

@gzotti gzotti added enhancement Improve existing functionality importance: low Small problem, rarely visible, no crash labels Dec 26, 2024
Copy link

Hello @axd1967!

Thank you for suggesting this enhancement.

@10110111
Copy link
Contributor

--log-file =/home/alex/projects...

You have an extra = here, which makes your path invalid.

@axd1967
Copy link
Contributor Author

axd1967 commented Dec 27, 2024

thank you. that must have been a copy-paste issue into GitHub.

The issue remains:

./builds/master/bin/stellarium --user-dir /home/alex/projects/stellarium/configs/master --log-file /home/alex/projects/stellarium/configs/master/logs/log_2024-12-27T08:01:54+00:00.txt 
 ---------------------------------------------------------------------------------
[ This is Stellarium 24.3+ (v24.3.301-4ee5f20 [master]) - https://stellarium.org/ ]
[ Copyright (C) 2000-2024 Stellarium Developers                                   ]
 ---------------------------------------------------------------------------------
Writing log file to: /home/alex/projects/stellarium/configs/master/log_2024-12-27T08:01:54+00:00.txt
File search paths:
 [0]: /home/alex/projects/stellarium/configs/master
 [1]: /home/alex/projects/astro/stellarium/builds/master/share/stellarium
Config file: /home/alex/projects/stellarium/configs/master/config.ini

The issue is here:

stellarium/src/main.cpp

Lines 269 to 271 in 226831f

// Strip external paths!
QFileInfo fi(logName);
logName=fi.fileName();

And mentioned in the manual:

For example, using the option \texttt{-c\ my\_config.ini} would resolve to the file
\file{\textless{}user\ directory\textgreater{}/my\_config.ini} whereas
\file{-c\ ./my\_config.ini} can be used to explicitly point to the file
\file{my\_config.ini} in the current working directory.\\
-\/-log-file or -l & log file name & Specify the log file name. The default value is \file{log.txt}.
Any path is stripped from the filename. When a path is specified like \texttt{-l\ /mypath/my\_log.txt},
the file will be written to \file{\textless{}user\ directory\textgreater{}/my\_log.txt}\\\midrule

It is not clear why this limit was imposed.

@10110111
Copy link
Contributor

It is not clear why this limit was imposed.

Well, apparently the intent was to change the name only. I don't think it's a good design though. I'd let the user specify any path, it's their machine after all, not ours.

@alex-w
Copy link
Member

alex-w commented Dec 27, 2024

Well, apparently the intent was to change the name only. I don't think it's a good design though. I'd let the user specify any path, it's their machine after all, not ours.

The behavior was changed by security reasons

@10110111
Copy link
Contributor

The behavior was changed by security reasons

Well first, this was introduced in a541fc7 in this form right from the start.

And second, what security reasons can be here? This is taken from the direct input by the user themselves! It's not a potentially misbehaving script, nothing like this — just user input. Or would GCC or Vim also need to impose similar restrictions on their command-line arguments for security reasons?

@gzotti
Copy link
Member

gzotti commented Dec 27, 2024

I cannot remember what we discussed in summer 2023 before I introduced the user-configurable logfile name. Probably this was under influence of the previous "security warning" around potentially "dangerous" scripting output. If you feel it's OK to write elsewhere, we can probably just do it.

@alex-w alex-w added this to the 25.1 milestone Dec 27, 2024
@10110111
Copy link
Contributor

If you feel it's OK to write elsewhere

I'm sure it is when it's the user who directed the program to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality importance: low Small problem, rarely visible, no crash
Development

No branches or pull requests

4 participants