Skip to content

Commit 5708bba

Browse files
link2xtlink2xt
committed
fix(query_row_optional): do not treat rows with NULL as missing rows
Instead of treating NULL type error as absence of the row, handle NULL values with SQL. Previously we sometimes accidentally treated a single column being NULL as the lack of the whole row.
1 parent 46922d4 commit 5708bba

File tree

6 files changed

+67
-20
lines changed

6 files changed

+67
-20
lines changed

‎deltachat-jsonrpc/src/api/types/message.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ pub struct MessageInfo {
640640
error: Option<String>,
641641
rfc724_mid: String,
642642
server_urls: Vec<String>,
643-
hop_info: Option<String>,
643+
hop_info: String,
644644
}
645645

646646
impl MessageInfo {

‎src/chat.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,13 @@ impl ChatId {
10421042
pub(crate) async fn get_timestamp(self, context: &Context) -> Result<Option<i64>> {
10431043
let timestamp = context
10441044
.sql
1045-
.query_get_value("SELECT MAX(timestamp) FROM msgs WHERE chat_id=?", (self,))
1045+
.query_get_value(
1046+
"SELECT MAX(timestamp)
1047+
FROM msgs
1048+
WHERE chat_id=?
1049+
HAVING COUNT(*) > 0",
1050+
(self,),
1051+
)
10461052
.await?;
10471053
Ok(timestamp)
10481054
}
@@ -1251,7 +1257,7 @@ impl ChatId {
12511257
) -> Result<Option<(String, String, String)>> {
12521258
self.parent_query(
12531259
context,
1254-
"rfc724_mid, mime_in_reply_to, mime_references",
1260+
"rfc724_mid, mime_in_reply_to, IFNULL(mime_references, '')",
12551261
state_out_min,
12561262
|row: &rusqlite::Row| {
12571263
let rfc724_mid: String = row.get(0)?;
@@ -1405,7 +1411,10 @@ impl ChatId {
14051411
context
14061412
.sql
14071413
.query_get_value(
1408-
"SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND state!=?",
1414+
"SELECT MAX(timestamp)
1415+
FROM msgs
1416+
WHERE chat_id=? AND state!=?
1417+
HAVING COUNT(*) > 0",
14091418
(self, MessageState::OutDraft),
14101419
)
14111420
.await?
@@ -1421,7 +1430,10 @@ impl ChatId {
14211430
context
14221431
.sql
14231432
.query_get_value(
1424-
"SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND hidden=0 AND state>?",
1433+
"SELECT MAX(timestamp)
1434+
FROM msgs
1435+
WHERE chat_id=? AND hidden=0 AND state>?
1436+
HAVING COUNT(*) > 0",
14251437
(self, MessageState::InFresh),
14261438
)
14271439
.await?
@@ -4345,7 +4357,10 @@ pub async fn add_device_msg_with_importance(
43454357
if let Some(last_msg_time) = context
43464358
.sql
43474359
.query_get_value(
4348-
"SELECT MAX(timestamp) FROM msgs WHERE chat_id=?",
4360+
"SELECT MAX(timestamp)
4361+
FROM msgs
4362+
WHERE chat_id=?
4363+
HAVING COUNT(*) > 0",
43494364
(chat_id,),
43504365
)
43514366
.await?

‎src/ephemeral.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,16 @@ impl ChatId {
176176
pub async fn get_ephemeral_timer(self, context: &Context) -> Result<Timer> {
177177
let timer = context
178178
.sql
179-
.query_get_value("SELECT ephemeral_timer FROM chats WHERE id=?;", (self,))
179+
.query_row(
180+
"SELECT IFNULL(ephemeral_timer, 0) FROM chats WHERE id=?",
181+
(self,),
182+
|row| {
183+
let timer: Timer = row.get(0)?;
184+
Ok(timer)
185+
},
186+
)
180187
.await?;
181-
Ok(timer.unwrap_or_default())
188+
Ok(timer)
182189
}
183190

184191
/// Set ephemeral timer value without sending a message.
@@ -509,7 +516,8 @@ async fn next_delete_device_after_timestamp(context: &Context) -> Result<Option<
509516
FROM msgs
510517
WHERE chat_id > ?
511518
AND chat_id != ?
512-
AND chat_id != ?;
519+
AND chat_id != ?
520+
HAVING count(*) > 0
513521
"#,
514522
(DC_CHAT_ID_TRASH, self_chat_id, device_chat_id),
515523
)
@@ -533,7 +541,8 @@ async fn next_expiration_timestamp(context: &Context) -> Option<i64> {
533541
SELECT min(ephemeral_timestamp)
534542
FROM msgs
535543
WHERE ephemeral_timestamp != 0
536-
AND chat_id != ?;
544+
AND chat_id != ?
545+
HAVING count(*) > 0
537546
"#,
538547
(DC_CHAT_ID_TRASH,), // Trash contains already deleted messages, skip them
539548
)
@@ -1410,4 +1419,14 @@ mod tests {
14101419

14111420
Ok(())
14121421
}
1422+
1423+
/// Tests that `.get_ephemeral_timer()` returns an error for invalid chat ID.
1424+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
1425+
async fn test_get_ephemeral_timer_wrong_chat_id() -> Result<()> {
1426+
let context = TestContext::new().await;
1427+
let chat_id = ChatId::new(12345);
1428+
assert!(chat_id.get_ephemeral_timer(&context).await.is_err());
1429+
1430+
Ok(())
1431+
}
14131432
}

‎src/message.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,11 +219,19 @@ impl MsgId {
219219
}
220220

221221
/// Returns information about hops of a message, used for message info
222-
pub async fn hop_info(self, context: &Context) -> Result<Option<String>> {
223-
context
222+
pub async fn hop_info(self, context: &Context) -> Result<String> {
223+
let hop_info = context
224224
.sql
225-
.query_get_value("SELECT hop_info FROM msgs WHERE id=?", (self,))
226-
.await
225+
.query_row(
226+
"SELECT IFNULL(hop_info, '') FROM msgs WHERE id=?",
227+
(self,),
228+
|row| {
229+
let hop_info: String = row.get(0)?;
230+
Ok(hop_info)
231+
},
232+
)
233+
.await?;
234+
Ok(hop_info)
227235
}
228236

229237
/// Returns detailed message information in a multi-line text form.
@@ -366,7 +374,11 @@ impl MsgId {
366374
let hop_info = self.hop_info(context).await?;
367375

368376
ret += "\n\n";
369-
ret += &hop_info.unwrap_or_else(|| "No Hop Info".to_owned());
377+
if hop_info.is_empty() {
378+
ret += "No Hop Info";
379+
} else {
380+
ret += &hop_info;
381+
}
370382

371383
Ok(ret)
372384
}
@@ -1998,7 +2010,9 @@ pub(crate) async fn rfc724_mid_exists_ex(
19982010
.query_row_optional(
19992011
&("SELECT id, timestamp_sent, MIN(".to_string()
20002012
+ expr
2001-
+ ") FROM msgs WHERE rfc724_mid=? ORDER BY timestamp_sent DESC"),
2013+
+ ") FROM msgs WHERE rfc724_mid=?
2014+
HAVING COUNT(*) > 0 -- Prevent MIN(expr) from returning NULL when there are no rows.
2015+
ORDER BY timestamp_sent DESC"),
20022016
(rfc724_mid,),
20032017
|row| {
20042018
let msg_id: MsgId = row.get(0)?;

‎src/mimefactory.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ impl MimeFactory {
201201
let (in_reply_to, references) = context
202202
.sql
203203
.query_row(
204-
"SELECT mime_in_reply_to, mime_references FROM msgs WHERE id=?",
204+
"SELECT mime_in_reply_to, IFNULL(mime_references, '')
205+
FROM msgs WHERE id=?",
205206
(msg.id,),
206207
|row| {
207208
let in_reply_to: String = row.get(0)?;

‎src/sql.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -558,15 +558,13 @@ impl Sql {
558558
self.call(move |conn| match conn.query_row(sql.as_ref(), params, f) {
559559
Ok(res) => Ok(Some(res)),
560560
Err(rusqlite::Error::QueryReturnedNoRows) => Ok(None),
561-
Err(rusqlite::Error::InvalidColumnType(_, _, rusqlite::types::Type::Null)) => Ok(None),
562561
Err(err) => Err(err.into()),
563562
})
564563
.await
565564
}
566565

567566
/// Executes a query which is expected to return one row and one
568-
/// column. If the query does not return a value or returns SQL
569-
/// `NULL`, returns `Ok(None)`.
567+
/// column. If the query does not return any rows, returns `Ok(None)`.
570568
pub async fn query_get_value<T>(
571569
&self,
572570
query: &str,

0 commit comments

Comments
 (0)