htwkmooc/codeocean
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change syntax for routes with filename

Browse Source 
Previously, the filename was URL-encoded, thus each / was replaced with %2F. This caused issues with some Apache2 configuration, smartly mingling with the URL to either encode it a second time (resulting in %252F) or decoding it (generating a real /). However, for authenticated file downloads with the JWT, we hardly require a byte-by-byte matching. With these changes, the URL parameter is no longer URL-encoded, so that Apache2 won't break our implementation any longer.

Further, we use this opportunity to get rid of the unnecessary .json extension for those filename routes, simplifying the routes generated and doing some further cleanup.
This commit is contained in:
Sebastian Serth
2023-12-19 09:36:15 +01:00
committed by Sebastian Serth
parent 65212c4b4b
commit 17dd8b1267
9 changed files with 64 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
app/assets/javascripts/editor/evaluation.js
View File

@ -14,11 +14,10 @@ CodeOceanEditorEvaluation = {
this.stopCode(event); this.stopCode(event);
this.clearScoringOutput(); this.clearScoringOutput();
$('#submit').addClass("d-none"); $('#submit').addClass("d-none");
this.createSubmission(cause, null, function (response) { this.createSubmission(cause, null, function (submission) {
this.showSpinner($('#assess')); this.showSpinner($('#assess'));
$('#score_div').removeClass('d-none'); $('#score_div').removeClass('d-none');
var url = response.score_url; this.initializeSocketForScoring(submission.id);
this.initializeSocketForScoring(url);
}.bind(this)); }.bind(this));
}, },

67
app/assets/javascripts/editor/execution.js
View File

@ -1,56 +1,59 @@
CodeOceanEditorWebsocket = { CodeOceanEditorWebsocket = {
websocket: null, 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) { initializeSocket: function(urlHelper, params, closeCallback) {
const sockURL = new URL(url, window.location); // 1. Specify the protocol for all URLs to generate
// not needed any longer, we put it directly into the url: sockURL.pathname = url; params.protocol = this.webSocketProtocol;
params._options = true;
// replace `http` with `ws` for the WebSocket connection. This also works with `https` and `wss`. // 2. Create a new Sentry transaction.
sockURL.protocol = sockURL.protocol.replace("http", "ws"); // 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 // 3. Create the actual WebSocket URL.
sockURL.hash = ''; // This URL might contain Sentry Tracing headers to propagate the Sentry transaction.
if (span) {
if (span) { const dynamicContext = this.sentryTransaction.getDynamicSamplingContext();
const dynamicContext = this.sentryTransaction.getDynamicSamplingContext(); const baggage = SentryUtils.dynamicSamplingContextToSentryBaggageHeader(dynamicContext);
const baggage = SentryUtils.dynamicSamplingContextToSentryBaggageHeader(dynamicContext); if (baggage) {
if (baggage) { params.HTTP_SENTRY_TRACE = span.toTraceparent();
sockURL.searchParams.set('HTTP_SENTRY_TRACE', span.toTraceparent()); params.HTTP_BAGGAGE = baggage;
sockURL.searchParams.set('HTTP_BAGGAGE', baggage);
}
} }
}
const url = urlHelper({...params});
return sockURL.toString(); // 4. Connect to the given URL.
}, this.websocket = new CommandSocket(url,
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),
function (evt) { function (evt) {
this.resetOutputTab(); this.resetOutputTab();
}.bind(this) }.bind(this)
); );
CodeOceanEditorWebsocket.websocket = this.websocket; CodeOceanEditorWebsocket.websocket = this.websocket;
this.websocket.onError(this.showWebsocketError.bind(this)); this.websocket.onError(this.showWebsocketError.bind(this));
this.websocket.onClose( function(span, callback){ this.websocket.onClose(function(span, callback){
span?.finish() span?.finish();
if(callback != null){ if(callback != null){
callback(); callback();
} }
}.bind(this, span, closeCallback)); }.bind(this, span, closeCallback));
}, },
initializeSocketForTesting: function(url) { initializeSocketForTesting: function(submissionID, filename) {
this.initializeSocket(url); this.initializeSocket(Routes.test_submission_url, {id: submissionID, filename: filename});
this.websocket.on('default',this.handleTestResponse.bind(this)); this.websocket.on('default',this.handleTestResponse.bind(this));
this.websocket.on('exit', this.handleExitCommand.bind(this)); this.websocket.on('exit', this.handleExitCommand.bind(this));
}, },
initializeSocketForScoring: function(url) { initializeSocketForScoring: function(submissionID) {
this.initializeSocket(url, function() { this.initializeSocket(Routes.score_submission_url, {id: submissionID}, function() {
$('#assess').one('click', this.scoreCode.bind(this)) $('#assess').one('click', this.scoreCode.bind(this))
}.bind(this)); }.bind(this));
this.websocket.on('default',this.handleScoringResponse.bind(this)); this.websocket.on('default',this.handleScoringResponse.bind(this));
@ -59,8 +62,8 @@ CodeOceanEditorWebsocket = {
this.websocket.on('status', this.showStatus.bind(this)); this.websocket.on('status', this.showStatus.bind(this));
}, },
initializeSocketForRunning: function(url) { initializeSocketForRunning: function(submissionID, filename) {
this.initializeSocket(url); this.initializeSocket(Routes.run_submission_url, {id: submissionID, filename: filename});
this.websocket.on('input',this.showPrompt.bind(this)); this.websocket.on('input',this.showPrompt.bind(this));
this.websocket.on('write', this.printWebsocketOutput.bind(this)); this.websocket.on('write', this.printWebsocketOutput.bind(this));
this.websocket.on('clear', this.clearOutput.bind(this)); this.websocket.on('clear', this.clearOutput.bind(this));

28
app/assets/javascripts/editor/submissions.js
View File

@ -1,6 +1,4 @@
CodeOceanEditorSubmissions = { CodeOceanEditorSubmissions = {
FILENAME_URL_PLACEHOLDER: '{filename}',
AUTOSAVE_INTERVAL: 15 * 1000, AUTOSAVE_INTERVAL: 15 * 1000,
autosaveTimer: null, autosaveTimer: null,
autosaveLabel: "#statusbar #autosave", autosaveLabel: "#statusbar #autosave",
@ -31,7 +29,7 @@ CodeOceanEditorSubmissions = {
}, },
dataType: 'json', dataType: 'json',
method: $(initiator).data('http-method') || 'POST', method: $(initiator).data('http-method') || 'POST',
url: url + '.json' url: url,
}); });
jqxhr.always(this.hideSpinner.bind(this)); jqxhr.always(this.hideSpinner.bind(this));
jqxhr.done(this.createSubmissionCallback.bind(this)); jqxhr.done(this.createSubmissionCallback.bind(this));
@ -99,12 +97,10 @@ CodeOceanEditorSubmissions = {
downloadCode: function(event) { downloadCode: function(event) {
event.preventDefault(); event.preventDefault();
this.createSubmission('#download', null,function(response) { this.createSubmission('#download', null,function(submission) {
var url = response.download_url;
// to download just a single file, use the following url // 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 = Routes.download_file_submission_url(submission.id, CodeOceanEditor.active_file.filename);
window.location = url; window.location = Routes.download_submission_url(submission.id);
}); });
}, },
@ -140,11 +136,11 @@ CodeOceanEditorSubmissions = {
this.startSentryTransaction(cause); this.startSentryTransaction(cause);
event.preventDefault(); event.preventDefault();
if (cause.is(':visible')) { if (cause.is(':visible')) {
this.createSubmission(cause, null, function (response) { this.createSubmission(cause, null, function (submission) {
if (response.render_url === undefined) return; if (submission.render_url === undefined) return;
const active_file = CodeOceanEditor.active_file.filename; 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; const url = desired_file[0].url;
// Allow to open the new tab even in Safari. // Allow to open the new tab even in Safari.
// See: https://stackoverflow.com/a/70463940 // See: https://stackoverflow.com/a/70463940
@ -156,7 +152,7 @@ CodeOceanEditorSubmissions = {
this.printOutput({ this.printOutput({
stderr: message stderr: message
}, true, 0); }, true, 0);
this.sendError(message, response.id); this.sendError(message, submission.id);
this.showOutputBar(); this.showOutputBar();
}; };
} }
@ -184,8 +180,7 @@ CodeOceanEditorSubmissions = {
this.showSpinner($('#run')); this.showSpinner($('#run'));
$('#score_div').addClass('d-none'); $('#score_div').addClass('d-none');
this.toggleButtonStates(); 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(submission.id, CodeOceanEditor.active_file.filename);
this.initializeSocketForRunning(url);
}, },
testCode: function(event) { testCode: function(event) {
@ -193,11 +188,10 @@ CodeOceanEditorSubmissions = {
this.startSentryTransaction(cause); this.startSentryTransaction(cause);
event.preventDefault(); event.preventDefault();
if (cause.is(':visible')) { if (cause.is(':visible')) {
this.createSubmission(cause, null, function(response) { this.createSubmission(cause, null, function(submission) {
this.showSpinner($('#test')); this.showSpinner($('#test'));
$('#score_div').addClass('d-none'); $('#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(submission.id, CodeOceanEditor.active_file.filename);
this.initializeSocketForTesting(url);
}.bind(this)); }.bind(this));
} }
}, },

6
app/assets/javascripts/exercises.js.erb
View File

@ -104,9 +104,9 @@ $(document).on('turbolinks:load', function () {
var jqxhr = $.ajax({ var jqxhr = $.ajax({
// normal file path (without json) would destroy the context object (the exercise) as well, due to redirection // normal file path (without json) would destroy the context object (the exercise) as well, due to redirection
// to the context after the :destroy action. // to the context after the :destroy action.
contentType: 'Application/json', dataType: 'json',
url: fileUrl + '.json', method: 'DELETE',
method: 'DELETE' url: fileUrl,
}); });
jqxhr.done(function () { jqxhr.done(function () {
removeFileForm(fileUrl) removeFileForm(fileUrl)

2
app/controllers/live_streams_controller.rb
View File

@ -22,7 +22,7 @@ class LiveStreamsController < ApplicationController
def download_arbitrary_file def download_arbitrary_file
@execution_environment = authorize ExecutionEnvironment.find(params[:id]) @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) runner = Runner.for(current_user, @execution_environment)
fallback_location = shell_execution_environment_path(@execution_environment) fallback_location = shell_execution_environment_path(@execution_environment)
privileged = params[:sudo] || @execution_environment.privileged_execution? privileged = params[:sudo] || @execution_environment.privileged_execution?

1
app/models/submission.rb
View File

@ -7,7 +7,6 @@ class Submission < ApplicationRecord
CAUSES = %w[assess download file render run save submit test autosave requestComments remoteAssess CAUSES = %w[assess download file render run save submit test autosave requestComments remoteAssess
remoteSubmit].freeze remoteSubmit].freeze
FILENAME_URL_PLACEHOLDER = '{filename}'
MAX_COMMENTS_ON_RECOMMENDED_RFC = 5 MAX_COMMENTS_ON_RECOMMENDED_RFC = 5
OLDEST_RFC_TO_SHOW = 1.month OLDEST_RFC_TO_SHOW = 1.month

7
app/views/submissions/show.json.jbuilder
View File

@ -4,10 +4,6 @@ json.id @submission.id
json.files @submission.files do |file| json.files @submission.files do |file|
json.extract! file, :id, :file_id json.extract! file, :id, :file_id
end 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] unless @embed_options[:disable_download]
json.render_url @submission.collect_files.select(&:visible) do |files| json.render_url @submission.collect_files.select(&:visible) do |files|
host = ApplicationController::RENDER_HOST || request.host host = ApplicationController::RENDER_HOST || request.host
@ -17,6 +13,3 @@ unless @embed_options[:disable_download]
json.url AuthenticatedUrlHelper.sign(url, @submission) json.url AuthenticatedUrlHelper.sign(url, @submission)
end end
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)

20
config/routes.rb
View File

@ -1,7 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
FILENAME_REGEXP = /.+/ unless Kernel.const_defined?(:FILENAME_REGEXP)
Rails.application.routes.draw do Rails.application.routes.draw do
resources :community_solutions, only: %i[index edit update] resources :community_solutions, only: %i[index edit update]
resources :error_template_attributes resources :error_template_attributes
@ -67,7 +65,7 @@ Rails.application.routes.draw do
get :shell get :shell
post 'shell', as: :execute_command, action: :execute_command post 'shell', as: :execute_command, action: :execute_command
get :list_files, as: :list_files_in 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 get :statistics
post :sync_to_runner_management post :sync_to_runner_management
end end
@ -131,8 +129,8 @@ Rails.application.routes.draw do
namespace :code_ocean do namespace :code_ocean do
resources :files, only: %i[create destroy] resources :files, only: %i[create destroy]
end end
get '/uploads/files/:id/:filename', to: 'code_ocean/files#show_protected_upload', as: :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, constraints: {filename: FILENAME_REGEXP} 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 resources :file_types
@ -154,14 +152,14 @@ Rails.application.routes.draw do
resources :submissions, only: %i[create index show] do resources :submissions, only: %i[create index show] do
member do member do
get 'download', as: :download, action: :download get 'download', as: :download, action: :download # Full submission download with RemoteEvaluationMapping
get 'download/:filename', as: :download_file, constraints: {filename: FILENAME_REGEXP}, action: :download_file 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, constraints: {filename: FILENAME_REGEXP}, action: :download_submission_file, controller: 'live_streams' 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, constraints: {filename: FILENAME_REGEXP}, action: :render_file get 'render/*filename', as: :render, action: :render_file, format: false
get 'run/:filename', as: :run, constraints: {filename: FILENAME_REGEXP}, action: :run get 'run/*filename', as: :run, action: :run, format: false
get :score get :score
get :statistics get :statistics
get 'test/:filename', as: :test, constraints: {filename: FILENAME_REGEXP}, action: :test get 'test/*filename', as: :test, action: :test, format: false
get :finalize get :finalize
end end
end end

24
spec/controllers/submissions_controller_spec.rb
View File

@ -279,19 +279,9 @@ RSpec.describe SubmissionsController do
expect_assigns(submission: :submission) expect_assigns(submission: :submission)
expect_http_status(:ok) expect_http_status(:ok)
%i[run test].each do |action| it 'includes the desired fields' do
describe "##{action}_url" do expect(response.parsed_body.keys).to include('id', 'files')
let(:url) { response.parsed_body.with_indifferent_access.fetch("#{action}_url") } expect(response.parsed_body['files'].first.keys).to include('id', 'file_id')
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
end end
describe '#render_url' do describe '#render_url' do
@ -307,14 +297,6 @@ RSpec.describe SubmissionsController do
expect(url).to include '?token=' expect(url).to include '?token='
end end
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 end
describe 'GET #test' do describe 'GET #test' do