From 599037451ad4181244fa6547bce0976bcfe414f4 Mon Sep 17 00:00:00 2001 From: jhs-panda Date: Fri, 20 Sep 2024 14:18:10 -0400 Subject: [PATCH 1/6] Add checks for existing and valid Action when creating new scheduler or updating current scheduler. --- app/controllers/schedulers_controller.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/controllers/schedulers_controller.rb b/app/controllers/schedulers_controller.rb index cb5061134..04a811734 100755 --- a/app/controllers/schedulers_controller.rb +++ b/app/controllers/schedulers_controller.rb @@ -23,6 +23,15 @@ def new; end action_auth_level :create, :instructor def create @scheduler = @course.scheduler.new(scheduler_params) + action_command = scheduler_params[:action] + # Check if the action file exists and is readable + action_path = Rails.root.join(action_command).to_path + unless File.exist?(action_path) && File.readable?(action_path) + flash[:error] = "Scheduler create failed. Action file does not exist or is not + readable at #{action_path}." + redirect_to(new_course_scheduler_path(@course)) and return + end + if @scheduler.save flash[:success] = "Scheduler created!" redirect_to(course_schedulers_path(@course)) @@ -87,6 +96,15 @@ def visual_run action_auth_level :update, :instructor def update @scheduler = Scheduler.find_by(id: params[:id]) + action_command = scheduler_params[:action] + # Check if the action file exists and is readable + action_path = Rails.root.join(action_command).to_path + unless File.exist?(action_path) && File.readable?(action_path) + flash[:error] = "Scheduler create failed. Action file does not exist or is not + readable at #{action_path}." + redirect_to(edit_course_scheduler_path(@course)) and return + end + if @scheduler&.update(scheduler_params) flash[:success] = "Scheduler updated." redirect_to(course_schedulers_path(@course)) From dcb71c26cb765d03965c17421f9db72eb01e9c8a Mon Sep 17 00:00:00 2001 From: jhs-panda Date: Fri, 20 Sep 2024 15:57:12 -0400 Subject: [PATCH 2/6] Fix formatting. --- app/controllers/schedulers_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/schedulers_controller.rb b/app/controllers/schedulers_controller.rb index 04a811734..4862d16f8 100755 --- a/app/controllers/schedulers_controller.rb +++ b/app/controllers/schedulers_controller.rb @@ -27,8 +27,8 @@ def create # Check if the action file exists and is readable action_path = Rails.root.join(action_command).to_path unless File.exist?(action_path) && File.readable?(action_path) - flash[:error] = "Scheduler create failed. Action file does not exist or is not - readable at #{action_path}." + flash[:error] = "Scheduler create failed. Action file does not exist or is + not readable at #{action_path}." redirect_to(new_course_scheduler_path(@course)) and return end @@ -100,8 +100,8 @@ def update # Check if the action file exists and is readable action_path = Rails.root.join(action_command).to_path unless File.exist?(action_path) && File.readable?(action_path) - flash[:error] = "Scheduler create failed. Action file does not exist or is not - readable at #{action_path}." + flash[:error] = "Scheduler create failed. Action file does not exist or is + not readable at #{action_path}." redirect_to(edit_course_scheduler_path(@course)) and return end From 288fc882d5b6562c4e58d3a55b02c6beb03cdf49 Mon Sep 17 00:00:00 2001 From: jhs-panda Date: Sun, 22 Sep 2024 13:53:58 -0400 Subject: [PATCH 3/6] Only allow for creating new schedulers and updating current schedulers if visual run under action does not return an error. --- app/controllers/schedulers_controller.rb | 72 ++++++++++++++++++++---- 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/app/controllers/schedulers_controller.rb b/app/controllers/schedulers_controller.rb index 4862d16f8..b2914888c 100755 --- a/app/controllers/schedulers_controller.rb +++ b/app/controllers/schedulers_controller.rb @@ -23,9 +23,8 @@ def new; end action_auth_level :create, :instructor def create @scheduler = @course.scheduler.new(scheduler_params) - action_command = scheduler_params[:action] # Check if the action file exists and is readable - action_path = Rails.root.join(action_command).to_path + action_path = Rails.root.join(scheduler_params[:action]).to_path unless File.exist?(action_path) && File.readable?(action_path) flash[:error] = "Scheduler create failed. Action file does not exist or is not readable at #{action_path}." @@ -33,8 +32,16 @@ def create end if @scheduler.save - flash[:success] = "Scheduler created!" - redirect_to(course_schedulers_path(@course)) + # Ensure visual run is successful + begin + test_run_visual_scheduler(@scheduler) + flash[:success] = "Scheduler created and executed successfully!" + redirect_to(course_schedulers_path(@course)) + rescue StandardError => e + @scheduler.destroy # Destroy the created scheduler if visual run fails + flash[:error] = "Scheduler creation failed. Error: #{e.message}" + redirect_to(new_course_scheduler_path(@course)) + end else flash[:error] = "Scheduler create failed. Please check all fields." redirect_to(new_course_scheduler_path(@course)) @@ -96,18 +103,29 @@ def visual_run action_auth_level :update, :instructor def update @scheduler = Scheduler.find_by(id: params[:id]) - action_command = scheduler_params[:action] # Check if the action file exists and is readable - action_path = Rails.root.join(action_command).to_path + action_path = Rails.root.join(scheduler_params[:action]).to_path unless File.exist?(action_path) && File.readable?(action_path) - flash[:error] = "Scheduler create failed. Action file does not exist or is + flash[:error] = "Scheduler update failed. Action file does not exist or is not readable at #{action_path}." redirect_to(edit_course_scheduler_path(@course)) and return end - if @scheduler&.update(scheduler_params) - flash[:success] = "Scheduler updated." - redirect_to(course_schedulers_path(@course)) + # Save the current state of the scheduler in case we need to revert + previous_scheduler_state = @scheduler.attributes + + if @scheduler.update(scheduler_params) + begin + # Run the visual scheduler to ensure the new update works + test_run_visual_scheduler(@scheduler) + flash[:success] = "Scheduler updated and executed successfully!" + redirect_to(course_schedulers_path(@course)) + rescue StandardError => e + @scheduler.update(previous_scheduler_state) # If error, revert to previous state. + flash[:error] = "Scheduler update failed. Reverting to previous state. + Error: #{e.message}" + redirect_to(edit_course_scheduler_path(@course, @scheduler)) + end else flash[:error] = "Scheduler update failed! Please check your fields." redirect_to(edit_course_scheduler_path(@course, @scheduler)) @@ -137,4 +155,38 @@ def set_manage_scheduler_breadcrumb @breadcrumbs << (view_context.link_to "Manage Schedulers", course_schedulers_path(@course)) end + + def test_run_visual_scheduler(scheduler) + # Do a test visual run to check if created/updated info valid + action = scheduler + read, write = IO.pipe + + pid = fork do + read.close + mod_name = Rails.root.join(action.action).to_path + fork_log = "" + begin + require mod_name + output = Updater.update(action.course) + if output.respond_to?(:to_str) + fork_log << "----- Script Output -----\n" + fork_log << output + fork_log << "\n----- End Script Output -----" + end + rescue ScriptError, StandardError => e + fork_log << "----- Script Error Output -----\n" + fork_log << "Error in '#{action.course.name}' updater: #{e.message}\n" + fork_log << e.backtrace.join("\n\t") + fork_log << "\n---- End Script Error Output -----" + end + write.print fork_log + end + + write.close + result = read.read + Process.wait2(pid) + + # Raise an exception if something goes wrong + raise "Scheduler execution failed." unless result.is_a?(String) && result.include?("Error") + end end From 352afbad6749e9010e2da886befc40077ec592e7 Mon Sep 17 00:00:00 2001 From: jhs-panda Date: Sun, 22 Sep 2024 14:22:55 -0400 Subject: [PATCH 4/6] Correct logic of returning error. --- app/controllers/schedulers_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/schedulers_controller.rb b/app/controllers/schedulers_controller.rb index b2914888c..bfa1ff2a1 100755 --- a/app/controllers/schedulers_controller.rb +++ b/app/controllers/schedulers_controller.rb @@ -187,6 +187,8 @@ def test_run_visual_scheduler(scheduler) Process.wait2(pid) # Raise an exception if something goes wrong - raise "Scheduler execution failed." unless result.is_a?(String) && result.include?("Error") + return unless result.is_a?(String) && result.include?("Error") + + raise "Scheduler execution failed." end end From 25d78cf04a008e59e5a9342f0d0b15cb7e9cc7bd Mon Sep 17 00:00:00 2001 From: jhs-panda Date: Fri, 1 Nov 2024 16:47:19 -0400 Subject: [PATCH 5/6] Addressed comments in review. --- app/controllers/schedulers_controller.rb | 206 +++++++++++------------ 1 file changed, 96 insertions(+), 110 deletions(-) diff --git a/app/controllers/schedulers_controller.rb b/app/controllers/schedulers_controller.rb index bfa1ff2a1..9fa435614 100755 --- a/app/controllers/schedulers_controller.rb +++ b/app/controllers/schedulers_controller.rb @@ -23,29 +23,23 @@ def new; end action_auth_level :create, :instructor def create @scheduler = @course.scheduler.new(scheduler_params) - # Check if the action file exists and is readable action_path = Rails.root.join(scheduler_params[:action]).to_path - unless File.exist?(action_path) && File.readable?(action_path) - flash[:error] = "Scheduler create failed. Action file does not exist or is - not readable at #{action_path}." - redirect_to(new_course_scheduler_path(@course)) and return - end - - if @scheduler.save - # Ensure visual run is successful - begin - test_run_visual_scheduler(@scheduler) - flash[:success] = "Scheduler created and executed successfully!" - redirect_to(course_schedulers_path(@course)) - rescue StandardError => e - @scheduler.destroy # Destroy the created scheduler if visual run fails - flash[:error] = "Scheduler creation failed. Error: #{e.message}" - redirect_to(new_course_scheduler_path(@course)) + # Check if the action file exists, is readable, and compiles + if validate_compile_action_file(action_path) + if @scheduler.save + # Ensure visual run is successful + if run_visual_scheduler(@scheduler) + flash[:success] = "Scheduler created and executed successfully!" + redirect_to(course_schedulers_path(@course)) and return + else + @scheduler.destroy + flash[:error] = "Scheduler creation failed during execution." + end + else + flash[:error] = "Scheduler create failed. Please check all fields." end - else - flash[:error] = "Scheduler create failed. Please check all fields." - redirect_to(new_course_scheduler_path(@course)) end + redirect_to(new_course_scheduler_path(@course)) end action_auth_level :edit, :instructor @@ -60,76 +54,32 @@ def run action_auth_level :visual_run, :instructor def visual_run - action = Scheduler.find(params[:scheduler_id]) - # https://stackoverflow.com/a/1076445 - read, write = IO.pipe - @log = "Executing #{Rails.root.join(action.action)}\n" - begin - pid = fork do - read.close - mod_name = Rails.root.join(action.action).to_path - fork_log = "" - begin - require mod_name - output = Updater.update(action.course) - if output.respond_to?(:to_str) - fork_log << "----- Script Output -----\n" - fork_log << output - fork_log << "\n----- End Script Output -----" - end - rescue ScriptError, StandardError => e - fork_log << "----- Script Error Output -----\n" - fork_log << "Error in '#{@course.name}' updater: #{e.message}\n" - fork_log << e.backtrace.join("\n\t") - fork_log << "\n---- End Script Error Output -----" - end - write.print fork_log - end - - write.close - result = read.read - Process.wait2(pid) - @log << result - rescue StandardError => e - @log << "----- Error Output -----\n" - @log << "Error in '#{@course.name}' updater: #{e.message}\n" - @log << e.backtrace.join("\n\t") - @log << "\n---- End Error Output -----" - end - @log << "\nCompleted running action." + @scheduler = Scheduler.find(params[:scheduler_id]) + @log = execute_action(@scheduler) render partial: "visual_test" end action_auth_level :update, :instructor def update @scheduler = Scheduler.find_by(id: params[:id]) - # Check if the action file exists and is readable action_path = Rails.root.join(scheduler_params[:action]).to_path - unless File.exist?(action_path) && File.readable?(action_path) - flash[:error] = "Scheduler update failed. Action file does not exist or is - not readable at #{action_path}." - redirect_to(edit_course_scheduler_path(@course)) and return - end - - # Save the current state of the scheduler in case we need to revert - previous_scheduler_state = @scheduler.attributes - - if @scheduler.update(scheduler_params) - begin - # Run the visual scheduler to ensure the new update works - test_run_visual_scheduler(@scheduler) - flash[:success] = "Scheduler updated and executed successfully!" - redirect_to(course_schedulers_path(@course)) - rescue StandardError => e - @scheduler.update(previous_scheduler_state) # If error, revert to previous state. - flash[:error] = "Scheduler update failed. Reverting to previous state. - Error: #{e.message}" - redirect_to(edit_course_scheduler_path(@course, @scheduler)) + # Check if the action file exists, is readable, and compiles + if validate_compile_action_file(action_path) + previous_state = @scheduler.attributes + if @scheduler.update(scheduler_params) + # Ensure visual run is successful + if run_visual_scheduler(@scheduler) + flash[:success] = "Scheduler updated and executed successfully!" + redirect_to(course_schedulers_path(@course)) and return + else + @scheduler.update(previous_state) # If error, revert to previous state. + flash[:error] = "Scheduler update failed during execution. Reverted to previous state." + end + else + flash[:error] = "Scheduler update failed! Please check your fields." end - else - flash[:error] = "Scheduler update failed! Please check your fields." - redirect_to(edit_course_scheduler_path(@course, @scheduler)) end + redirect_to(edit_course_scheduler_path(@course, @scheduler)) end action_auth_level :destroy, :instructor @@ -156,39 +106,75 @@ def set_manage_scheduler_breadcrumb @breadcrumbs << (view_context.link_to "Manage Schedulers", course_schedulers_path(@course)) end - def test_run_visual_scheduler(scheduler) - # Do a test visual run to check if created/updated info valid - action = scheduler - read, write = IO.pipe + def validate_compile_action_file(action_path) + # Check if the action file exists and is readable + unless File.exist?(action_path) && File.readable?(action_path) + flash[:error] = "Scheduler update failed. Action file does not exist or is + not readable at #{action_path}." + return false + end - pid = fork do - read.close - mod_name = Rails.root.join(action.action).to_path - fork_log = "" - begin - require mod_name - output = Updater.update(action.course) - if output.respond_to?(:to_str) - fork_log << "----- Script Output -----\n" - fork_log << output - fork_log << "\n----- End Script Output -----" - end - rescue ScriptError, StandardError => e - fork_log << "----- Script Error Output -----\n" - fork_log << "Error in '#{action.course.name}' updater: #{e.message}\n" - fork_log << e.backtrace.join("\n\t") - fork_log << "\n---- End Script Error Output -----" - end - write.print fork_log + # compile action file to check for syntax errors + begin + RubyVM::InstructionSequence.compile(File.read(action_path)) + rescue SyntaxError => e + flash[:error] = "Syntax error in action file: #{e.message}" + return false end - write.close - result = read.read - Process.wait2(pid) + true + end - # Raise an exception if something goes wrong - return unless result.is_a?(String) && result.include?("Error") + def run_visual_scheduler(scheduler) + log = execute_action(scheduler) + # Ensure visual run is successful or return error + if log.include?("Error") + flash[:error] = "Scheduler execution failed." + false + else + flash[:success] = "Scheduler executed successfully!" + true + end + end - raise "Scheduler execution failed." + def execute_action(scheduler) + action_path = Rails.root.join(scheduler.action).to_path + # https://stackoverflow.com/a/1076445 + read, write = IO.pipe + log = "Executing #{action_path}\n" + begin + pid = fork do + read.close + mod_name = action_path + fork_log = "" + begin + require mod_name + output = Updater.update(scheduler.course) + if output.respond_to?(:to_str) + fork_log << "----- Script Output -----\n" + fork_log << output + fork_log << "\n----- End Script Output -----" + end + rescue ScriptError, StandardError => e + fork_log << "----- Script Error Output -----\n" + fork_log << "Error in '#{scheduler.course.name}' updater: #{e.message}\n" + fork_log << e.backtrace.join("\n\t") + fork_log << "\n---- End Script Error Output -----" + end + write.print fork_log + end + + write.close + result = read.read + Process.wait2(pid) + log << result + rescue StandardError => e + log << "----- Error Output -----\n" + log << "Error during execution: #{e.message}\n" + log << e.backtrace.join("\n\t") + log << "\n---- End Error Output -----" + end + log << "\nCompleted running action." + log end end From 5f48195acf6b0f4e18cd9f4c8f0446c1a060bd50 Mon Sep 17 00:00:00 2001 From: jhs-panda Date: Wed, 13 Nov 2024 12:01:25 -0500 Subject: [PATCH 6/6] Added additional error checking suggested by CodeRabbit. --- app/controllers/schedulers_controller.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/controllers/schedulers_controller.rb b/app/controllers/schedulers_controller.rb index 9fa435614..0863fb474 100755 --- a/app/controllers/schedulers_controller.rb +++ b/app/controllers/schedulers_controller.rb @@ -120,6 +120,9 @@ def validate_compile_action_file(action_path) rescue SyntaxError => e flash[:error] = "Syntax error in action file: #{e.message}" return false + rescue StandardError => e + flash[:error] = "Error validating action file: #{e.message}" + return false end true