diff --git a/app/assets/javascripts/editor/evaluation.js b/app/assets/javascripts/editor/evaluation.js index 0be38dc0..2a992c08 100644 --- a/app/assets/javascripts/editor/evaluation.js +++ b/app/assets/javascripts/editor/evaluation.js @@ -14,11 +14,10 @@ CodeOceanEditorEvaluation = { this.stopCode(event); this.clearScoringOutput(); $('#submit').addClass("d-none"); - this.createSubmission(cause, null, function (response) { + this.createSubmission(cause, null, function (submission) { this.showSpinner($('#assess')); $('#score_div').removeClass('d-none'); - var url = response.score_url; - this.initializeSocketForScoring(url); + this.initializeSocketForScoring(submission.id); }.bind(this)); }, diff --git a/app/assets/javascripts/editor/execution.js b/app/assets/javascripts/editor/execution.js index c4248ae4..0d4287c8 100644 --- a/app/assets/javascripts/editor/execution.js +++ b/app/assets/javascripts/editor/execution.js @@ -1,56 +1,59 @@ CodeOceanEditorWebsocket = { websocket: null, + // Replace `http` with `ws` for the WebSocket connection. This also works with `https` and `wss`. + webSocketProtocol: window.location.protocol.replace(/^http/, 'ws').split(':')[0], - createSocketUrl: function(url, span) { - const sockURL = new URL(url, window.location); - // not needed any longer, we put it directly into the url: sockURL.pathname = url; + initializeSocket: function(urlHelper, params, closeCallback) { + // 1. Specify the protocol for all URLs to generate + params.protocol = this.webSocketProtocol; + params._options = true; - // replace `http` with `ws` for the WebSocket connection. This also works with `https` and `wss`. - sockURL.protocol = sockURL.protocol.replace("http", "ws"); + // 2. Create a new Sentry transaction. + // Since we want to group similar URLs, we use the URL without the ID and filename as the description. + const cleanedUrl = urlHelper({ + ...params, + ...(params.id && {id: '*'}), // Overwrite the ID with a wildcard only if it is present. + ...(params.filename && {filename: '*'}), // Overwrite the filename with a wildcard only if it is present. + }); + const sentryDescription = `WebSocket ${cleanedUrl}`; + const span = this.sentryTransaction?.startChild({op: 'websocket.client', description: sentryDescription, data: {...params}}) - // strip anchor if it is in the url - sockURL.hash = ''; - - if (span) { - const dynamicContext = this.sentryTransaction.getDynamicSamplingContext(); - const baggage = SentryUtils.dynamicSamplingContextToSentryBaggageHeader(dynamicContext); - if (baggage) { - sockURL.searchParams.set('HTTP_SENTRY_TRACE', span.toTraceparent()); - sockURL.searchParams.set('HTTP_BAGGAGE', baggage); - } + // 3. Create the actual WebSocket URL. + // This URL might contain Sentry Tracing headers to propagate the Sentry transaction. + if (span) { + const dynamicContext = this.sentryTransaction.getDynamicSamplingContext(); + const baggage = SentryUtils.dynamicSamplingContextToSentryBaggageHeader(dynamicContext); + if (baggage) { + params.HTTP_SENTRY_TRACE = span.toTraceparent(); + params.HTTP_BAGGAGE = baggage; } + } + const url = urlHelper({...params}); - return sockURL.toString(); - }, - - initializeSocket: function(url, closeCallback) { - const cleanedPath = url.replace(/\/\d+\//, '/*/').replace(/\/[^\/]+$/, '/*'); - const websocketHost = window.location.origin.replace(/^http/, 'ws'); - const sentryDescription = `WebSocket ${websocketHost}${cleanedPath}`; - const span = this.sentryTransaction?.startChild({op: 'websocket.client', description: sentryDescription}) - this.websocket = new CommandSocket(this.createSocketUrl(url, span), + // 4. Connect to the given URL. + this.websocket = new CommandSocket(url, function (evt) { this.resetOutputTab(); }.bind(this) ); CodeOceanEditorWebsocket.websocket = this.websocket; this.websocket.onError(this.showWebsocketError.bind(this)); - this.websocket.onClose( function(span, callback){ - span?.finish() + this.websocket.onClose(function(span, callback){ + span?.finish(); if(callback != null){ callback(); } }.bind(this, span, closeCallback)); }, - initializeSocketForTesting: function(url) { - this.initializeSocket(url); + initializeSocketForTesting: function(submissionID, filename) { + this.initializeSocket(Routes.test_submission_url, {id: submissionID, filename: filename}); this.websocket.on('default',this.handleTestResponse.bind(this)); this.websocket.on('exit', this.handleExitCommand.bind(this)); }, - initializeSocketForScoring: function(url) { - this.initializeSocket(url, function() { + initializeSocketForScoring: function(submissionID) { + this.initializeSocket(Routes.score_submission_url, {id: submissionID}, function() { $('#assess').one('click', this.scoreCode.bind(this)) }.bind(this)); this.websocket.on('default',this.handleScoringResponse.bind(this)); @@ -59,8 +62,8 @@ CodeOceanEditorWebsocket = { this.websocket.on('status', this.showStatus.bind(this)); }, - initializeSocketForRunning: function(url) { - this.initializeSocket(url); + initializeSocketForRunning: function(submissionID, filename) { + this.initializeSocket(Routes.run_submission_url, {id: submissionID, filename: filename}); this.websocket.on('input',this.showPrompt.bind(this)); this.websocket.on('write', this.printWebsocketOutput.bind(this)); this.websocket.on('clear', this.clearOutput.bind(this)); diff --git a/app/assets/javascripts/editor/submissions.js b/app/assets/javascripts/editor/submissions.js index e267553c..0e5d92b4 100644 --- a/app/assets/javascripts/editor/submissions.js +++ b/app/assets/javascripts/editor/submissions.js @@ -1,6 +1,4 @@ CodeOceanEditorSubmissions = { - FILENAME_URL_PLACEHOLDER: '{filename}', - AUTOSAVE_INTERVAL: 15 * 1000, autosaveTimer: null, autosaveLabel: "#statusbar #autosave", @@ -31,7 +29,7 @@ CodeOceanEditorSubmissions = { }, dataType: 'json', method: $(initiator).data('http-method') || 'POST', - url: url + '.json' + url: url, }); jqxhr.always(this.hideSpinner.bind(this)); jqxhr.done(this.createSubmissionCallback.bind(this)); @@ -99,12 +97,10 @@ CodeOceanEditorSubmissions = { downloadCode: function(event) { event.preventDefault(); - this.createSubmission('#download', null,function(response) { - var url = response.download_url; - + this.createSubmission('#download', null,function(submission) { // to download just a single file, use the following url - //var url = response.download_file_url.replace(FILENAME_URL_PLACEHOLDER, active_file.filename); - window.location = url; + // window.location = Routes.download_file_submission_url(submission.id, CodeOceanEditor.active_file.filename); + window.location = Routes.download_submission_url(submission.id); }); }, @@ -140,11 +136,11 @@ CodeOceanEditorSubmissions = { this.startSentryTransaction(cause); event.preventDefault(); if (cause.is(':visible')) { - this.createSubmission(cause, null, function (response) { - if (response.render_url === undefined) return; + this.createSubmission(cause, null, function (submission) { + if (submission.render_url === undefined) return; const active_file = CodeOceanEditor.active_file.filename; - const desired_file = response.render_url.filter(hash => hash.filepath === active_file); + const desired_file = submission.render_url.filter(hash => hash.filepath === active_file); const url = desired_file[0].url; // Allow to open the new tab even in Safari. // See: https://stackoverflow.com/a/70463940 @@ -156,7 +152,7 @@ CodeOceanEditorSubmissions = { this.printOutput({ stderr: message }, true, 0); - this.sendError(message, response.id); + this.sendError(message, submission.id); this.showOutputBar(); }; } @@ -184,8 +180,7 @@ CodeOceanEditorSubmissions = { this.showSpinner($('#run')); $('#score_div').addClass('d-none'); this.toggleButtonStates(); - const url = submission.run_url.replace(this.FILENAME_URL_PLACEHOLDER, CodeOceanEditor.active_file.filename.replace(/#$/,'')); // remove # if it is the last character, this is not part of the filename and just an anchor - this.initializeSocketForRunning(url); + this.initializeSocketForRunning(submission.id, CodeOceanEditor.active_file.filename); }, testCode: function(event) { @@ -193,11 +188,10 @@ CodeOceanEditorSubmissions = { this.startSentryTransaction(cause); event.preventDefault(); if (cause.is(':visible')) { - this.createSubmission(cause, null, function(response) { + this.createSubmission(cause, null, function(submission) { this.showSpinner($('#test')); $('#score_div').addClass('d-none'); - var url = response.test_url.replace(this.FILENAME_URL_PLACEHOLDER, CodeOceanEditor.active_file.filename.replace(/#$/,'')); // remove # if it is the last character, this is not part of the filename and just an anchor - this.initializeSocketForTesting(url); + this.initializeSocketForTesting(submission.id, CodeOceanEditor.active_file.filename); }.bind(this)); } }, diff --git a/app/assets/javascripts/exercises.js.erb b/app/assets/javascripts/exercises.js.erb index dee4a82e..ab0f14a0 100644 --- a/app/assets/javascripts/exercises.js.erb +++ b/app/assets/javascripts/exercises.js.erb @@ -104,9 +104,9 @@ $(document).on('turbolinks:load', function () { var jqxhr = $.ajax({ // normal file path (without json) would destroy the context object (the exercise) as well, due to redirection // to the context after the :destroy action. - contentType: 'Application/json', - url: fileUrl + '.json', - method: 'DELETE' + dataType: 'json', + method: 'DELETE', + url: fileUrl, }); jqxhr.done(function () { removeFileForm(fileUrl) diff --git a/app/controllers/live_streams_controller.rb b/app/controllers/live_streams_controller.rb index c51a6075..6a523f17 100644 --- a/app/controllers/live_streams_controller.rb +++ b/app/controllers/live_streams_controller.rb @@ -22,7 +22,7 @@ class LiveStreamsController < ApplicationController def download_arbitrary_file @execution_environment = authorize ExecutionEnvironment.find(params[:id]) - desired_file = params[:filename].to_s + desired_file = "/#{params[:filename]}" # The filename given is absolute; this is an admin-only action. runner = Runner.for(current_user, @execution_environment) fallback_location = shell_execution_environment_path(@execution_environment) privileged = params[:sudo] || @execution_environment.privileged_execution? diff --git a/app/models/submission.rb b/app/models/submission.rb index 75565104..00d06612 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -7,7 +7,6 @@ class Submission < ApplicationRecord CAUSES = %w[assess download file render run save submit test autosave requestComments remoteAssess remoteSubmit].freeze - FILENAME_URL_PLACEHOLDER = '{filename}' MAX_COMMENTS_ON_RECOMMENDED_RFC = 5 OLDEST_RFC_TO_SHOW = 1.month diff --git a/app/views/submissions/show.json.jbuilder b/app/views/submissions/show.json.jbuilder index d281d0f2..4ad8d9fb 100644 --- a/app/views/submissions/show.json.jbuilder +++ b/app/views/submissions/show.json.jbuilder @@ -4,10 +4,6 @@ json.id @submission.id json.files @submission.files do |file| json.extract! file, :id, :file_id end -json.download_url download_submission_path(@submission, format: :json) -json.score_url score_submission_path(@submission, format: :json) -json.download_file_url download_file_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, - '{filename}.json') unless @embed_options[:disable_download] json.render_url @submission.collect_files.select(&:visible) do |files| host = ApplicationController::RENDER_HOST || request.host @@ -17,6 +13,3 @@ unless @embed_options[:disable_download] json.url AuthenticatedUrlHelper.sign(url, @submission) end end -json.run_url run_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json') -json.test_url test_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json') -json.finalize_url finalize_submission_path(@submission) diff --git a/config/routes.rb b/config/routes.rb index f2d4477a..c880ac96 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -FILENAME_REGEXP = /.+/ unless Kernel.const_defined?(:FILENAME_REGEXP) - Rails.application.routes.draw do resources :community_solutions, only: %i[index edit update] resources :error_template_attributes @@ -67,7 +65,7 @@ Rails.application.routes.draw do get :shell post 'shell', as: :execute_command, action: :execute_command get :list_files, as: :list_files_in - get 'download/:filename', as: :download_file_from, constraints: {filename: FILENAME_REGEXP}, action: :download_arbitrary_file, controller: 'live_streams' + get 'download/*filename', as: :download_file_from, action: :download_arbitrary_file, controller: 'live_streams', format: false # Admin file-system access to runners get :statistics post :sync_to_runner_management end @@ -131,8 +129,8 @@ Rails.application.routes.draw do namespace :code_ocean do resources :files, only: %i[create destroy] end - get '/uploads/files/:id/:filename', to: 'code_ocean/files#show_protected_upload', as: :protected_upload, constraints: {filename: FILENAME_REGEXP} - get '/uploads/render_files/:id/:filename', to: 'code_ocean/files#render_protected_upload', as: :render_protected_upload, constraints: {filename: FILENAME_REGEXP} + get '/uploads/files/:id/*filename', to: 'code_ocean/files#show_protected_upload', as: :protected_upload, format: false # View file, e.g., when implementing or viewing an exercise + get '/uploads/render_files/:id/*filename', to: 'code_ocean/files#render_protected_upload', as: :render_protected_upload, format: false # Render action with embedded files, i.e., images in user-created HTML resources :file_types @@ -154,14 +152,14 @@ Rails.application.routes.draw do resources :submissions, only: %i[create index show] 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 'download', as: :download, action: :download # Full submission download with RemoteEvaluationMapping + get 'download/*filename', as: :download_file, action: :download_file, format: false # Single file download, currently not used in the frontend (but working) + get 'download_stream/*filename', as: :download_stream_file, action: :download_submission_file, controller: 'live_streams', format: false # Access runner artifacts + get 'render/*filename', as: :render, action: :render_file, format: false + get 'run/*filename', as: :run, action: :run, format: false get :score get :statistics - get 'test/:filename', as: :test, constraints: {filename: FILENAME_REGEXP}, action: :test + get 'test/*filename', as: :test, action: :test, format: false get :finalize end end diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 52ce2d6f..37f07c39 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -279,19 +279,9 @@ RSpec.describe SubmissionsController do expect_assigns(submission: :submission) expect_http_status(:ok) - %i[run test].each do |action| - describe "##{action}_url" do - let(:url) { response.parsed_body.with_indifferent_access.fetch("#{action}_url") } - - it "starts like the #{action} path" do - filename = File.basename(__FILE__) - expect(url).to start_with(Rails.application.routes.url_helpers.send(:"#{action}_submission_path", submission, filename).sub(filename, '')) - end - - it 'ends with a placeholder' do - expect(url).to end_with("#{Submission::FILENAME_URL_PLACEHOLDER}.json") - end - end + it 'includes the desired fields' do + expect(response.parsed_body.keys).to include('id', 'files') + expect(response.parsed_body['files'].first.keys).to include('id', 'file_id') end describe '#render_url' do @@ -307,14 +297,6 @@ RSpec.describe SubmissionsController do expect(url).to include '?token=' end end - - describe '#score_url' do - let(:url) { response.parsed_body.with_indifferent_access.fetch('score_url') } - - it 'corresponds to the score path' do - expect(url).to eq(Rails.application.routes.url_helpers.score_submission_path(submission, format: :json)) - end - end end describe 'GET #test' do