From 1500f3de5431dfae15c3afb58fbb62d3c43098cc Mon Sep 17 00:00:00 2001
From: Robert Czechowski <robert@code-intelligence.com>
Date: Fri, 20 May 2022 20:41:03 +0200
Subject: [PATCH] Handle error case of unknown {contest|task} id gracefully
instead of panicking
---
src/core.rs | 27 ++++++++++++++-------------
src/db_conn.base.rs | 33 ++++++++++++++++++---------------
src/db_conn.rs | 10 +++++-----
src/db_conn_postgres.rs | 32 +++++++++++++++++---------------
src/db_conn_sqlite_new.rs | 32 +++++++++++++++++---------------
src/webfw_iron.rs | 5 ++++-
templates/jwinf/index.hbs | 2 +-
7 files changed, 76 insertions(+), 65 deletions(-)
diff --git a/src/core.rs b/src/core.rs
index 60454e54..4d048b2a 100644
--- a/src/core.rs
+++ b/src/core.rs
@@ -59,6 +59,7 @@ pub enum MedalError {
UnmatchedPasswords,
NotFound,
AccountIncomplete,
+ UnknownId,
OauthError(String),
}
@@ -413,7 +414,7 @@ pub fn show_contest<T: MedalConnection>(conn: &T, contest_id: i32, session_token
return Err(MedalError::AccountIncomplete);
}
- let contest = conn.get_contest_by_id_complete(contest_id);
+ let contest = conn.get_contest_by_id_complete(contest_id).ok_or(MedalError::UnknownId)?;
let grades = conn.get_contest_user_grades(&session_token, contest_id);
let mut opt_part = conn.get_participation(session.id, contest_id);
@@ -613,7 +614,7 @@ pub fn show_contest_results<T: MedalConnection>(conn: &T, contest_id: i32, sessi
data.insert("taskname".to_string(), to_json(&tasknames));
data.insert("result".to_string(), to_json(&results));
- let c = conn.get_contest_by_id(contest_id);
+ let c = conn.get_contest_by_id(contest_id).ok_or(MedalError::UnknownId)?;
let ci = ContestInfo { id: c.id.unwrap(), name: c.name.clone(), duration: c.duration, public: c.public };
data.insert("contest".to_string(), to_json(&ci));
@@ -627,7 +628,7 @@ pub fn start_contest<T: MedalConnection>(conn: &T, contest_id: i32, session_toke
-> MedalResult<()> {
// TODO: Is _or_new the right semantic? We need a CSRF token anyway …
let session = conn.get_session_or_new(&session_token).unwrap();
- let contest = conn.get_contest_by_id(contest_id);
+ let contest = conn.get_contest_by_id(contest_id).ok_or(MedalError::UnknownId)?;
// Check logged in or open contest
if contest.duration != 0
@@ -761,7 +762,7 @@ pub fn save_submission<T: MedalConnection>(conn: &T, task_id: i32, session_token
return Err(MedalError::CsrfCheckFailed);
}
- let (t, _, contest) = conn.get_task_by_id_complete(task_id);
+ let (t, _, contest) = conn.get_task_by_id_complete(task_id).ok_or(MedalError::UnknownId)?;
match conn.get_participation(session.id, contest.id.expect("Value from database")) {
None => return Err(MedalError::AccessDenied),
@@ -826,12 +827,12 @@ pub fn save_submission<T: MedalConnection>(conn: &T, task_id: i32, session_token
}
pub fn show_task<T: MedalConnection>(conn: &T, task_id: i32, session_token: &str, autosaveinterval: u64)
- -> Result<MedalValue, i32> {
+ -> MedalResult<Result<MedalValue, i32>> {
let session = conn.get_session_or_new(&session_token).unwrap();
- let (t, tg, contest) = conn.get_task_by_id_complete(task_id);
+ let (t, tg, contest) = conn.get_task_by_id_complete(task_id).ok_or(MedalError::UnknownId)?;
let grade = conn.get_taskgroup_user_grade(&session_token, tg.id.unwrap()); // TODO: Unwrap?
- let tasklist = conn.get_contest_by_id_complete(contest.id.unwrap()); // TODO: Unwrap?
+ let tasklist = conn.get_contest_by_id_complete(contest.id.unwrap()).ok_or(MedalError::UnknownId)?; // TODO: Unwrap?
let mut prevtaskgroup: Option<Taskgroup> = None;
let mut nexttaskgroup: Option<Taskgroup> = None;
@@ -854,7 +855,7 @@ pub fn show_task<T: MedalConnection>(conn: &T, task_id: i32, session_token: &str
}
match conn.get_own_participation(&session_token, contest.id.expect("Value from database")) {
- None => Err(contest.id.unwrap()),
+ None => Ok(Err(contest.id.unwrap())),
Some(participation) => {
let mut data = json_val::Map::new();
data.insert("subtasks".to_string(), to_json(&subtaskstars));
@@ -896,10 +897,10 @@ pub fn show_task<T: MedalConnection>(conn: &T, task_id: i32, session_token: &str
let taskpath = format!("{}{}", contest.location, &tasklocation);
data.insert("taskpath".to_string(), to_json(&taskpath));
- Ok((template, data))
+ Ok(Ok((template, data)))
} else {
// Contest over
- Err(contest.id.unwrap())
+ Ok(Err(contest.id.unwrap()))
}
}
}
@@ -1658,7 +1659,7 @@ pub fn admin_show_participation<T: MedalConnection>(conn: &T, user_id: i32, cont
}
}
- let contest = conn.get_contest_by_id_complete(contest_id);
+ let contest = conn.get_contest_by_id_complete(contest_id).ok_or(MedalError::UnknownId)?;
#[rustfmt::skip]
let subms: Vec<(String, Vec<(i32, Vec<(String, i32)>)>)> =
@@ -1721,7 +1722,7 @@ pub fn admin_delete_participation<T: MedalConnection>(conn: &T, user_id: i32, co
let (user, opt_group) = conn.get_user_and_group_by_id(user_id).ok_or(MedalError::AccessDenied)?;
let _part = conn.get_participation(user.id, contest_id).ok_or(MedalError::AccessDenied)?;
- let contest = conn.get_contest_by_id_complete(contest_id);
+ let contest = conn.get_contest_by_id_complete(contest_id).ok_or(MedalError::UnknownId)?;
if !session.is_admin() {
// Check access for teachers
@@ -1771,7 +1772,7 @@ pub fn admin_contest_export<T: MedalConnection>(conn: &T, contest_id: i32, sessi
.ensure_admin()
.ok_or(MedalError::AccessDenied)?;
- let contest = conn.get_contest_by_id_complete(contest_id);
+ let contest = conn.get_contest_by_id_complete(contest_id).ok_or(MedalError::UnknownId)?;
let taskgroup_ids: Vec<(i32, String)> =
contest.taskgroups.into_iter().map(|tg| (tg.id.unwrap(), tg.name)).collect();
diff --git a/src/db_conn.base.rs b/src/db_conn.base.rs
index 94c11e01..01f0bc8e 100644
--- a/src/db_conn.base.rs
+++ b/src/db_conn.base.rs
@@ -1065,7 +1065,7 @@ impl MedalConnection for Connection {
.unwrap()
}
- fn get_contest_by_id(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT location, filename, name, duration, public, start_date, end_date, review_start_date,
review_end_date, min_grade, max_grade, protected, requires_login, requires_contest, secret,
message
@@ -1091,10 +1091,9 @@ impl MedalConnection for Connection {
message: row.get(15),
taskgroups: Vec::new() })
.unwrap()
- .unwrap() // TODO: Should return Option?
}
- fn get_contest_by_id_complete(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id_complete(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
contest.min_grade, contest.max_grade, contest.protected, contest.requires_login,
@@ -1150,7 +1149,7 @@ impl MedalConnection for Connection {
taskgroup.tasks.push(t);
}
contest.taskgroups.push(taskgroup);
- contest
+ Some(contest)
} else {
// If the contest has no tasks, we fall back to the function, that does not try to gather the task
// information
@@ -1158,7 +1157,7 @@ impl MedalConnection for Connection {
}
}
- fn get_contest_by_id_partial(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id_partial(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
contest.min_grade, contest.max_grade, contest.protected, contest.requires_login,
@@ -1198,13 +1197,19 @@ impl MedalConnection for Connection {
.unwrap();
let mut taskgroupcontest_iter = taskgroupcontest.into_iter();
- let (mut contest, taskgroup) = taskgroupcontest_iter.next().unwrap();
- contest.taskgroups.push(taskgroup);
- for tgc in taskgroupcontest_iter {
- let (_, tg) = tgc;
- contest.taskgroups.push(tg);
+ if let Some((mut contest, taskgroup)) = taskgroupcontest_iter.next() {
+ contest.taskgroups.push(taskgroup);
+ for tgc in taskgroupcontest_iter {
+ let (_, tg) = tgc;
+ contest.taskgroups.push(tg);
+ }
+ Some(contest)
+ }
+ else {
+ // If the contest has no tasks, we fall back to the function, that does not try to gather the task
+ // information
+ self.get_contest_by_id(contest_id)
}
- contest
}
fn get_participation(&self, session_id: i32, contest_id: i32) -> Option<Participation> {
@@ -1293,7 +1298,7 @@ impl MedalConnection for Connection {
}
}
}
- fn get_task_by_id(&self, task_id: i32) -> Task {
+ fn get_task_by_id(&self, task_id: i32) -> Option<Task> {
let query = "SELECT location, stars, taskgroup
FROM task
WHERE id = $1";
@@ -1302,9 +1307,8 @@ impl MedalConnection for Connection {
location: row.get(0),
stars: row.get(1) })
.unwrap()
- .unwrap()
}
- fn get_task_by_id_complete(&self, task_id: i32) -> (Task, Taskgroup, Contest) {
+ fn get_task_by_id_complete(&self, task_id: i32) -> Option<(Task, Taskgroup, Contest)> {
let query = "SELECT task.location, task.stars, taskgroup.id, taskgroup.name, taskgroup.active, contest.id,
contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
@@ -1343,7 +1347,6 @@ impl MedalConnection for Connection {
taskgroups: Vec::new() })
})
.unwrap()
- .unwrap()
}
fn get_submission_to_validate(&self, tasklocation: &str, subtask: Option<&str>) -> i32 {
diff --git a/src/db_conn.rs b/src/db_conn.rs
index 6210f324..759849fe 100644
--- a/src/db_conn.rs
+++ b/src/db_conn.rs
@@ -99,15 +99,15 @@ pub trait MedalConnection {
/// Returns the contest identified by `contest_id` without any associated taskgroups. Panics if the contest does not
/// exist.
- fn get_contest_by_id(&self, contest_id: i32) -> Contest;
+ fn get_contest_by_id(&self, contest_id: i32) -> Option<Contest>;
/// Returns the contest identified by `contest_id` with associated taskgroups but without any associated tasks of
/// the taskgroups. Panics if the contest does not exist.
- fn get_contest_by_id_partial(&self, contest_id: i32) -> Contest;
+ fn get_contest_by_id_partial(&self, contest_id: i32) -> Option<Contest>;
/// Returns the contest identified by `contest_id` with associated taskgroups and all associated tasks of the
/// taskgroups. Panics if the contest does not exist.
- fn get_contest_by_id_complete(&self, contest_id: i32) -> Contest;
+ fn get_contest_by_id_complete(&self, contest_id: i32) -> Option<Contest>;
/// Try to get the participation associated to the session id `session_id` and the contest id `contest_id`.
///
@@ -132,8 +132,8 @@ pub trait MedalConnection {
/// Returns an `Result` that either contains the new `Participation` if the checks succeded or no value if the
/// checks failed.
fn new_participation(&self, session: &str, contest_id: i32) -> Result<Participation, ()>;
- fn get_task_by_id(&self, task_id: i32) -> Task;
- fn get_task_by_id_complete(&self, task_id: i32) -> (Task, Taskgroup, Contest);
+ fn get_task_by_id(&self, task_id: i32) -> Option<Task>;
+ fn get_task_by_id_complete(&self, task_id: i32) -> Option<(Task, Taskgroup, Contest)>;
fn get_submission_to_validate(&self, tasklocation: &str, subtask: Option<&str>) -> i32;
fn find_next_submission_to_validate(&self, userid: i32, taskgroupid: i32);
diff --git a/src/db_conn_postgres.rs b/src/db_conn_postgres.rs
index fe6c5ad7..7cf34137 100644
--- a/src/db_conn_postgres.rs
+++ b/src/db_conn_postgres.rs
@@ -1158,7 +1158,7 @@ impl MedalConnection for Connection {
.unwrap()
}
- fn get_contest_by_id(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT location, filename, name, duration, public, start_date, end_date, review_start_date,
review_end_date, min_grade, max_grade, protected, requires_login, requires_contest, secret,
message
@@ -1184,10 +1184,9 @@ impl MedalConnection for Connection {
message: row.get(15),
taskgroups: Vec::new() })
.unwrap()
- .unwrap() // TODO: Should return Option?
}
- fn get_contest_by_id_complete(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id_complete(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
contest.min_grade, contest.max_grade, contest.protected, contest.requires_login,
@@ -1243,7 +1242,7 @@ impl MedalConnection for Connection {
taskgroup.tasks.push(t);
}
contest.taskgroups.push(taskgroup);
- contest
+ Some(contest)
} else {
// If the contest has no tasks, we fall back to the function, that does not try to gather the task
// information
@@ -1251,7 +1250,7 @@ impl MedalConnection for Connection {
}
}
- fn get_contest_by_id_partial(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id_partial(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
contest.min_grade, contest.max_grade, contest.protected, contest.requires_login,
@@ -1291,13 +1290,18 @@ impl MedalConnection for Connection {
.unwrap();
let mut taskgroupcontest_iter = taskgroupcontest.into_iter();
- let (mut contest, taskgroup) = taskgroupcontest_iter.next().unwrap();
- contest.taskgroups.push(taskgroup);
- for tgc in taskgroupcontest_iter {
- let (_, tg) = tgc;
- contest.taskgroups.push(tg);
+ if let Some((mut contest, taskgroup)) = taskgroupcontest_iter.next() {
+ contest.taskgroups.push(taskgroup);
+ for tgc in taskgroupcontest_iter {
+ let (_, tg) = tgc;
+ contest.taskgroups.push(tg);
+ }
+ Some(contest)
+ } else {
+ // If the contest has no tasks, we fall back to the function, that does not try to gather the task
+ // information
+ self.get_contest_by_id(contest_id)
}
- contest
}
fn get_participation(&self, session_id: i32, contest_id: i32) -> Option<Participation> {
@@ -1386,7 +1390,7 @@ impl MedalConnection for Connection {
}
}
}
- fn get_task_by_id(&self, task_id: i32) -> Task {
+ fn get_task_by_id(&self, task_id: i32) -> Option<Task> {
let query = "SELECT location, stars, taskgroup
FROM task
WHERE id = $1";
@@ -1395,9 +1399,8 @@ impl MedalConnection for Connection {
location: row.get(0),
stars: row.get(1) })
.unwrap()
- .unwrap()
}
- fn get_task_by_id_complete(&self, task_id: i32) -> (Task, Taskgroup, Contest) {
+ fn get_task_by_id_complete(&self, task_id: i32) -> Option<(Task, Taskgroup, Contest)> {
let query = "SELECT task.location, task.stars, taskgroup.id, taskgroup.name, taskgroup.active, contest.id,
contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
@@ -1436,7 +1439,6 @@ impl MedalConnection for Connection {
taskgroups: Vec::new() })
})
.unwrap()
- .unwrap()
}
fn get_submission_to_validate(&self, tasklocation: &str, subtask: Option<&str>) -> i32 {
diff --git a/src/db_conn_sqlite_new.rs b/src/db_conn_sqlite_new.rs
index 0ddf8305..4f7ba2e4 100644
--- a/src/db_conn_sqlite_new.rs
+++ b/src/db_conn_sqlite_new.rs
@@ -1158,7 +1158,7 @@ impl MedalConnection for Connection {
.unwrap()
}
- fn get_contest_by_id(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT location, filename, name, duration, public, start_date, end_date, review_start_date,
review_end_date, min_grade, max_grade, protected, requires_login, requires_contest, secret,
message
@@ -1184,10 +1184,9 @@ impl MedalConnection for Connection {
message: row.get(15),
taskgroups: Vec::new() })
.unwrap()
- .unwrap() // TODO: Should return Option?
}
- fn get_contest_by_id_complete(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id_complete(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
contest.min_grade, contest.max_grade, contest.protected, contest.requires_login,
@@ -1243,7 +1242,7 @@ impl MedalConnection for Connection {
taskgroup.tasks.push(t);
}
contest.taskgroups.push(taskgroup);
- contest
+ Some(contest)
} else {
// If the contest has no tasks, we fall back to the function, that does not try to gather the task
// information
@@ -1251,7 +1250,7 @@ impl MedalConnection for Connection {
}
}
- fn get_contest_by_id_partial(&self, contest_id: i32) -> Contest {
+ fn get_contest_by_id_partial(&self, contest_id: i32) -> Option<Contest> {
let query = "SELECT contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
contest.min_grade, contest.max_grade, contest.protected, contest.requires_login,
@@ -1291,13 +1290,18 @@ impl MedalConnection for Connection {
.unwrap();
let mut taskgroupcontest_iter = taskgroupcontest.into_iter();
- let (mut contest, taskgroup) = taskgroupcontest_iter.next().unwrap();
- contest.taskgroups.push(taskgroup);
- for tgc in taskgroupcontest_iter {
- let (_, tg) = tgc;
- contest.taskgroups.push(tg);
+ if let Some((mut contest, taskgroup)) = taskgroupcontest_iter.next() {
+ contest.taskgroups.push(taskgroup);
+ for tgc in taskgroupcontest_iter {
+ let (_, tg) = tgc;
+ contest.taskgroups.push(tg);
+ }
+ Some(contest)
+ } else {
+ // If the contest has no tasks, we fall back to the function, that does not try to gather the task
+ // information
+ self.get_contest_by_id(contest_id)
}
- contest
}
fn get_participation(&self, session_id: i32, contest_id: i32) -> Option<Participation> {
@@ -1386,7 +1390,7 @@ impl MedalConnection for Connection {
}
}
}
- fn get_task_by_id(&self, task_id: i32) -> Task {
+ fn get_task_by_id(&self, task_id: i32) -> Option<Task> {
let query = "SELECT location, stars, taskgroup
FROM task
WHERE id = ?1";
@@ -1395,9 +1399,8 @@ impl MedalConnection for Connection {
location: row.get(0),
stars: row.get(1) })
.unwrap()
- .unwrap()
}
- fn get_task_by_id_complete(&self, task_id: i32) -> (Task, Taskgroup, Contest) {
+ fn get_task_by_id_complete(&self, task_id: i32) -> Option<(Task, Taskgroup, Contest)> {
let query = "SELECT task.location, task.stars, taskgroup.id, taskgroup.name, taskgroup.active, contest.id,
contest.location, contest.filename, contest.name, contest.duration, contest.public,
contest.start_date, contest.end_date, contest.review_start_date, contest.review_end_date,
@@ -1436,7 +1439,6 @@ impl MedalConnection for Connection {
taskgroups: Vec::new() })
})
.unwrap()
- .unwrap()
}
fn get_submission_to_validate(&self, tasklocation: &str, subtask: Option<&str>) -> i32 {
diff --git a/src/webfw_iron.rs b/src/webfw_iron.rs
index 5a668800..50e27f55 100644
--- a/src/webfw_iron.rs
+++ b/src/webfw_iron.rs
@@ -305,6 +305,9 @@ impl<'c, 'a, 'b> From<AugMedalError<'c, 'a, 'b>> for IronError {
core::MedalError::AccessDenied => IronError { error: Box::new(SessionError { message:
"Access denied".to_string() }),
response: Response::with(status::Unauthorized) },
+ core::MedalError::UnknownId => IronError { error: Box::new(SessionError { message:
+ "Not found".to_string() }),
+ response: Response::with(status::NotFound) },
core::MedalError::CsrfCheckFailed => IronError { error: Box::new(SessionError { message:
"CSRF Error".to_string() }),
response: Response::with(status::Forbidden) },
@@ -757,7 +760,7 @@ fn task<C>(req: &mut Request) -> IronResult<Response>
config.auto_save_interval.unwrap_or(10)
};
- match with_conn![core::show_task, C, req, task_id, &session_token, autosaveinterval] {
+ match with_conn![core::show_task, C, req, task_id, &session_token, autosaveinterval].aug(req)? {
Ok((template, data)) => {
let mut resp = Response::new();
resp.set_mut(Template::new(&template, data)).set_mut(status::Ok);
diff --git a/templates/jwinf/index.hbs b/templates/jwinf/index.hbs
index fb4c6d91..04042713 100644
--- a/templates/jwinf/index.hbs
+++ b/templates/jwinf/index.hbs
@@ -30,7 +30,7 @@
{{#if admin}}
<div class="columns alogin">
<div class="column is-8 is-offset-2">
- <a href="/admin/">
+ <a href="/admin">
<div class="notification is-primary">
<h3 class="title is-5">Adminstration</h3>
<p>Administrationsseite öffnen und Benutzer, Gruppen und Teilnahmen verwalten</p>
--
GitLab