diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 160eceee..6477994c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -27,7 +27,7 @@ Metrics/CyclomaticComplexity: Max: 20 Metrics/PerceivedComplexity: - Max: 22 + Max: 23 # We don't want to change previous migrations... # diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index a0d0203e..a444bac8 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -669,6 +669,7 @@ var CodeOceanEditor = { $('#flowrHint').fadeOut(); this.clearHints(); this.showOutputBar(); + this.clearFileDownloads(); }, isActiveFileBinary: function () { @@ -737,6 +738,24 @@ var CodeOceanEditor = { container.fadeIn(); }, + prepareFileDownloads: function(message) { + const fileTree = $('#download-file-tree'); + fileTree.jstree(message.data); + fileTree.on('select_node.jstree', function (node, selected, _event) { + selected.instance.deselect_all(); + const downloadPath = selected.node.original.download_path; + if (downloadPath) { + window.location = downloadPath; + } + }.bind(this)); + $('#download-files').removeClass('d-none'); + }, + + clearFileDownloads: function() { + $('#download-files').addClass('d-none'); + $('#download-file-tree').replaceWith($('
')); + }, + showContainerDepletedMessage: function () { $.flash.danger({ icon: ['fa-regular', 'fa-clock'], diff --git a/app/assets/javascripts/editor/execution.js b/app/assets/javascripts/editor/execution.js index 2fd6245b..d9787545 100644 --- a/app/assets/javascripts/editor/execution.js +++ b/app/assets/javascripts/editor/execution.js @@ -48,6 +48,7 @@ CodeOceanEditorWebsocket = { this.websocket.on('exit', this.handleExitCommand.bind(this)); this.websocket.on('status', this.showStatus.bind(this)); this.websocket.on('hint', this.showHint.bind(this)); + this.websocket.on('files', this.prepareFileDownloads.bind(this)); }, handleExitCommand: function() { diff --git a/app/controllers/concerns/file_conversion.rb b/app/controllers/concerns/file_conversion.rb new file mode 100644 index 00000000..bdc66675 --- /dev/null +++ b/app/controllers/concerns/file_conversion.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module FileConversion + private + + def convert_files_json_to_files(files_json) + all_file_types = FileType.all + directories = [] + files = files_json['files'].filter_map do |file| + # entryType: `-` describes a regular file, `d` a directory. See `info ls` for others + directories.push(file['name']) if file['entryType'] == 'd' + next unless file['entryType'] == '-' + + extension = File.extname(file['name']) + name = File.basename(file['name'], extension) + path = File.dirname(file['name']).sub(%r{^(?>\./|\.)}, '').presence + file_type = all_file_types.detect {|ft| ft.file_extension == extension } || FileType.new(file_extension: extension) + CodeOcean::File.new( + name: name, + path: path, + size: file['size'], + owner: file['owner'], + group: file['group'], + permissions: file['permissions'], + updated_at: file['modificationTime'], + file_type: file_type + ) + end + [augment_files_for_download(files), directories] + end + + def augment_files_for_download(files) + raise NotImplementedError + end +end diff --git a/app/controllers/live_streams_controller.rb b/app/controllers/live_streams_controller.rb new file mode 100644 index 00000000..fc86d462 --- /dev/null +++ b/app/controllers/live_streams_controller.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +class LiveStreamsController < ApplicationController + # Including ActionController::Live changes all actions in this controller! + # Therefore, it is extracted into a separate controller + include ActionController::Live + + def download_submission_file + begin + @submission = authorize AuthenticatedUrlHelper.retrieve!(Submission, request, force_render_host: false) + rescue Pundit::NotAuthorizedError + # TODO: Option to disable? + # Using the submission ID parameter would allow looking up the corresponding exercise ID + # Therefore, we just redirect to the root_path, but actually expect to redirect back (that should work!) + redirect_back(fallback_location: root_path, alert: t('exercises.download_file_tree.gone')) + end + + desired_file = params[:filename].to_s + runner = Runner.for(current_user, @submission.exercise.execution_environment) + fallback_location = implement_exercise_path(@submission.exercise) + send_runner_file(runner, desired_file, fallback_location) + end + + private + + def send_runner_file(runner, desired_file, redirect_fallback = root_path) + filename = File.basename(desired_file) + send_stream(filename: filename, disposition: 'attachment') do |stream| + runner.download_file desired_file do |chunk, overall_size, content_type| + unless response.committed? + # Disable Rack::ETag, which would otherwise cause the response to be cached + # See https://github.com/rack/rack/issues/1619#issuecomment-848460528 + response.set_header('Last-Modified', Time.now.httpdate) + response.set_header('Content-Length', overall_size) if overall_size + response.set_header('Content-Type', content_type) if content_type + # Commit the response headers immediately, as streaming would otherwise remove the Content-Length header + # This will prevent chunked transfer encoding from being used, which is okay as we know the overall size + # See https://github.com/rails/rails/issues/18714 + response.commit! + end + + if stream.connected? + stream.write chunk + else + # The client disconnected, so we stop streaming + break + end + end + rescue Runner::Error + redirect_back(fallback_location: redirect_fallback, alert: t('exercises.download_file_tree.gone')) + end + end + + # TODO: Taken from Rails 7, remove when upgrading + # rubocop:disable all + def send_stream(filename:, disposition: "attachment", type: nil) + response.headers["Content-Type"] = + (type.is_a?(Symbol) ? Mime[type].to_s : type) || + Mime::Type.lookup_by_extension(File.extname(filename).downcase.delete(".")) || + "application/octet-stream" + + response.headers["Content-Disposition"] = + ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename) + + yield response.stream + ensure + response.stream.close + end + # rubocop:enable all +end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index b5923848..d1b90aa2 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -3,6 +3,7 @@ class SubmissionsController < ApplicationController include CommonBehavior include Lti + include FileConversion include SubmissionParameters include Tubesock::Hijack @@ -177,7 +178,7 @@ class SubmissionsController < ApplicationController send_and_store client_socket, message end - runner_socket.on :exit do |exit_code| + runner_socket.on :exit do |exit_code, files| @testrun[:exit_code] = exit_code exit_statement = if @testrun[:output].empty? && exit_code.zero? @@ -200,6 +201,12 @@ class SubmissionsController < ApplicationController @testrun[:status] = :out_of_memory end + downloadable_files, = convert_files_json_to_files files + if downloadable_files.present? + js_tree = FileTree.new(downloadable_files).to_js_tree + send_and_store client_socket, {cmd: :files, data: js_tree} + end + close_client_connection(client_socket) end end @@ -427,4 +434,19 @@ class SubmissionsController < ApplicationController rescue JSON::ParserError {cmd: :write, stream: stream, data: data} end + + def augment_files_for_download(files) + submission_files = @submission.collect_files + files.filter_map do |file| + # Reject files that were already present in the submission + # We further reject files that share the same name (excl. file extension) and path as a file in the submission + # This is, for example, used to filter compiled .class files in Java submissions + next if submission_files.any? {|submission_file| submission_file.filepath_without_extension == file.filepath_without_extension } + + # Downloadable files get a signed download_path and an indicator whether we performed a privileged execution + file.download_path = AuthenticatedUrlHelper.sign(download_stream_file_submission_url(@submission, file.filepath), @submission) + file.privileged_execution = @submission.execution_environment.privileged_execution + file + end + end end diff --git a/app/helpers/authenticated_url_helper.rb b/app/helpers/authenticated_url_helper.rb index d949f555..d650a7aa 100644 --- a/app/helpers/authenticated_url_helper.rb +++ b/app/helpers/authenticated_url_helper.rb @@ -17,14 +17,14 @@ module AuthenticatedUrlHelper add_query_parameters(url, {TOKEN_PARAM => token}) end - def retrieve!(klass, request, cookies = {}) + def retrieve!(klass, request, cookies = {}, force_render_host: true) # Don't use the default session mechanism and default cookie - request.session_options[:skip] = true + request.session_options[:skip] = true if force_render_host # Show errors as JSON format, if any request.format = :json # Disallow access from normal domain and show an error instead - if ApplicationController::RENDER_HOST.present? && request.host != ApplicationController::RENDER_HOST + if force_render_host && ApplicationController::RENDER_HOST.present? && request.host != ApplicationController::RENDER_HOST raise Pundit::NotAuthorizedError end diff --git a/app/models/code_ocean/file.rb b/app/models/code_ocean/file.rb index 6d2621b0..2bbf38c1 100644 --- a/app/models/code_ocean/file.rb +++ b/app/models/code_ocean/file.rb @@ -18,6 +18,8 @@ module CodeOcean before_validation :set_ancestor_values, if: :incomplete_descendent? attr_writer :size + # These attributes are mainly used when retrieving files from a runner + attr_accessor :download_path belongs_to :context, polymorphic: true belongs_to :file, class_name: 'CodeOcean::File', optional: true # This is only required for submissions and is validated below @@ -100,6 +102,14 @@ module CodeOcean end end + def filepath_without_extension + if path.present? + ::File.join(path, name) + else + name + end + end + def hash_content self.hashed_content = Digest::MD5.new.hexdigest(read || '') end @@ -114,6 +124,10 @@ module CodeOcean name.to_s + (file_type&.file_extension || '') end + def name_with_extension_and_size + "#{name_with_extension} (#{ActionController::Base.helpers.number_to_human_size(size)})" + end + def set_ancestor_values %i[feedback_message file_type_id hidden name path read_only role weight].each do |attribute| send(:"#{attribute}=", ancestor.send(attribute)) diff --git a/app/models/runner.rb b/app/models/runner.rb index 1de054d6..f6475dac 100644 --- a/app/models/runner.rb +++ b/app/models/runner.rb @@ -52,6 +52,34 @@ class Runner < ApplicationRecord @strategy.copy_files(files) end + def download_file(path, **options, &block) + @strategy.download_file(path, **options, &block) + end + + def retrieve_files(raise_exception: true, **options) + try = 0 + begin + if try.nonzero? + request_new_id + save + end + @strategy.retrieve_files(**options) + rescue Runner::Error::RunnerNotFound => e + Rails.logger.debug { "Retrieving files failed for the first time: #{e.message}" } + try += 1 + + if try == 1 + # This is only used if no files were copied to the runner. Thus requesting a second runner is performed here + # Reset the variable. This is required to prevent raising an outdated exception after a successful second try + e = nil + retry + end + ensure + # We forward the exception if requested + raise e if raise_exception && defined?(e) && e.present? + end + end + def attach_to_execution(command, privileged_execution: false, &block) Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Starting execution with Runner #{id} for #{user_type} #{user_id}." } starting_time = Time.zone.now diff --git a/app/models/testrun_message.rb b/app/models/testrun_message.rb index 2b6d0b97..5b2e163a 100644 --- a/app/models/testrun_message.rb +++ b/app/models/testrun_message.rb @@ -17,6 +17,7 @@ class TestrunMessage < ApplicationRecord exception: 10, result: 11, canvasevent: 12, + files: 13, }, _default: :write, _prefix: true enum stream: { diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index c174c074..52dbae1e 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -6,7 +6,8 @@ class SubmissionPolicy < ApplicationPolicy end # insights? is used in the flowr_controller.rb as we use it to authorize the user for a submission - %i[download? download_file? run? score? show? statistics? stop? test? + # download_submission_file? is used in the live_streams_controller.rb + %i[download? download_file? download_submission_file? run? score? show? statistics? stop? test? insights?].each do |action| define_method(action) { admin? || author? } end diff --git a/app/views/exercises/_download_file_tree.html.slim b/app/views/exercises/_download_file_tree.html.slim new file mode 100644 index 00000000..44ed6b52 --- /dev/null +++ b/app/views/exercises/_download_file_tree.html.slim @@ -0,0 +1,8 @@ +div.enforce-bottom-margin.overflow-scroll.d-none#download-files + .card.border-secondary + .card-header.d-flex.justify-content-between.align-items-center.px-0.py-1 + .px-2 = t('exercises.download_file_tree.file_root') + + .card-body.pt-0.pe-0.ps-1.pb-1 + + #download-file-tree diff --git a/app/views/exercises/_editor_output.html.slim b/app/views/exercises/_editor_output.html.slim index c4838cba..5441eff3 100644 --- a/app/views/exercises/_editor_output.html.slim +++ b/app/views/exercises/_editor_output.html.slim @@ -3,7 +3,10 @@ div.d-grid id='output_sidebar_collapsed' div.d-grid id='output_sidebar_uncollapsed' class='d-none col-sm-12 enforce-bottom-margin' data-message-no-output=t('exercises.implement.no_output_yet') = render('editor_button', classes: 'btn-outline-dark btn overflow-hidden mb-2', icon: 'fa-solid fa-square-minus', id: 'toggle-sidebar-output', label: t('exercises.editor.collapse_output_sidebar')) + #content-right-sidebar.overflow-scroll + = render('download_file_tree') + div.enforce-bottom-margin.overflow-auto.d-none id='score_div' #results h2 = t('exercises.implement.results') diff --git a/config/locales/de.yml b/config/locales/de.yml index 1e8305f3..4d1fc6aa 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -385,6 +385,9 @@ de: disclaimer: Bei Fragen zu Deadlines wenden Sie sich bitte an das Teaching Team. Die hier angezeigte Abgabefrist dient nur zur Information und Angaben auf der jeweiligen Kursseite in der E-Learning-Plattform sollen immer Vorrang haben. editor_file_tree: file_root: Dateien + download_file_tree: + file_root: Erstellte Dateien + gone: Die angeforderte Datei konnte nicht abgerufen werden. Erstellte Dateien werden nur kurzzeitig vorgehalten und dann gelöscht. Bitte führen Sie den Code erneut aus und versuchen Sie dann wieder den Download der Datei. import_codeharbor: import_errors: invalid: Fehlerhafte Aufgabe diff --git a/config/locales/en.yml b/config/locales/en.yml index 8deb9f84..62781aab 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -385,6 +385,9 @@ en: disclaimer: If unsure about a deadline, please contact a course instructor. The deadline shown here is only informational and information from the e-learning platform should always take precedence. editor_file_tree: file_root: Files + download_file_tree: + file_root: Generated Files + gone: The requested file could not be retrieved. Generated files are only held for a short time and are then deleted. Please run the code again and then try downloading the file a second time. import_codeharbor: import_errors: invalid: Invalid exercise diff --git a/config/routes.rb b/config/routes.rb index 81781f70..846fa0dd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -153,6 +153,7 @@ Rails.application.routes.draw do member do get 'download', as: :download, action: :download get 'download/:filename', as: :download_file, constraints: {filename: FILENAME_REGEXP}, action: :download_file + get 'download_stream/:filename', as: :download_stream_file, constraints: {filename: FILENAME_REGEXP}, action: :download_submission_file, controller: 'live_streams' get 'render/:filename', as: :render, constraints: {filename: FILENAME_REGEXP}, action: :render_file get 'run/:filename', as: :run, constraints: {filename: FILENAME_REGEXP}, action: :run get :score diff --git a/lib/file_tree.rb b/lib/file_tree.rb index 295852ca..113ef640 100644 --- a/lib/file_tree.rb +++ b/lib/file_tree.rb @@ -58,7 +58,8 @@ class FileTree disabled: !node.leaf?, opened: !node.leaf?, }, - text: node.name, + text: name(node), + download_path: node.content.try(:download_path), } end private :map_to_js_tree @@ -72,6 +73,17 @@ class FileTree end private :node_icon + def name(node) + # We just need any information that is only present in files retrieved from the runner's file system. + # In our case, that is the presence of the `privileged_execution` attribute. + if node.content.is_a?(CodeOcean::File) && !node.content.privileged_execution.nil? + node.content.name_with_extension_and_size + else + node.name + end + end + private :name + def to_js_tree { core: { diff --git a/lib/runner/connection.rb b/lib/runner/connection.rb index 5c6ebc1d..5900eb0a 100644 --- a/lib/runner/connection.rb +++ b/lib/runner/connection.rb @@ -146,7 +146,7 @@ class Runner::Connection @strategy.destroy_at_management @error = Runner::Error::ExecutionTimeout.new('Execution exceeded its time limit') when :terminated_by_codeocean, :terminated_by_management - @exit_callback.call @exit_code + @exit_callback.call @exit_code, @strategy.retrieve_files when :terminated_by_client, :error @strategy.destroy_at_management else # :established diff --git a/lib/runner/strategy.rb b/lib/runner/strategy.rb index b0e1a42d..08fa8de6 100644 --- a/lib/runner/strategy.rb +++ b/lib/runner/strategy.rb @@ -33,6 +33,14 @@ class Runner::Strategy raise NotImplementedError end + def retrieve_files(_path:, _recursive:, privileged_execution:) + raise NotImplementedError + end + + def download_file(_file, privileged_execution:, &_block) + raise NotImplementedError + end + def attach_to_execution(_command, _event_loop, _starting_time, privileged_execution:) raise NotImplementedError end diff --git a/lib/runner/strategy/docker_container_pool.rb b/lib/runner/strategy/docker_container_pool.rb index 8cb41a47..857993eb 100644 --- a/lib/runner/strategy/docker_container_pool.rb +++ b/lib/runner/strategy/docker_container_pool.rb @@ -104,6 +104,10 @@ class Runner::Strategy::DockerContainerPool < Runner::Strategy Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Finished copying files" } end + def retrieve_files(_path:, _recursive:, privileged_execution:) + # The DockerContainerPool does not support retrieving files from the runner. + end + def attach_to_execution(command, event_loop, starting_time, privileged_execution: false) # rubocop:disable Lint/UnusedMethodArgument for the keyword argument reset_inactivity_timer diff --git a/lib/runner/strategy/null.rb b/lib/runner/strategy/null.rb index 242eebfd..c678d60d 100644 --- a/lib/runner/strategy/null.rb +++ b/lib/runner/strategy/null.rb @@ -25,6 +25,12 @@ class Runner::Strategy::Null < Runner::Strategy def copy_files(_files); end + def retrieve_files(_path:, _recursive:, privileged_execution: false); end + + def download_file(_file, privileged_execution: false, &_block) # rubocop:disable Lint/UnusedMethodArgument for the keyword argument + raise Runner::Error.new + end + def attach_to_execution(command, event_loop, starting_time, privileged_execution: false) # rubocop:disable Lint/UnusedMethodArgument for the keyword argument socket = Connection.new(nil, self, event_loop) # We don't want to return an error if the execution environment is changed diff --git a/lib/runner/strategy/poseidon.rb b/lib/runner/strategy/poseidon.rb index 3c8626f0..88e2568b 100644 --- a/lib/runner/strategy/poseidon.rb +++ b/lib/runner/strategy/poseidon.rb @@ -132,6 +132,64 @@ class Runner::Strategy::Poseidon < Runner::Strategy Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Finished copying files" } end + def retrieve_files(path: './', recursive: true, privileged_execution: false) + url = "#{runner_url}/files" + params = { + path: path, + recursive: recursive, + privilegedExecution: privileged_execution || @execution_environment.privileged_execution, + } + Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Retrieving files at #{runner_url} with #{params}" } + response = self.class.http_connection.get url, params + case response.status + when 200 + JSON.parse(response.body) + when 424 + raise Runner::Error::WorkspaceError.new("The path #{path} is not available or could not be read.") + else + self.class.handle_error response + end + rescue Faraday::Error => e + raise Runner::Error::FaradayError.new("Request to Poseidon failed: #{e.inspect}") + ensure + Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Finished listing files" } + end + + def download_file(file, privileged_execution: false, &block) + url = "#{runner_url}/files/raw" + params = { + path: file, + privilegedExecution: privileged_execution || @execution_environment.privileged_execution, + } + Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Download file #{params} from #{runner_url}" } + response = self.class.new_http_connection.get url, params do |request| + content_length = nil + content_type = nil + next if block.blank? + + request.options.on_data = proc do |chunk, _overall_received_bytes, env| + next unless env.success? + + content_length ||= env.response_headers['Content-Length'].presence&.to_i + content_type ||= env.response_headers['Content-Type'].presence || 'application/octet-stream' + yield chunk, content_length, content_type + end + request.options + end + case response.status + when 200 + response.body + when 424 + raise Runner::Error::WorkspaceError.new("The file #{file} is not available or could not be read.") + else + self.class.handle_error response + end + rescue Faraday::Error => e + raise Runner::Error::FaradayError.new("Request to Poseidon failed: #{e.inspect}") + ensure + Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Finished downloading file" } + end + def attach_to_execution(command, event_loop, starting_time, privileged_execution: false) websocket_url = execute_command(command, privileged_execution: privileged_execution) socket = Connection.new(websocket_url, self, event_loop) @@ -232,6 +290,12 @@ class Runner::Strategy::Poseidon < Runner::Strategy end end + def self.new_http_connection + Faraday.new(ssl: {ca_file: config[:ca_file]}, headers: headers) do |faraday| + faraday.adapter :net_http + end + end + def self.parse(response) JSON.parse(response.body).deep_symbolize_keys rescue JSON::ParserError => e