Scheduled task cannot open log file

[ublukas]

Describe the bug
The scheduled task for mailable.editorialReminder fails to open its logfile when using the german locale.
It uses the translation as a log file name (EineautomatisierteE-Mail,dieanRedakteur/innenmitaussstehendenAufgabengeschicktwird-664961d155097-20240519.log) which cannot be created since it contains a /.

Proposed Solution
Remove or escape all special characters in the file name.

What application are you using?
OMP version 3.4.0-5

Additional information
stacktrace:

PHP Warning:  fopen(/data/omp-files/scheduledTaskLogs/EineautomatisierteE-Mail,dieanRedakteur/innenmitaussstehendenAufgabengeschicktwird-664961d155097-20240519.log): Failed to open stream: No such file or directory in /data/www/omp-test/lib/pkp/classes/scheduledTask/ScheduledTask.php on line 127
PHP Fatal error:  Uncaught TypeError: flock(): Argument #1 ($stream) must be of type resource, bool given in /data/www/omp-test/lib/pkp/classes/scheduledTask/ScheduledTask.php:128
Stack trace:
#0 /data/www/omp-test/lib/pkp/classes/scheduledTask/ScheduledTask.php(128): flock()
#1 /data/www/omp-test/lib/pkp/classes/scheduledTask/ScheduledTask.php(162): PKP\scheduledTask\ScheduledTask->addExecutionLogEntry()
#2 /data/www/omp-test/lib/pkp/classes/cliTool/ScheduledTaskTool.php(134): PKP\scheduledTask\ScheduledTask->execute()
#3 /data/www/omp-test/lib/pkp/classes/cliTool/ScheduledTaskTool.php(111): PKP\cliTool\ScheduledTaskTool->executeTask()
#4 /data/www/omp-test/lib/pkp/classes/cliTool/ScheduledTaskTool.php(81): PKP\cliTool\ScheduledTaskTool->parseTasks()
#5 /data/www/omp-test/tools/runScheduledTasks.php(26): PKP\cliTool\ScheduledTaskTool->execute()
#6 {main}
  thrown in /data/www/omp-test/lib/pkp/classes/scheduledTask/ScheduledTask.php on line 128

---

PRs:
stable-3_3_0:

• pkp/pkp-lib#9979 use class name for scheduled task log file name #10043
• pkp/pkp-lib#9979 submodule update ##bozana/9979-3_3_0## ojs#4310 (only submodule update)
• pkp/pkp-lib#9979 submodule update ##bozana/9979-3_3_0## omp#1586 (only submodule update)
• pkp/pkp-lib#9979 submodule update ##bozana/9979-3_3_0## ops#700 (only submodule update)

stable-3_4_0:

• pkp/pkp-lib#9979 use class name for scheduled task log file name #10037
• pkp/pkp-lib#9979 submodule update ##bozana/9979-3_4_0## ojs#4306 (only submodule update)
• pkp/pkp-lib#9979 submodule update ##bozana/9979-3_4_0## omp#1582 (only submodule update)
• pkp/pkp-lib#9979 submodule update ##bozana/9979-3_4_0## ops#697 (only submodule update)

main:

• pkp/pkp-lib#9979 use class name for scheduled task log file name #10044
• pkp/pkp-lib#9979 submodule update ##bozana/9979## ojs#4311 (only submodule update)
• pkp/pkp-lib#9979 submodule update ##bozana/9979## omp#1588 (only submodule update)
• pkp/pkp-lib#9979 submodule update ##bozana/9979## ops#701 (only submodule update)

[bozana]

@asmecher, what do you think about using the class name for the scheduled tasks log file names instead of translated scheduled task name, s. https://github.com/pkp/pkp-lib/blob/stable-3_4_0/classes/scheduledTask/ScheduledTask.php#L55 ?

[ublukas]

The admin.scheduledTask.removeUnvalidatedExpiredUsers Task has the same problem.

The filename is UnvalidierteabgelaufeneBenutzer/innenentfernen-665a8551ee071-20240601.log

[jonasraoni]

@bozana I was also going to propose changing this 😁

• Looks like there's room for security issues on the implementation. The "file manager" isn't sandboxed, so a filename with special characters (../) opens room for creativity.
• It's harder to instruct users to find the logs Please, look for the log with the name errrrr....
• As it's a log file, meant for sysadmins/developers, I also think that the internal messages of the file should be kept in English (perhaps to be handled by another issue).

[bozana]

Thanks a lot!
I will then change the file name to contain the class name, instead of the name translation of the scheduled tasks...

[bozana]

@jonasraoni, could you please review the PR above? Do you think I should also change it for stable_3_3_0?
Thanks a lot!

[jonasraoni]

I consider it as a bug, so I think it's ok to modify the stable-3_3_0.

[jonasraoni]

Ah, as we're using just the last part of the class name, one possible problem that might happen in the future (for the applications that use namespace) is a name collision (e.g. /a/b/c/ScheduledTask and /x/y/z/ScheduledTask).

Perhaps it's not needed to fix, depending on how the #9678 is being implemented.

[touhidurabir]

using just the last part of the class name, one possible problem that might happen in the future (for the applications that use namespace) is a name collision (e.g. /a/b/c/ScheduledTask and /x/y/z/ScheduledTask).

@jonasraoni as we add prefix the class name with a unique process id using uniqid method, the exact name collision will not happens but it will be hard to understand for given the above example as which task it is . I am wondering if we should take the full class namespace path and replace \ with _ .

Also the last part only use Ymd and I am thinking of changing it to Y_M_D_H_i_S which provide a better information of exact time to log generation .

[jonasraoni]

Yep, that's what I've inferred and what I'm worried about, to have a meaningful name for a sysadmin. Using the full class name doesn't make up for a beautiful name, but it's better.

Instead of including the time, I'd rather have a single log file for the task (every entry on the file already has a timestamp). It should simplify the implementation of a log rotation by end users and decrease the filesystem pollution.

I'll ask internally if the hosting team agrees and/or has extra considerations.

@asmecher / @bozana If you agree with the above, I'll create a side-task.

[bozana]

Hi @jonasraoni, I agree.
How would/could then the log file rotation function? Based on the file size?

[jonasraoni]

Yep, that's how it works, once it gets too large, the old content is archived.
We could provide the log rotation as an extra feature, but sysadmins should have this under their belt already (see: https://linuxconfig.org/logrotate).

[touhidurabir]

Instead of including the time, I'd rather have a single log file for the task (every entry on the file already has a timestamp). It should simplify the implementation of a log rotation by end users and decrease the filesystem pollution.

@jonasraoni is that is the path we plan to take on (which is better), perhaps we can consider #7966 ? However it will be some good amount of work .

[ajnyga]

Hi,

On a new installation of latest stable release of OJS, I keep getting this error on each page load:
PHP Fatal error: Uncaught TypeError: flock(): Argument #1 ($stream) must be of type resource, bool given in /public_html/lib/pkp/classes/observers/listeners/LogUsageEvent.php:104

Was this not fixed already / how can I avoid filling my error with this?

edit: Nevermind! This type of error can also be caused if the write permissions in files folder are not set correctly. Sorry for alerting you!