From 7c74a9929e8df2167034e8e43df28ba2dfaa3472 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 3 Aug 2023 18:30:37 -0500 Subject: [PATCH] Remove the pass through createDirectory method in PGcore.pm. This method is not used anywhere in PG except directly in that file, and PGcore.pm can direcly use the WeBWorK::PG::IO::createDirectory method. This method existing means that any problem can call `$PG->createDirectory('/path/to/dir', 0770, -1)` to create a directory anywhere that the server has write permissions for. Also the WeBWorK::PG::IO module should not export methods by default (this is done by setting `our @EXPORT`). Instead it should only export methods on request using `our @EXPORT_OK`. Furthermore, since that array doubles as the list of methods that are shared in the safe compartment, care should be taken as to what is included in that list. In particular the `createDirectory` method should not be. Otherwise problems can directly call `createDirectory('/path/to/dir', 0770, -1)`. Any other `lib` module that wants to use the WeBWorK::PG::IO methods can still do so either by importing the desired exported method by name (if exported), or by calling the method with its full namespace (as is done everywhere currently). Note that it is not necessary to `use WeBWorK::PG::IO` to use the methods therein when calling a method with its full namespace. It is necessary that at some point the module is "required" for this to work. This technically can be done in any file, but if a file uses a module, then for clarity and to be sure the module is loaded that file should call `require module`. Also correct the list of modules in `pg_config.dist.yml` and the comments on how that works. This is all about the difference between "use Module", "require Module", and "Module->import" (note `import` is a method and should not be called as `import Module`). Calling `use Module LIST` is exactly equivalent to `BEGIN { require Module; Module->import(LIST); }`. Note that to call `require` on a module it must be contained in a file by the same name with the `.pm` extension. If a module is listed first in the modules list, then essentially that is equivalent to calling `use Module` (which both requires and imports the module and the module must be contained in its own file). If a module is listed second, then that is roughly equivalent to calling `Module->import`. For PG and translation, this means that if a module has its own file (i.e., Module is defined in Module.pm), then it should be first in the list, and if Module2 is defined in Module.pm, then Module2 should be listed in the same list after Module. Note that these changes will also need to made for webwork2 in defaults.config. --- conf/pg_config.dist.yml | 26 ++++++++++++++++++-------- lib/LaTeXImage.pm | 8 ++++---- lib/PGcore.pm | 13 +++---------- lib/WeBWorK/PG/IO.pm | 8 +------- lib/WeBWorK/PG/Translator.pm | 2 +- 5 files changed, 27 insertions(+), 30 deletions(-) diff --git a/conf/pg_config.dist.yml b/conf/pg_config.dist.yml index 144587cdc5..80b16595ce 100644 --- a/conf/pg_config.dist.yml +++ b/conf/pg_config.dist.yml @@ -52,7 +52,6 @@ directories: - $pg_root/macros/ui - $pg_root/macros/deprecated - URLs: # The public URL of the html directory above. html: /pg_files @@ -201,9 +200,10 @@ displayModeOptions: passwd: '' # PG modules to load -# The first item of each list is the module to load. The remaining items are additional packages to import. -# That is: If you wish to include a module MyModule.pm which depends on additional modules Dependency1.pm and -# Dependency2.pm, these should appear as [Mymodule, Dependency1, Dependency2] +# The first item of each list is the module file to load. The remaining items are additional packages to import that are +# also contained in that file. +# That is, if you wish to include a file MyModule.pm which containes the package MyModule and the additional packages +# Dependency1 and Dependency2, then these should appear as [Mymodule, Dependency1, Dependency2]. modules: - [Encode] - ['Encode::Encoding'] @@ -212,7 +212,8 @@ modules: - [DynaLoader] - [Exporter] - [GD] - - [AlgParser, AlgParserWithImplicitExpand, Expr, ExprWithImplicitExpand, utf8] + - [utf8] + - [AlgParser, AlgParserWithImplicitExpand, Expr, ExprWithImplicitExpand] - [AnswerHash, AnswerEvaluator] - [LaTeXImage] - [WWPlot] # required by Circle (and others) @@ -235,14 +236,23 @@ modules: - [Select] - [Units] - [VectorField] - - [Parser, Value] + - [Parser] + - [Value] - ['Parser::Legacy'] - [Statistics] - [Chromatic] # for Northern Arizona graph problems - [Applet] - - [PGcore, PGalias, PGresource, PGloadfiles, PGanswergroup, PGresponsegroup, 'Tie::IxHash'] + - [PGcore] + - [PGalias] + - [PGresource] + - [PGloadfiles] + - [PGanswergroup] + - [PGresponsegroup] + - ['Tie::IxHash'] - ['Locale::Maketext'] - [JSON] - - [Rserve, 'Class::Tiny', 'IO::Handle'] + - ['Class::Tiny'] + - ['IO::Handle'] + - ['Rserve'] - [DragNDrop] - ['Types::Serialiser'] diff --git a/lib/LaTeXImage.pm b/lib/LaTeXImage.pm index b3503c3cec..042634a572 100644 --- a/lib/LaTeXImage.pm +++ b/lib/LaTeXImage.pm @@ -18,13 +18,13 @@ # simple images using LaTeX, and converting them into a web-useable format. Its # typical usage is via the macro PGtikz.pl and is documented there. +package LaTeXImage; + use strict; use warnings; -use Carp; -use WeBWorK::PG::IO; -use WeBWorK::PG::ImageGenerator; -package LaTeXImage; +require WeBWorK::PG::IO; +require WeBWorK::PG::ImageGenerator; # The constructor (it takes no parameters) sub new { diff --git a/lib/PGcore.pm b/lib/PGcore.pm index b9f7e809bd..0bc480fff8 100755 --- a/lib/PGcore.pm +++ b/lib/PGcore.pm @@ -32,7 +32,7 @@ use PGrandom; use PGalias; use PGloadfiles; use AnswerHash; -use WeBWorK::PG::IO(); # don't important any command directly +require WeBWorK::PG::IO; use Tie::IxHash; use MIME::Base64(); use PGUtil(); @@ -764,7 +764,6 @@ sub getUniqueName { convertPath fileFromPath directoryFromPath - createDirectory =cut @@ -801,11 +800,6 @@ sub directoryFromPath { WeBWorK::PG::IO::directoryFromPath(@_); } -sub createDirectory { - my $self = shift; - WeBWorK::PG::IO::createDirectory(@_); -} - sub AskSage { my $self = shift; my $python = shift; @@ -835,7 +829,6 @@ course temp directory. # ^function surePathToTmpFile # ^uses getCourseTempDirectory -# ^uses createDirectory sub surePathToTmpFile { # constructs intermediate directories if needed beginning at ${Global::htmlDirectory}tmp/ @@ -851,7 +844,7 @@ sub surePathToTmpFile { $parentDirectory = $self->directoryFromPath($parentDirectory); my ($perms, $groupID) = (stat $parentDirectory)[ 2, 5 ]; #warn "Creating tmp directory at $tmpDirectory, perms $perms groupID $groupID"; - $self->createDirectory($tmpDirectory, $perms, $groupID) + WeBWorK::PG::IO::createDirectory($tmpDirectory, $perms, $groupID) or warn "Failed to create parent tmp directory at $path"; } @@ -872,7 +865,7 @@ sub surePathToTmpFile { $path = $path . shift(@nodes) . "/"; #convertPath($path . shift (@nodes) . "/"); unless (-e $path) { - $self->createDirectory($path, $perms, $groupID) + WeBWorK::PG::IO::createDirectory($path, $perms, $groupID) or $self->warning_message( "Failed to create directory at $path with permissions $perms and groupID $groupID"); } diff --git a/lib/WeBWorK/PG/IO.pm b/lib/WeBWorK/PG/IO.pm index 9329cf8da0..912c3c08f6 100644 --- a/lib/WeBWorK/PG/IO.pm +++ b/lib/WeBWorK/PG/IO.pm @@ -37,21 +37,15 @@ WeBWorK::PG::IO - Functions used by WeBWorK::PG::Translator for file IO. =cut -our @EXPORT = qw( +our @EXPORT_OK = qw( includePGtext read_whole_problem_file read_whole_file convertPath fileFromPath directoryFromPath - createDirectory ); -=head1 SYNOPSIS - - use WeBWorK::PG::IO; - my %functions_to_share = %WeBWorK::PG::IO::SHARE; - =head1 DESCRIPTION This module defines several functions to be shared with a safe compartment by diff --git a/lib/WeBWorK/PG/Translator.pm b/lib/WeBWorK/PG/Translator.pm index 1311d4fbae..d8f5133289 100644 --- a/lib/WeBWorK/PG/Translator.pm +++ b/lib/WeBWorK/PG/Translator.pm @@ -302,7 +302,7 @@ sub initialize { my $safe_cmpt = $self->{safe}; $safe_cmpt->share_from('WeBWorK::PG::Translator', \@Translator_shared_subroutine_array); - $safe_cmpt->share_from('WeBWorK::PG::IO', \@WeBWorK::PG::IO::EXPORT); + $safe_cmpt->share_from('WeBWorK::PG::IO', \@WeBWorK::PG::IO::EXPORT_OK); no strict; local (%envir) = %{ $self->{envir} };