From afc063aacff98c62b6e5bf2abe4241e688b7a398 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Sat, 14 Mar 2020 15:25:37 +0000 Subject: [PATCH] Add stricter checks to adding a node to address space - the namespace index must exist or it will panic. Fix samples to do things the proper way. --- samples/chess-server/src/main.rs | 23 ++++++++----- samples/demo-server/src/control.rs | 4 +-- samples/demo-server/src/main.rs | 14 +++++--- samples/demo-server/src/methods.rs | 6 ++-- samples/demo-server/src/scalar.rs | 42 +++++++++++------------ samples/mqtt-client/src/main.rs | 7 ++-- samples/simple-client/src/main.rs | 6 ++-- samples/simple-server/src/main.rs | 18 ++++++---- server/src/address_space/address_space.rs | 18 +++++++++- server/src/events/event.rs | 2 +- 10 files changed, 87 insertions(+), 53 deletions(-) diff --git a/samples/chess-server/src/main.rs b/samples/chess-server/src/main.rs index 48f2d6066..755c51ae6 100644 --- a/samples/chess-server/src/main.rs +++ b/samples/chess-server/src/main.rs @@ -40,8 +40,11 @@ fn main() { let address_space = server.address_space(); - { + let ns = { let mut address_space = address_space.write().unwrap(); + + let ns = address_space.register_namespace("urn:chess-server").unwrap(); + let board_node_id = address_space .add_folder("Board", "Board", &NodeId::objects_folder_id()) .unwrap(); @@ -49,7 +52,7 @@ fn main() { BOARD_SQUARES.iter().for_each(|square| { // Variable represents each square's state let browse_name = *square; - let node_id = NodeId::new(2, *square); + let node_id = NodeId::new(ns, *square); VariableBuilder::new(&node_id, browse_name, browse_name) .organized_by(&board_node_id) .value(0u8) @@ -57,7 +60,7 @@ fn main() { // Another variable is a highlighting flag for the square let browse_name = format!("{}.highlight", square); - let node_id = NodeId::new(2, browse_name.clone()); + let node_id = NodeId::new(ns, browse_name.clone()); VariableBuilder::new(&node_id, browse_name, "") .organized_by(&board_node_id) .value(false) @@ -65,8 +68,10 @@ fn main() { }); let game = game.lock().unwrap(); - update_board_state(&game, &mut address_space); - } + update_board_state(&game, &mut address_space, ns); + + ns + }; // Spawn a thread for the game which will update server state @@ -96,7 +101,7 @@ fn main() { { let mut address_space = address_space.write().unwrap(); - update_board_state(&game, &mut address_space); + update_board_state(&game, &mut address_space, ns); } } @@ -108,17 +113,17 @@ fn main() { server.run(); } -fn update_board_state(game: &game::Game, address_space: &mut AddressSpace) { +fn update_board_state(game: &game::Game, address_space: &mut AddressSpace, ns: u16) { let now = DateTime::now(); BOARD_SQUARES.iter().for_each(|square| { // Piece on the square let square_value = game.square_from_str(square); - let node_id = NodeId::new(2, *square); + let node_id = NodeId::new(ns, *square); let _ = address_space.set_variable_value(node_id, square_value as u8, &now, &now); // Highlight the square - let node_id = NodeId::new(2, format!("{}.highlight", square)); + let node_id = NodeId::new(ns, format!("{}.highlight", square)); let highlight_square = if let Some(ref last_move) = game.last_move { last_move.contains(square) } else { diff --git a/samples/demo-server/src/control.rs b/samples/demo-server/src/control.rs index bdc1705f6..043264ca6 100644 --- a/samples/demo-server/src/control.rs +++ b/samples/demo-server/src/control.rs @@ -2,9 +2,9 @@ use opcua_server::{ prelude::*, }; -pub fn add_control_switches(server: &mut Server) { +pub fn add_control_switches(server: &mut Server, ns: u16) { // The address space is guarded so obtain a lock to change it - let abort_node_id = NodeId::new(2u16, "abort"); + let abort_node_id = NodeId::new(ns, "abort"); let address_space = server.address_space(); let server_state = server.server_state(); diff --git a/samples/demo-server/src/main.rs b/samples/demo-server/src/main.rs index 858c6526d..8c37c6d50 100644 --- a/samples/demo-server/src/main.rs +++ b/samples/demo-server/src/main.rs @@ -33,20 +33,26 @@ fn main() { // Create an OPC UA server with sample configuration and default node set let mut server = Server::new(ServerConfig::load(&PathBuf::from("../server.conf")).unwrap()); + let ns = { + let address_space = server.address_space(); + let mut address_space = address_space.write().unwrap(); + address_space.register_namespace("urn:demo-server").unwrap() + }; + // Add some objects representing machinery machine::add_machinery(&mut server); // Add some scalar variables - scalar::add_scalar_variables(&mut server); + scalar::add_scalar_variables(&mut server, ns); // Add some rapidly changing values - scalar::add_stress_variables(&mut server); + scalar::add_stress_variables(&mut server, ns); // Add some control switches, e.g. abort flag - control::add_control_switches(&mut server); + control::add_control_switches(&mut server, ns); // Add some methods - methods::add_methods(&mut server); + methods::add_methods(&mut server, ns); // Add historical data providers historical::add_providers(&mut server); diff --git a/samples/demo-server/src/methods.rs b/samples/demo-server/src/methods.rs index 45c1ecc72..2728c2c12 100644 --- a/samples/demo-server/src/methods.rs +++ b/samples/demo-server/src/methods.rs @@ -7,17 +7,17 @@ use opcua_server::{ session::Session, }; -pub fn add_methods(server: &mut Server) { +pub fn add_methods(server: &mut Server, ns: u16) { let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); - let object_id = NodeId::new(2, "Functions"); + let object_id = NodeId::new(ns, "Functions"); ObjectBuilder::new(&object_id, "Functions", "Functions") .event_notifier(EventNotifier::SUBSCRIBE_TO_EVENTS) .organized_by(ObjectId::ObjectsFolder) .insert(&mut address_space); - let fn_node_id = NodeId::new(2, "HelloWorld"); + let fn_node_id = NodeId::new(ns, "HelloWorld"); MethodBuilder::new(&fn_node_id, "HelloWorld", "HelloWorld") .component_of(object_id.clone()) .output_args(&mut address_space, &[ diff --git a/samples/demo-server/src/scalar.rs b/samples/demo-server/src/scalar.rs index d6c8e482c..ce0cde090 100644 --- a/samples/demo-server/src/scalar.rs +++ b/samples/demo-server/src/scalar.rs @@ -5,7 +5,7 @@ use opcua_server::{ prelude::*, }; -pub fn add_scalar_variables(server: &mut Server) { +pub fn add_scalar_variables(server: &mut Server, ns: u16) { let (static_folder_id, dynamic_folder_id) = { let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); @@ -20,13 +20,13 @@ pub fn add_scalar_variables(server: &mut Server) { }; // Add static scalar values - add_static_scalar_variables(server, &static_folder_id); - add_static_array_variables(server, &static_folder_id); + add_static_scalar_variables(server, ns, &static_folder_id); + add_static_array_variables(server, ns, &static_folder_id); // Add dynamically changing scalar values - add_dynamic_scalar_variables(server, &dynamic_folder_id); - add_dynamic_array_variables(server, &dynamic_folder_id); - set_dynamic_timers(server); + add_dynamic_scalar_variables(server, ns, &dynamic_folder_id); + add_dynamic_array_variables(server, ns, &dynamic_folder_id); + set_dynamic_timers(server, ns); } const SCALAR_TYPES: [DataTypeId; 14] = [ @@ -35,7 +35,7 @@ const SCALAR_TYPES: [DataTypeId; 14] = [ DataTypeId::Double, DataTypeId::String, DataTypeId::DateTime, DataTypeId::Guid ]; -pub fn scalar_node_id(id: DataTypeId, is_dynamic: bool, is_array: bool) -> NodeId { +pub fn scalar_node_id(ns: u16, id: DataTypeId, is_dynamic: bool, is_array: bool) -> NodeId { let mut name = scalar_name(id).to_string(); if is_dynamic { name.push_str("Dynamic"); @@ -43,7 +43,7 @@ pub fn scalar_node_id(id: DataTypeId, is_dynamic: bool, is_array: bool) -> NodeI if is_array { name.push_str("Array"); } - NodeId::new(2, name) + NodeId::new(ns, name) } pub fn scalar_name(id: DataTypeId) -> &'static str { @@ -113,7 +113,7 @@ pub fn scalar_random_value(id: DataTypeId) -> Variant { } /// Creates some sample variables, and some push / pull examples that update them -fn add_static_scalar_variables(server: &mut Server, static_folder_id: &NodeId) { +fn add_static_scalar_variables(server: &mut Server, ns: u16, static_folder_id: &NodeId) { // The address space is guarded so obtain a lock to change it let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); @@ -125,7 +125,7 @@ fn add_static_scalar_variables(server: &mut Server, static_folder_id: &NodeId) { for sn in SCALAR_TYPES.iter() { let name = scalar_name(*sn); - let node_id = scalar_node_id(*sn, false, false); + let node_id = scalar_node_id(ns, *sn, false, false); VariableBuilder::new(&node_id, name, name) .data_type(sn) .value(scalar_default_value(*sn)) @@ -134,7 +134,7 @@ fn add_static_scalar_variables(server: &mut Server, static_folder_id: &NodeId) { } } -fn add_static_array_variables(server: &mut Server, static_folder_id: &NodeId) { +fn add_static_array_variables(server: &mut Server, ns: u16, static_folder_id: &NodeId) { // The address space is guarded so obtain a lock to change it let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); @@ -145,7 +145,7 @@ fn add_static_array_variables(server: &mut Server, static_folder_id: &NodeId) { .unwrap(); SCALAR_TYPES.iter().for_each(|sn| { - let node_id = scalar_node_id(*sn, false, true); + let node_id = scalar_node_id(ns, *sn, false, true); let name = scalar_name(*sn); let values = (0..100).map(|_| scalar_default_value(*sn)).collect::>(); VariableBuilder::new(&node_id, name, name) @@ -157,7 +157,7 @@ fn add_static_array_variables(server: &mut Server, static_folder_id: &NodeId) { }); } -fn add_dynamic_scalar_variables(server: &mut Server, dynamic_folder_id: &NodeId) { +fn add_dynamic_scalar_variables(server: &mut Server, ns: u16, dynamic_folder_id: &NodeId) { // The address space is guarded so obtain a lock to change it let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); @@ -168,7 +168,7 @@ fn add_dynamic_scalar_variables(server: &mut Server, dynamic_folder_id: &NodeId) .unwrap(); SCALAR_TYPES.iter().for_each(|sn| { - let node_id = scalar_node_id(*sn, true, false); + let node_id = scalar_node_id(ns, *sn, true, false); let name = scalar_name(*sn); VariableBuilder::new(&node_id, name, name) .data_type(*sn) @@ -178,7 +178,7 @@ fn add_dynamic_scalar_variables(server: &mut Server, dynamic_folder_id: &NodeId) }); } -fn add_dynamic_array_variables(server: &mut Server, dynamic_folder_id: &NodeId) { +fn add_dynamic_array_variables(server: &mut Server, ns: u16, dynamic_folder_id: &NodeId) { // The address space is guarded so obtain a lock to change it let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); @@ -189,7 +189,7 @@ fn add_dynamic_array_variables(server: &mut Server, dynamic_folder_id: &NodeId) .unwrap(); SCALAR_TYPES.iter().for_each(|sn| { - let node_id = scalar_node_id(*sn, true, true); + let node_id = scalar_node_id(ns, *sn, true, true); let name = scalar_name(*sn); let values = (0..10).map(|_| scalar_default_value(*sn)).collect::>(); VariableBuilder::new(&node_id, name, name) @@ -201,7 +201,7 @@ fn add_dynamic_array_variables(server: &mut Server, dynamic_folder_id: &NodeId) }); } -fn set_dynamic_timers(server: &mut Server) { +fn set_dynamic_timers(server: &mut Server, ns: u16) { let address_space = server.address_space(); // Standard change timers @@ -210,18 +210,18 @@ fn set_dynamic_timers(server: &mut Server) { // Scalar let now = DateTime::now(); SCALAR_TYPES.iter().for_each(|sn| { - let node_id = scalar_node_id(*sn, true, false); + let node_id = scalar_node_id(ns, *sn, true, false); let _ = address_space.set_variable_value_by_ref(&node_id, scalar_random_value(*sn), &now, &now); - let node_id = scalar_node_id(*sn, true, true); + let node_id = scalar_node_id(ns, *sn, true, true); let values = (0..10).map(|_| scalar_random_value(*sn)).collect::>(); let _ = address_space.set_variable_value_by_ref(&node_id, values, &now, &now); }); }); } -pub fn add_stress_variables(server: &mut Server) { - let node_ids = (0..1000).map(|i| NodeId::new(2, format!("v{:04}", i))).collect::>(); +pub fn add_stress_variables(server: &mut Server, ns: u16) { + let node_ids = (0..1000).map(|i| NodeId::new(ns, format!("v{:04}", i))).collect::>(); let address_space = server.address_space(); let mut address_space = address_space.write().unwrap(); diff --git a/samples/mqtt-client/src/main.rs b/samples/mqtt-client/src/main.rs index ce7850fb0..e13883f28 100644 --- a/samples/mqtt-client/src/main.rs +++ b/samples/mqtt-client/src/main.rs @@ -93,8 +93,9 @@ fn main() -> Result<(), ()> { // endpoints one of which is marked as the default. let mut client = Client::new(ClientConfig::load(&PathBuf::from(config_file)).unwrap()); let endpoint_id: Option<&str> = if !endpoint_id.is_empty() { Some(&endpoint_id) } else { None }; + let ns = 2; if let Ok(session) = client.connect_to_endpoint_id(endpoint_id) { - let _ = subscription_loop(session, tx).map_err(|err| { + let _ = subscription_loop(session, tx, ns).map_err(|err| { println!("ERROR: Got an error while performing action - {}", err); }); } @@ -102,7 +103,7 @@ fn main() -> Result<(), ()> { Ok(()) } -fn subscription_loop(session: Arc>, tx: mpsc::Sender<(NodeId, DataValue)>) -> Result<(), StatusCode> { +fn subscription_loop(session: Arc>, tx: mpsc::Sender<(NodeId, DataValue)>, ns: u16) -> Result<(), StatusCode> { // Create a subscription println!("Creating subscription"); @@ -127,7 +128,7 @@ fn subscription_loop(session: Arc>, tx: mpsc::Sender<(NodeId, Da // Create some monitored items let items_to_create: Vec = ["v1", "v2", "v3", "v4"].iter() - .map(|v| NodeId::new(2, *v).into()).collect(); + .map(|v| NodeId::new(ns, *v).into()).collect(); let _ = session.create_monitored_items(subscription_id, TimestampsToReturn::Both, &items_to_create)?; } diff --git a/samples/simple-client/src/main.rs b/samples/simple-client/src/main.rs index b079fef5b..bf44f8b75 100644 --- a/samples/simple-client/src/main.rs +++ b/samples/simple-client/src/main.rs @@ -51,7 +51,7 @@ fn main() -> Result<(), ()> { .client().unwrap(); if let Ok(session) = client.connect_to_endpoint((args.url.as_ref(), SecurityPolicy::None.to_str(), MessageSecurityMode::None, UserTokenPolicy::anonymous()), IdentityToken::Anonymous) { - if let Err(result) = subscribe_to_variables(session.clone()) { + if let Err(result) = subscribe_to_variables(session.clone(), 2) { println!("ERROR: Got an error while subscribing to variables - {}", result); } else { // Loops forever. The publish thread will call the callback with changes on the variables @@ -62,7 +62,7 @@ fn main() -> Result<(), ()> { Ok(()) } -fn subscribe_to_variables(session: Arc>) -> Result<(), StatusCode> { +fn subscribe_to_variables(session: Arc>, ns: u16) -> Result<(), StatusCode> { let mut session = session.write().unwrap(); // Creates a subscription with a data change callback let subscription_id = session.create_subscription(2000.0, 10, 30, 0, 0, true, DataChangeCallback::new(|changed_monitored_items| { @@ -73,7 +73,7 @@ fn subscribe_to_variables(session: Arc>) -> Result<(), StatusCod // Create some monitored items let items_to_create: Vec = ["v1", "v2", "v3", "v4"].iter() - .map(|v| NodeId::new(2, *v).into()).collect(); + .map(|v| NodeId::new(ns, *v).into()).collect(); let _ = session.create_monitored_items(subscription_id, TimestampsToReturn::Both, &items_to_create)?; Ok(()) diff --git a/samples/simple-server/src/main.rs b/samples/simple-server/src/main.rs index a3da9577f..e07dde968 100644 --- a/samples/simple-server/src/main.rs +++ b/samples/simple-server/src/main.rs @@ -14,20 +14,26 @@ fn main() { // Create an OPC UA server with sample configuration and default node set let mut server = Server::new(ServerConfig::load(&PathBuf::from("../server.conf")).unwrap()); + let ns = { + let address_space = server.address_space(); + let mut address_space = address_space.write().unwrap(); + address_space.register_namespace("urn:simple-server").unwrap() + }; + // Add some variables of our own - add_example_variables(&mut server); + add_example_variables(&mut server, ns); // Run the server. This does not ordinarily exit so you must Ctrl+C to terminate server.run(); } /// Creates some sample variables, and some push / pull examples that update them -fn add_example_variables(server: &mut Server) { +fn add_example_variables(server: &mut Server, ns: u16) { // These will be the node ids of the new variables - let v1_node = NodeId::new(2, "v1"); - let v2_node = NodeId::new(2, "v2"); - let v3_node = NodeId::new(2, "v3"); - let v4_node = NodeId::new(2, "v4"); + let v1_node = NodeId::new(ns, "v1"); + let v2_node = NodeId::new(ns, "v2"); + let v3_node = NodeId::new(ns, "v3"); + let v4_node = NodeId::new(ns, "v4"); let address_space = server.address_space(); diff --git a/server/src/address_space/address_space.rs b/server/src/address_space/address_space.rs index 1bc03f2fb..2e3474a0a 100644 --- a/server/src/address_space/address_space.rs +++ b/server/src/address_space/address_space.rs @@ -199,6 +199,8 @@ impl AddressSpace { /// Registers a namespace described by a uri with address space. The return code is the index /// of the newly added namespace / index. The index is used with `NodeId`. Registering a /// namespace that is already registered will return the index to the previous instance. + /// The last registered namespace becomes the default namespace unless you explcitly call + /// `set_default_namespace()` after this. pub fn register_namespace(&mut self, namespace: &str) -> Result { use std::u16; let now = DateTime::now(); @@ -214,7 +216,10 @@ impl AddressSpace { self.namespaces.push(namespace.into()); self.set_namespaces(&now); // New namespace index - Ok((self.namespaces.len() - 1) as u16) + let ns = (self.namespaces.len() - 1) as u16; + // Make this the new default namespace + self.default_namespace = ns; + Ok(ns) } } } @@ -477,6 +482,12 @@ impl AddressSpace { expect_and_find_object!(self, &NodeId::views_folder_id()) } + fn assert_namespace(&self, node_id: &NodeId) { + if node_id.namespace as usize > self.namespaces.len() { + panic!("Namespace index {} does not exist", node_id.namespace); + } + } + /// Sets the default namespace pub fn set_default_namespace(&mut self, default_namespace: u16) { self.default_namespace = default_namespace; @@ -505,6 +516,9 @@ impl AddressSpace { S: Into + Clone { let node_type = node.into(); let node_id = node_type.node_id(); + + self.assert_namespace(&node_id); + if self.node_exists(&node_id) { error!("This node {:?} already exists", node_id); false @@ -560,6 +574,7 @@ impl AddressSpace { pub fn add_folder_with_id(&mut self, node_id: &NodeId, browse_name: R, display_name: S, parent_node_id: &NodeId) -> bool where R: Into, S: Into { + self.assert_namespace(node_id); ObjectBuilder::new(node_id, browse_name, display_name) .is_folder() .organized_by(parent_node_id.clone()) @@ -571,6 +586,7 @@ impl AddressSpace { where R: Into, S: Into { let node_id = NodeId::next_numeric(self.default_namespace); + self.assert_namespace(&node_id); if self.add_folder_with_id(&node_id, browse_name, display_name, parent_node_id) { Ok(node_id) } else { diff --git a/server/src/events/event.rs b/server/src/events/event.rs index 2a86744ea..fc604e049 100644 --- a/server/src/events/event.rs +++ b/server/src/events/event.rs @@ -421,7 +421,7 @@ fn test_purge_events() { let mut address_space = AddressSpace::new(); // Nodes will be created in this namespace - let ns = 100; + let ns = address_space.register_namespace("urn:mynamespace").unwrap(); // This test is going to raise a bunch of events and then purge some of them. The purged // events should be the ones expected to be purged and there should be no trace of them