Skip to content

Commit 2bb27b7

Browse files
AnotherDanielAnotherDaniel
authored
Implement request topic validation (eclipse-uprotocol#28)
Implements: dsn~usubscription-subscribe-invalid-topic~1 dsn~usubscription-unsubscribe-invalid-topic~1 dsn~usubscription-unregister-notifications-invalid-topic~1
1 parent 4c1b98a commit 2bb27b7

File tree

3 files changed

+138
-3
lines changed

3 files changed

+138
-3
lines changed

‎up-subscription/src/handlers/subscribe.rs‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ impl RequestHandler for SubscriptionRequestHandler {
5959
));
6060
};
6161

62+
// [impl->dsn~usubscription-subscribe-invalid-topic~1]
63+
helpers::validate_uri(topic).map_err(|e| {
64+
ServiceInvocationError::InvalidArgument(format!("Invalid topic uri '{topic}': {e}"))
65+
})?;
66+
6267
// Provisionally compute milliseconds to subscription expiry, from protobuf.google.Timestamp input in second granularity (we ignore the nanos).
6368
// Likely to change in the future, when we get rid of the protobuf.google.Timestamp type and track in milliseconds throughought.
6469
let expiry: Option<usubscription::ExpiryTimestamp> =
@@ -116,9 +121,13 @@ impl RequestHandler for SubscriptionRequestHandler {
116121

117122
#[cfg(test)]
118123
mod tests {
124+
use std::str::FromStr;
125+
119126
use super::*;
127+
use test_case::test_case;
120128
use tokio::sync::mpsc::{self};
121-
use up_rust::core::usubscription::State;
129+
130+
use up_rust::{core::usubscription::State, UUri};
122131

123132
use crate::{helpers, tests::test_lib};
124133

@@ -387,4 +396,37 @@ mod tests {
387396

388397
assert!(result.is_err_and(|e| matches!(e, ServiceInvocationError::InvalidArgument(_))));
389398
}
399+
400+
// [utest->dsn~usubscription-subscribe-invalid-topic~1]
401+
#[test_case("up:/0/0/0"; "Bad topic UUri")]
402+
#[test_case("up://*/100000/1/8AC7"; "Wildcard authority in topic UUri")]
403+
#[test_case("up://LOCAL/FFFF0000/1/8AC7"; "Wildcard entity id in topic UUri")]
404+
#[test_case("up://LOCAL/100000/1/FFFF"; "Wildcard resource id in topic UUri")]
405+
#[tokio::test]
406+
async fn test_invalid_topic_uri(topic: &str) {
407+
helpers::init_once();
408+
409+
// create request and other required object(s)
410+
let topic = UUri::from_str(topic).expect("Test parameter UUri failed to parse");
411+
let subscribe_request = test_lib::helpers::subscription_request(topic, None);
412+
let request_payload = UPayload::try_from_protobuf(subscribe_request.clone()).unwrap();
413+
let message_attributes = UAttributes {
414+
source: Some(test_lib::helpers::subscriber_uri1()).into(),
415+
..Default::default()
416+
};
417+
let (subscription_sender, _) = mpsc::channel::<SubscriptionEvent>(1);
418+
419+
// create handler and perform tested operation
420+
let request_handler = SubscriptionRequestHandler::new(subscription_sender);
421+
422+
let result = request_handler
423+
.handle_request(
424+
up_rust::core::usubscription::RESOURCE_ID_SUBSCRIBE,
425+
&message_attributes,
426+
Some(request_payload),
427+
)
428+
.await;
429+
430+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
431+
}
390432
}

‎up-subscription/src/handlers/unregister_for_notifications.rs‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,27 @@ impl RequestHandler for UnregisterNotificationsRequestHandler {
4646
request_payload: Option<UPayload>,
4747
) -> Result<Option<UPayload>, ServiceInvocationError> {
4848
// [impl->dsn~usubscription-unregister-notifications-protobuf~1]
49-
let (_subscription_request, source) = helpers::extract_inputs::<NotificationsRequest>(
49+
let (notification_request, source) = helpers::extract_inputs::<NotificationsRequest>(
5050
RESOURCE_ID_UNREGISTER_FOR_NOTIFICATIONS,
5151
resource_id,
5252
&request_payload,
5353
message_attributes,
5454
)?;
5555

56+
let Some(topic) = notification_request.topic.as_ref() else {
57+
return Err(ServiceInvocationError::InvalidArgument(
58+
"No topic defined in request".to_string(),
59+
));
60+
};
61+
62+
// [impl->dsn~usubscription-unregister-notifications-invalid-topic~1]
63+
helpers::validate_uri(topic).map_err(|e| {
64+
ServiceInvocationError::InvalidArgument(format!("Invalid topic uri '{topic}': {e}"))
65+
})?;
66+
67+
// TODO: can/should we actually use the topic alongside the subscriber, for notification-removal?
68+
// UGH - this entire notifications storage thing doesn't work, can only store one topic per subscriber atm. Needs fixed...
69+
5670
// Interact with notification manager backend
5771
let se = NotificationEvent::RemoveNotifyee {
5872
subscriber: source.clone(),
@@ -78,8 +92,12 @@ impl RequestHandler for UnregisterNotificationsRequestHandler {
7892
#[cfg(test)]
7993
mod tests {
8094
use super::*;
95+
use std::str::FromStr;
96+
use test_case::test_case;
8197
use tokio::sync::mpsc::{self};
8298

99+
use up_rust::UUri;
100+
83101
use crate::{helpers, tests::test_lib};
84102

85103
// [utest->dsn~usubscription-unregister-notifications-protobuf~1]
@@ -236,4 +254,38 @@ mod tests {
236254

237255
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
238256
}
257+
258+
// [utest->dsn~usubscription-unregister-notifications-invalid-topic~1]
259+
#[test_case("up:/0/0/0"; "Bad topic UUri")]
260+
#[test_case("up://*/100000/1/8AC7"; "Wildcard authority in topic UUri")]
261+
#[test_case("up://LOCAL/FFFF0000/1/8AC7"; "Wildcard entity id in topic UUri")]
262+
#[test_case("up://LOCAL/100000/1/FFFF"; "Wildcard resource id in topic UUri")]
263+
#[tokio::test]
264+
async fn test_invalid_topic_uri(topic: &str) {
265+
helpers::init_once();
266+
267+
// create request and other required object(s)
268+
let topic = UUri::from_str(topic).expect("Test parameter UUri failed to parse");
269+
// create request and other required object(s)
270+
let subscribe_request = test_lib::helpers::subscription_request(topic, None);
271+
let request_payload = UPayload::try_from_protobuf(subscribe_request.clone()).unwrap();
272+
let message_attributes = UAttributes {
273+
source: Some(test_lib::helpers::subscriber_uri1()).into(),
274+
..Default::default()
275+
};
276+
let (subscription_sender, _) = mpsc::channel::<NotificationEvent>(1);
277+
278+
// create handler and perform tested operation
279+
let request_handler = UnregisterNotificationsRequestHandler::new(subscription_sender);
280+
281+
let result = request_handler
282+
.handle_request(
283+
up_rust::core::usubscription::RESOURCE_ID_UNREGISTER_FOR_NOTIFICATIONS,
284+
&message_attributes,
285+
Some(request_payload),
286+
)
287+
.await;
288+
289+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
290+
}
239291
}

‎up-subscription/src/handlers/unsubscribe.rs‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ impl RequestHandler for UnubscribeRequestHandler {
5858
));
5959
};
6060

61+
// [impl->dsn~usubscription-unsubscribe-invalid-topic~1]
62+
helpers::validate_uri(topic).map_err(|e| {
63+
ServiceInvocationError::InvalidArgument(format!("Invalid topic uri '{topic}': {e}"))
64+
})?;
65+
6166
let (respond_to, receive_from) = oneshot::channel::<SubscriptionStatus>();
6267
let se = SubscriptionEvent::RemoveSubscription {
6368
subscriber: source.clone(),
@@ -91,8 +96,11 @@ impl RequestHandler for UnubscribeRequestHandler {
9196
#[cfg(test)]
9297
mod tests {
9398
use super::*;
99+
use std::str::FromStr;
100+
use test_case::test_case;
94101
use tokio::sync::mpsc::{self};
95-
use up_rust::core::usubscription::State;
102+
103+
use up_rust::{core::usubscription::State, UUri};
96104

97105
use crate::{helpers, tests::test_lib};
98106

@@ -258,4 +266,37 @@ mod tests {
258266

259267
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
260268
}
269+
270+
// [utest->dsn~usubscription-unsubscribe-invalid-topic~1]
271+
#[test_case("up:/0/0/0"; "Bad topic UUri")]
272+
#[test_case("up://*/100000/1/8AC7"; "Wildcard authority in topic UUri")]
273+
#[test_case("up://LOCAL/FFFF0000/1/8AC7"; "Wildcard entity id in topic UUri")]
274+
#[test_case("up://LOCAL/100000/1/FFFF"; "Wildcard resource id in topic UUri")]
275+
#[tokio::test]
276+
async fn test_invalid_topic_uri(topic: &str) {
277+
helpers::init_once();
278+
279+
// create request and other required object(s)
280+
let topic = UUri::from_str(topic).expect("Test parameter UUri failed to parse");
281+
let subscribe_request = test_lib::helpers::subscription_request(topic, None);
282+
let request_payload = UPayload::try_from_protobuf(subscribe_request.clone()).unwrap();
283+
let message_attributes = UAttributes {
284+
source: Some(test_lib::helpers::subscriber_uri1()).into(),
285+
..Default::default()
286+
};
287+
let (subscription_sender, _) = mpsc::channel::<SubscriptionEvent>(1);
288+
289+
// create handler and perform tested operation
290+
let request_handler = UnubscribeRequestHandler::new(subscription_sender);
291+
292+
let result = request_handler
293+
.handle_request(
294+
up_rust::core::usubscription::RESOURCE_ID_UNSUBSCRIBE,
295+
&message_attributes,
296+
Some(request_payload),
297+
)
298+
.await;
299+
300+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
301+
}
261302
}

0 commit comments

Comments
 (0)