From b869aab5d03b0ec606d664939782f42ea57ce624 Mon Sep 17 00:00:00 2001
From: Devin Ragotzy <devin.ragotzy@gmail.com>
Date: Fri, 4 Dec 2020 17:16:29 -0500
Subject: [PATCH] Cleanup removing debug printing and logging, append non state
events
---
src/client_server/backup.rs | 7 +-
src/database/rooms.rs | 4 +-
src/database/sending.rs | 4 +-
src/pdu.rs | 35 ++++++++-
src/ruma_wrapper.rs | 46 ++++++-----
src/server_server.rs | 153 +++++++++++++-----------------------
6 files changed, 119 insertions(+), 130 deletions(-)
diff --git a/src/client_server/backup.rs b/src/client_server/backup.rs
index 676b5a3f..0f34ba78 100644
--- a/src/client_server/backup.rs
+++ b/src/client_server/backup.rs
@@ -256,7 +256,12 @@ pub async fn get_backup_key_session_route(
let key_data = db
.key_backups
.get_session(&sender_user, &body.version, &body.room_id, &body.session_id)?
- .ok_or_else(|| Error::bad_database("Backup key not found for this user's session."))?;
+ .ok_or_else(|| {
+ Error::BadRequest(
+ ErrorKind::NotFound,
+ "Backup key not found for this user's session.",
+ )
+ })?;
Ok(get_backup_key_session::Response { key_data }.into())
}
diff --git a/src/database/rooms.rs b/src/database/rooms.rs
index e7b6eaac..0618fd6e 100644
--- a/src/database/rooms.rs
+++ b/src/database/rooms.rs
@@ -41,7 +41,7 @@ pub type StateHashId = IVec;
///
/// An events parent is any event we are aware of that is part of
/// the events `prev_events` array.
-pub enum ClosestParent {
+pub(crate) enum ClosestParent {
Append,
Insert(u64),
}
@@ -417,7 +417,7 @@ impl Rooms {
/// First we check if the last PDU inserted to the given room is a parent
/// if not we recursively check older `prev_events` to insert the incoming
/// event after.
- pub fn get_closest_parent(
+ pub(crate) fn get_latest_pduid_before(
&self,
room: &RoomId,
incoming_prev_ids: &[EventId],
diff --git a/src/database/sending.rs b/src/database/sending.rs
index 0be14f85..cd88e088 100644
--- a/src/database/sending.rs
+++ b/src/database/sending.rs
@@ -2,7 +2,7 @@ use std::{collections::HashMap, convert::TryFrom, time::SystemTime};
use crate::{server_server, utils, Error, PduEvent, Result};
use federation::transactions::send_transaction_message;
-use log::{debug, error};
+use log::{debug, warn};
use rocket::futures::stream::{FuturesUnordered, StreamExt};
use ruma::{api::federation, ServerName};
use sled::IVec;
@@ -116,7 +116,7 @@ impl Sending {
}
}
Err((server, e)) => {
- error!("server: {}\nerror: {}", server, e)
+ warn!("Couldn't send transaction to {}: {}", server, e)
// TODO: exponential backoff
}
};
diff --git a/src/pdu.rs b/src/pdu.rs
index cffd4a32..e56e81ad 100644
--- a/src/pdu.rs
+++ b/src/pdu.rs
@@ -5,12 +5,17 @@ use ruma::{
pdu::EventHash, room::member::MemberEventContent, AnyEvent, AnyRoomEvent, AnyStateEvent,
AnyStrippedStateEvent, AnySyncRoomEvent, AnySyncStateEvent, EventType, StateEvent,
},
- serde::{CanonicalJsonObject, CanonicalJsonValue},
- EventId, Raw, RoomId, ServerKeyId, ServerName, UserId,
+ serde::{to_canonical_value, CanonicalJsonObject, CanonicalJsonValue},
+ EventId, Raw, RoomId, RoomVersionId, ServerKeyId, ServerName, UserId,
};
use serde::{Deserialize, Serialize};
use serde_json::json;
-use std::{collections::BTreeMap, convert::TryInto, sync::Arc, time::UNIX_EPOCH};
+use std::{
+ collections::BTreeMap,
+ convert::{TryFrom, TryInto},
+ sync::Arc,
+ time::UNIX_EPOCH,
+};
#[derive(Deserialize, Serialize, Debug)]
pub struct PduEvent {
@@ -279,6 +284,30 @@ impl PduEvent {
}
}
+/// Generates a correct eventId for the incoming pdu.
+///
+/// Returns a tuple of the new `EventId` and the PDU with the eventId inserted as a `serde_json::Value`.
+pub(crate) fn process_incoming_pdu(
+ pdu: &ruma::Raw<ruma::events::pdu::Pdu>,
+) -> (EventId, CanonicalJsonObject) {
+ let mut value =
+ serde_json::from_str(pdu.json().get()).expect("A Raw<...> is always valid JSON");
+
+ let event_id = EventId::try_from(&*format!(
+ "${}",
+ ruma::signatures::reference_hash(&value, &RoomVersionId::Version6)
+ .expect("ruma can calculate reference hashes")
+ ))
+ .expect("ruma's reference hashes are valid event ids");
+
+ value.insert(
+ "event_id".to_owned(),
+ to_canonical_value(&event_id).expect("EventId is a valid CanonicalJsonValue"),
+ );
+
+ (event_id, value)
+}
+
/// Build the start of a PDU in order to add it to the `Database`.
#[derive(Debug, Deserialize)]
pub struct PduBuilder {
diff --git a/src/ruma_wrapper.rs b/src/ruma_wrapper.rs
index a68b09d0..4b3d08d5 100644
--- a/src/ruma_wrapper.rs
+++ b/src/ruma_wrapper.rs
@@ -65,31 +65,29 @@ where
.await
.expect("database was loaded");
- let (sender_user, sender_device) =
- // TODO: Do we need to matches! anything else here? ServerSignatures
- match T::METADATA.authentication {
- AuthScheme::AccessToken | AuthScheme::QueryOnlyAccessToken => {
- // Get token from header or query value
- let token = match request
- .headers()
- .get_one("Authorization")
- .map(|s| s[7..].to_owned()) // Split off "Bearer "
- .or_else(|| request.get_query_value("access_token").and_then(|r| r.ok()))
- {
- // TODO: M_MISSING_TOKEN
- None => return Failure((Status::Unauthorized, ())),
- Some(token) => token,
- };
-
- // Check if token is valid
- match db.users.find_from_token(&token).unwrap() {
- // TODO: M_UNKNOWN_TOKEN
- None => return Failure((Status::Unauthorized, ())),
- Some((user_id, device_id)) => (Some(user_id), Some(device_id.into())),
- }
+ let (sender_user, sender_device) = match T::METADATA.authentication {
+ AuthScheme::AccessToken | AuthScheme::QueryOnlyAccessToken => {
+ // Get token from header or query value
+ let token = match request
+ .headers()
+ .get_one("Authorization")
+ .map(|s| s[7..].to_owned()) // Split off "Bearer "
+ .or_else(|| request.get_query_value("access_token").and_then(|r| r.ok()))
+ {
+ // TODO: M_MISSING_TOKEN
+ None => return Failure((Status::Unauthorized, ())),
+ Some(token) => token,
+ };
+
+ // Check if token is valid
+ match db.users.find_from_token(&token).unwrap() {
+ // TODO: M_UNKNOWN_TOKEN
+ None => return Failure((Status::Unauthorized, ())),
+ Some((user_id, device_id)) => (Some(user_id), Some(device_id.into())),
}
- _ => (None, None)
- };
+ }
+ _ => (None, None),
+ };
let mut http_request = http::Request::builder()
.uri(request.uri().to_string())
diff --git a/src/server_server.rs b/src/server_server.rs
index 8d3de1e4..a7f6391d 100644
--- a/src/server_server.rs
+++ b/src/server_server.rs
@@ -20,13 +20,13 @@ use ruma::{
OutgoingRequest,
},
directory::{IncomingFilter, IncomingRoomNetwork},
- serde::{to_canonical_value, CanonicalJsonObject},
- EventId, RoomId, RoomVersionId, ServerName, UserId,
+ EventId, RoomId, ServerName, UserId,
};
use std::{
collections::BTreeMap,
convert::{TryFrom, TryInto},
fmt::Debug,
+ sync::Arc,
time::{Duration, SystemTime},
};
use trust_dns_resolver::AsyncResolver;
@@ -415,8 +415,7 @@ pub async fn send_transaction_message_route<'a>(
"m.receipt" => {}
_ => {}
},
- Err(err) => {
- error!("{}", err);
+ Err(_err) => {
continue;
}
}
@@ -431,19 +430,53 @@ pub async fn send_transaction_message_route<'a>(
// would return a M_BAD_JSON error.
let mut resolved_map = BTreeMap::new();
for pdu in &body.pdus {
- let (event_id, value) = process_incoming_pdu(pdu);
- // TODO: this is an unfortunate conversion dance...
- let pdu = serde_json::from_value::<PduEvent>(serde_json::to_value(&value).expect("msg"))
- .expect("all ruma pdus are conduit pdus");
+ // Ruma/PduEvent/StateEvent satifies - 1. Is a valid event, otherwise it is dropped.
+
+ // state-res checks signatures - 2. Passes signature checks, otherwise event is dropped.
+
+ // 3. Passes hash checks, otherwise it is redacted before being processed further.
+ // TODO: redact event if hashing fails
+ let (event_id, value) = crate::pdu::process_incoming_pdu(pdu);
+
+ let pdu = serde_json::from_value::<PduEvent>(
+ serde_json::to_value(&value).expect("CanonicalJsonObj is a valid JsonValue"),
+ )
+ .expect("all ruma pdus are conduit pdus");
let room_id = &pdu.room_id;
// If we have no idea about this room skip the PDU
if !db.rooms.exists(room_id)? {
- error!("Room does not exist on this server.");
resolved_map.insert(event_id, Err("Room is unknown to this server".into()));
continue;
}
+ // If it is not a state event, we can skip state-res
+ if value.get("state_key").is_none() {
+ if !db.rooms.is_joined(&pdu.sender, room_id)? {
+ warn!("Sender is not joined {}", pdu.kind);
+ resolved_map.insert(event_id, Err("User is not in this room".into()));
+ continue;
+ }
+
+ let count = db.globals.next_count()?;
+ let mut pdu_id = room_id.as_bytes().to_vec();
+ pdu_id.push(0xff);
+ pdu_id.extend_from_slice(&count.to_be_bytes());
+ db.rooms.append_pdu(
+ &pdu,
+ &value,
+ count,
+ pdu_id.into(),
+ &db.globals,
+ &db.account_data,
+ &db.admin,
+ )?;
+
+ resolved_map.insert(event_id, Ok::<(), String>(()));
+ continue;
+ }
+
+ // We have a state event so we need info for state-res
let get_state_response = match send_request(
&db.globals,
body.body.origin.clone(),
@@ -461,7 +494,6 @@ pub async fn send_transaction_message_route<'a>(
// As an example a possible error
// {"errcode":"M_FORBIDDEN","error":"Host not in room."}
Err(err) => {
- error!("Request failed: {}", err);
resolved_map.insert(event_id, Err(err.to_string()));
continue;
}
@@ -472,10 +504,10 @@ pub async fn send_transaction_message_route<'a>(
.iter()
.chain(get_state_response.auth_chain.iter()) // add auth events
.map(|pdu| {
- let (event_id, json) = process_incoming_pdu(pdu);
+ let (event_id, json) = crate::pdu::process_incoming_pdu(pdu);
(
event_id.clone(),
- std::sync::Arc::new(
+ Arc::new(
// When creating a StateEvent the event_id arg will be used
// over any found in the json and it will not use ruma::reference_hash
// to generate one
@@ -486,65 +518,12 @@ pub async fn send_transaction_message_route<'a>(
})
.collect::<BTreeMap<_, _>>();
- if value.get("state_key").is_none() {
- if !db.rooms.is_joined(&pdu.sender, room_id)? {
- error!("Sender is not joined {}", pdu.kind);
- resolved_map.insert(event_id, Err("User is not in this room".into()));
- continue;
- }
-
- // If the event is older than the last event in pduid_pdu Tree then find the
- // closest ancestor we know of and insert after the known ancestor by
- // altering the known events pduid to = same roomID + same count bytes + 0x1
- // pushing a single byte every time a simple append cannot be done.
- match db
- .rooms
- .get_closest_parent(room_id, &pdu.prev_events, &their_current_state)?
- {
- Some(ClosestParent::Append) => {
- let count = db.globals.next_count()?;
- let mut pdu_id = room_id.as_bytes().to_vec();
- pdu_id.push(0xff);
- pdu_id.extend_from_slice(&count.to_be_bytes());
-
- db.rooms.append_pdu(
- &pdu,
- &value,
- count,
- pdu_id.into(),
- &db.globals,
- &db.account_data,
- &db.admin,
- )?;
- }
- Some(ClosestParent::Insert(old_count)) => {
- println!("INSERT PDU FOUND {}", old_count);
-
- let count = old_count;
- let mut pdu_id = room_id.as_bytes().to_vec();
- pdu_id.push(0xff);
- pdu_id.extend_from_slice(&count.to_be_bytes());
- // Create a new count that is after old_count but before
- // the pdu appended after
- pdu_id.push(1);
-
- db.rooms.append_pdu(
- &pdu,
- &value,
- count,
- pdu_id.into(),
- &db.globals,
- &db.account_data,
- &db.admin,
- )?;
- }
- _ => panic!("Not a sequential event or no parents found"),
- };
- resolved_map.insert(event_id, Ok::<(), String>(()));
- continue;
- }
-
let our_current_state = db.rooms.room_state_full(room_id)?;
+ // State resolution takes care of these checks
+ // 4. Passes authorization rules based on the event's auth events, otherwise it is rejected.
+ // 5. Passes authorization rules based on the state at the event, otherwise it is rejected.
+
+ // TODO: 6. Passes authorization rules based on the current state of the room, otherwise it is "soft failed".
match state_res::StateResolution::resolve(
room_id,
&ruma::RoomVersionId::Version6,
@@ -576,7 +555,7 @@ pub async fn send_transaction_message_route<'a>(
// closest ancestor we know of and insert after the known ancestor by
// altering the known events pduid to = same roomID + same count bytes + 0x1
// pushing a single byte every time a simple append cannot be done.
- match db.rooms.get_closest_parent(
+ match db.rooms.get_latest_pduid_before(
room_id,
&pdu.prev_events,
&their_current_state,
@@ -598,8 +577,6 @@ pub async fn send_transaction_message_route<'a>(
)?;
}
Some(ClosestParent::Insert(old_count)) => {
- println!("INSERT STATE PDU FOUND {}", old_count);
-
let count = old_count;
let mut pdu_id = room_id.as_bytes().to_vec();
pdu_id.push(0xff);
@@ -618,14 +595,16 @@ pub async fn send_transaction_message_route<'a>(
&db.admin,
)?;
}
- _ => panic!("Not a sequential event or no parents found"),
+ _ => {
+ error!("Not a sequential event or no parents found");
+ continue;
+ }
}
resolved_map.insert(event_id, Ok::<(), String>(()));
}
// If the eventId is not found in the resolved state auth has failed
Ok(_) => {
- // TODO have state_res give the actual auth error in this case
resolved_map.insert(
event_id,
Err("This event failed authentication, not found in resolved set".into()),
@@ -637,7 +616,7 @@ pub async fn send_transaction_message_route<'a>(
};
}
- Ok(dbg!(send_transaction_message::v1::Response { pdus: resolved_map }).into())
+ Ok(send_transaction_message::v1::Response { pdus: resolved_map }.into())
}
#[cfg_attr(
@@ -748,25 +727,3 @@ pub fn get_user_devices_route<'a>(
.into())
}
*/
-
-/// Generates a correct eventId for the incoming pdu.
-///
-/// Returns a tuple of the new `EventId` and the PDU with the eventId inserted as a `serde_json::Value`.
-fn process_incoming_pdu(pdu: &ruma::Raw<ruma::events::pdu::Pdu>) -> (EventId, CanonicalJsonObject) {
- let mut value =
- serde_json::from_str(pdu.json().get()).expect("A Raw<...> is always valid JSON");
-
- let event_id = EventId::try_from(&*format!(
- "${}",
- ruma::signatures::reference_hash(&value, &RoomVersionId::Version6)
- .expect("ruma can calculate reference hashes")
- ))
- .expect("ruma's reference hashes are valid event ids");
-
- value.insert(
- "event_id".to_owned(),
- to_canonical_value(&event_id).expect("EventId is a valid CanonicalJsonValue"),
- );
-
- (event_id, value)
-}