From 2f6e1c7f27ccf2542f2b51f018953f145c405757 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Fri, 5 Apr 2019 09:14:49 +0100 Subject: [PATCH 1/6] Remove acknowledged notifications when publish request is received, not when response is sent. Per spec and stops spurious available notifications being sent to client. --- server/src/subscriptions/mod.rs | 11 ++++-- server/src/subscriptions/subscriptions.rs | 27 +++++++++++---- server/src/tests/mod.rs | 41 +++++++++++++++-------- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/server/src/subscriptions/mod.rs b/server/src/subscriptions/mod.rs index c18d7b195..17e4f9bb2 100644 --- a/server/src/subscriptions/mod.rs +++ b/server/src/subscriptions/mod.rs @@ -1,5 +1,8 @@ -use opcua_types::SupportedMessage; -use opcua_types::service_types::PublishRequest; +use opcua_types::{ + SupportedMessage, + status_code::StatusCode, + service_types::PublishRequest, +}; /// The publish request entry preserves the request_id which is part of the chunk layer but clients /// are fickle about receiving responses from the same as the request. Normally this is easy because @@ -7,8 +10,12 @@ use opcua_types::service_types::PublishRequest; /// so that later we can send out responses that have the proper req id #[derive(Clone)] pub struct PublishRequestEntry { + // The request id pub request_id: u32, + // The request itself pub request: PublishRequest, + // The result of clearing acknowledgments when the request was received. + pub results: Option>, } #[derive(Clone, Debug, PartialEq)] diff --git a/server/src/subscriptions/subscriptions.rs b/server/src/subscriptions/subscriptions.rs index 9e412be8a..e36579522 100644 --- a/server/src/subscriptions/subscriptions.rs +++ b/server/src/subscriptions/subscriptions.rs @@ -56,7 +56,17 @@ impl Subscriptions { } #[cfg(test)] - pub fn retransmission_queue(&mut self) -> &mut BTreeMap<(u32, u32), NotificationMessage> { + pub(crate) fn publish_request_queue(&mut self) -> &mut VecDeque { + &mut self.publish_request_queue + } + + #[cfg(test)] + pub(crate) fn publish_response_queue(&mut self) -> &mut VecDeque { + &mut self.publish_response_queue + } + + #[cfg(test)] + pub(crate) fn retransmission_queue(&mut self) -> &mut BTreeMap<(u32, u32), NotificationMessage> { &mut self.retransmission_queue } @@ -96,10 +106,15 @@ impl Subscriptions { } Err(StatusCode::BadTooManyPublishRequests) } else { + // Clear all acknowledged items here + // Acknowledge results + let results = self.process_subscription_acknowledgements(&request); + // Add to the front of the queue - older items are popped from the back self.publish_request_queue.push_front(PublishRequestEntry { request_id, request, + results }); Ok(()) } @@ -196,10 +211,8 @@ impl Subscriptions { // The notification to be sent is now put into the retransmission queue self.retransmission_queue.insert((subscription_id, notification_message.sequence_number), notification_message.clone()); - // Acknowledge results - let results = self.process_subscription_acknowledgements(&publish_request.request); - - let response = self.make_publish_response(&publish_request, subscription_id, now, notification_message, more_notifications, available_sequence_numbers, results); + // Enqueue a publish response + let response = self.make_publish_response(publish_request, subscription_id, now, notification_message, more_notifications, available_sequence_numbers); self.publish_response_queue.push_back(response); } @@ -309,7 +322,7 @@ impl Subscriptions { } } - fn make_publish_response(&self, publish_request: &PublishRequestEntry, subscription_id: u32, now: &DateTimeUtc, notification_message: NotificationMessage, more_notifications: bool, available_sequence_numbers: Option>, results: Option>) -> PublishResponseEntry { + fn make_publish_response(&self, publish_request: PublishRequestEntry, subscription_id: u32, now: &DateTimeUtc, notification_message: NotificationMessage, more_notifications: bool, available_sequence_numbers: Option>) -> PublishResponseEntry { let now = DateTime::from(now.clone()); PublishResponseEntry { request_id: publish_request.request_id, @@ -319,7 +332,7 @@ impl Subscriptions { available_sequence_numbers, more_notifications, notification_message, - results, + results: publish_request.results, diagnostic_infos: None, }.into(), } diff --git a/server/src/tests/mod.rs b/server/src/tests/mod.rs index f1ea3505b..00d44789e 100644 --- a/server/src/tests/mod.rs +++ b/server/src/tests/mod.rs @@ -1,5 +1,4 @@ use std; -use std::collections::VecDeque; use std::path::PathBuf; use chrono; @@ -111,6 +110,7 @@ pub fn expired_publish_requests() { request_header: RequestHeader::new(&NodeId::null(), &now, 1000), subscription_acknowledgements: None, }, + results: None, }; pr1.request.request_header.timeout_hint = 5001; @@ -120,14 +120,17 @@ pub fn expired_publish_requests() { request_header: RequestHeader::new(&NodeId::null(), &now, 2000), subscription_acknowledgements: None, }, + results: None, }; pr2.request.request_header.timeout_hint = 3000; // Create session with publish requests let secure_channel: SecureChannel = (SecurityPolicy::None, MessageSecurityMode::None).into(); let mut session = Session::new_no_certificate_store(secure_channel); - session.subscriptions.publish_request_queue = { - let mut publish_request_queue = VecDeque::with_capacity(2); + + { + let publish_request_queue = session.subscriptions.publish_request_queue(); + publish_request_queue.clear(); publish_request_queue.push_back(pr1); publish_request_queue.push_back(pr2); publish_request_queue @@ -135,18 +138,28 @@ pub fn expired_publish_requests() { // Expire requests, see which expire session.expire_stale_publish_requests(&now_plus_5s); - let expired_responses = &session.subscriptions.publish_response_queue; + // The > 30s timeout hint request should be expired and the other should remain - assert_eq!(expired_responses.len(), 1); - assert_eq!(session.subscriptions.publish_request_queue.len(), 1); - assert_eq!(session.subscriptions.publish_request_queue[0].request.request_header.request_handle, 1000); - - let r1 = &expired_responses[0]; - if let SupportedMessage::ServiceFault(ref response_header) = r1.response { - assert_eq!(response_header.response_header.request_handle, 2000); - assert_eq!(response_header.response_header.service_result, StatusCode::BadTimeout); - } else { - panic!("Expected service faults for timed out publish requests") + + // Remain + { + let publish_request_queue = session.subscriptions.publish_request_queue(); + assert_eq!(publish_request_queue.len(), 1); + assert_eq!(publish_request_queue[0].request.request_header.request_handle, 1000); + } + + // Expire + { + let publish_response_queue = session.subscriptions.publish_response_queue(); + assert_eq!(publish_response_queue.len(), 1); + + let r1 = &publish_response_queue[0]; + if let SupportedMessage::ServiceFault(ref response_header) = r1.response { + assert_eq!(response_header.response_header.request_handle, 2000); + assert_eq!(response_header.response_header.service_result, StatusCode::BadTimeout); + } else { + panic!("Expected service faults for timed out publish requests") + } } } \ No newline at end of file From 1fb1786012f87a71bbf238338c0b1ecc4a75a487 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Sat, 6 Apr 2019 11:28:50 +0100 Subject: [PATCH 2/6] Display the proper value in the client. Add a stub for a server. --- 3rd-party/open62541/CMakelists.txt | 52 +++--- 3rd-party/open62541/client.cpp | 256 ++++++++++++++--------------- 3rd-party/open62541/server.cpp | 6 + 3 files changed, 160 insertions(+), 154 deletions(-) create mode 100644 3rd-party/open62541/server.cpp diff --git a/3rd-party/open62541/CMakelists.txt b/3rd-party/open62541/CMakelists.txt index 49bd8dc16..387d63e3b 100644 --- a/3rd-party/open62541/CMakelists.txt +++ b/3rd-party/open62541/CMakelists.txt @@ -1,25 +1,27 @@ -cmake_minimum_required(VERSION 3.0) - -project(open62541) - -add_subdirectory(libopen62541) - -include_directories( - ./libopen62541 -) - -add_executable(open62541-client - ./client.cpp - ) - -set(PLATFORM_LIBS) -if (WIN32) - set(PLATFORM_LIBS ws2_32) -endif () - -target_link_libraries(open62541-client - libopen62541 - ${PLATFORM_LIBS} - ) - -# add_executable(open62541-server) +cmake_minimum_required(VERSION 3.0) + +project(open62541) + +add_subdirectory(libopen62541) + +include_directories( + ./libopen62541 +) + +if (WIN32) + set(PLATFORM_LIBS ws2_32) +else () + set(PLATFORM_LIBS) +endif () + +add_executable(open62541-client ./client.cpp) +target_link_libraries(open62541-client + libopen62541 + ${PLATFORM_LIBS} + ) + +add_executable(open62541-server ./server.cpp) +target_link_libraries(open62541-server + libopen62541 + ${PLATFORM_LIBS} + ) \ No newline at end of file diff --git a/3rd-party/open62541/client.cpp b/3rd-party/open62541/client.cpp index e7599efc9..93dfcd63e 100644 --- a/3rd-party/open62541/client.cpp +++ b/3rd-party/open62541/client.cpp @@ -1,130 +1,128 @@ -// MODIFIED FROM open62541 example - -#ifdef _WIN32 - -#include -#include - -# define UA_sleep_ms(X) Sleep(X) -#else -# include -# define UA_sleep_ms(X) usleep(X * 1000) -#endif - -#include -#include - -#include "libopen62541/open62541.h" - -UA_Boolean running = true; -UA_Logger logger = UA_Log_Stdout; - -static void stopHandler(int sign) { - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Received Ctrl-C"); - running = 0; -} - -static void -handler_currentTimeChanged(UA_Client *client, UA_UInt32 subId, void *subContext, - UA_UInt32 monId, void *monContext, UA_DataValue *value) { - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "currentTime has changed!"); - if (UA_Variant_hasScalarType(&value->value, &UA_TYPES[UA_TYPES_DATETIME])) { - UA_DateTime raw_date = *(UA_DateTime *) value->value.data; - UA_DateTimeStruct dts = UA_DateTime_toStruct(raw_date); - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "date is: %02u-%02u-%04u %02u:%02u:%02u.%03u", - dts.day, dts.month, dts.year, dts.hour, dts.min, dts.sec, dts.milliSec); - } -} - -static void -deleteSubscriptionCallback(UA_Client *client, UA_UInt32 subscriptionId, void *subscriptionContext) { - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Subscription Id %u was deleted", subscriptionId); -} - -static void -subscriptionInactivityCallback(UA_Client *client, UA_UInt32 subId, void *subContext) { - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Inactivity for subscription %u", subId); -} - -static void -stateCallback(UA_Client *client, UA_ClientState clientState) { - switch (clientState) { - case UA_CLIENTSTATE_DISCONNECTED: - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "The client is disconnected"); - break; - case UA_CLIENTSTATE_CONNECTED: - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A TCP connection to the server is open"); - break; - case UA_CLIENTSTATE_SECURECHANNEL: - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A SecureChannel to the server is open"); - break; - case UA_CLIENTSTATE_SESSION: { - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A session with the server is open"); - /* A new session was created. We need to create the subscription. */ - /* Create a subscription */ - UA_CreateSubscriptionRequest request = UA_CreateSubscriptionRequest_default(); - UA_CreateSubscriptionResponse response = UA_Client_Subscriptions_create(client, request, - NULL, NULL, - deleteSubscriptionCallback); - - if (response.responseHeader.serviceResult == UA_STATUSCODE_GOOD) - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Create subscription succeeded, id %u", - response.subscriptionId); - else - return; - - /* Add a MonitoredItem */ - - char *node_id = const_cast("v1"); - - UA_MonitoredItemCreateRequest monRequest = - UA_MonitoredItemCreateRequest_default(UA_NODEID_STRING(2, node_id)); - - UA_MonitoredItemCreateResult monResponse = - UA_Client_MonitoredItems_createDataChange(client, response.subscriptionId, - UA_TIMESTAMPSTORETURN_BOTH, - monRequest, NULL, handler_currentTimeChanged, NULL); - if (monResponse.statusCode == UA_STATUSCODE_GOOD) - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, - "Monitoring ns=2;s=v1', id %u", monResponse.monitoredItemId); - } - break; - case UA_CLIENTSTATE_SESSION_RENEWED: - UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A session with the server is open (renewed)"); - /* The session was renewed. We don't need to recreate the subscription. */ - break; - } - return; -} - -int main(void) { - signal(SIGINT, stopHandler); /* catches ctrl-c */ - - UA_ClientConfig config = UA_ClientConfig_default; - /* Set stateCallback */ - config.stateCallback = stateCallback; - config.subscriptionInactivityCallback = subscriptionInactivityCallback; - - UA_Client *client = UA_Client_new(config); - - /* Endless loop runAsync */ - while (running) { - /* if already connected, this will return GOOD and do nothing */ - /* if the connection is closed/errored, the connection will be reset and then reconnected */ - /* Alternatively you can also use UA_Client_getState to get the current state */ - UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4855"); - if (retval != UA_STATUSCODE_GOOD) { - UA_LOG_ERROR(logger, UA_LOGCATEGORY_USERLAND, "Not connected. Retrying to connect in 1 second"); - /* The connect may timeout after 5 seconds (default timeout) or it may fail immediately on network errors */ - /* E.g. name resolution errors or unreachable network. Thus there should be a small sleep here */ - UA_sleep_ms(1000); - continue; - } - - UA_Client_runAsync(client, 1000); - }; - - /* Clean up */ - UA_Client_delete(client); /* Disconnects the client internally */ - return UA_STATUSCODE_GOOD; +// MODIFIED FROM open62541 example + +#ifdef _WIN32 + +#include +#include + +# define UA_sleep_ms(X) Sleep(X) +#else +# include +# define UA_sleep_ms(X) usleep(X * 1000) +#endif + +#include +#include + +#include "libopen62541/open62541.h" + +UA_Boolean running = true; +UA_Logger logger = UA_Log_Stdout; + +static void stopHandler(int sign) { + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Received Ctrl-C"); + running = 0; +} + +static void +handler_valueChanged(UA_Client *client, UA_UInt32 subId, void *subContext, + UA_UInt32 monId, void *monContext, UA_DataValue *value) { + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "value has changed!"); + if (UA_Variant_hasScalarType(&value->value, &UA_TYPES[UA_TYPES_INT32])) { + UA_Int32 rawValue = *(UA_Int32 *) value->value.data; + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "value is %d", rawValue); + } +} + +static void +deleteSubscriptionCallback(UA_Client *client, UA_UInt32 subscriptionId, void *subscriptionContext) { + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Subscription Id %u was deleted", subscriptionId); +} + +static void +subscriptionInactivityCallback(UA_Client *client, UA_UInt32 subId, void *subContext) { + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Inactivity for subscription %u", subId); +} + +static void +stateCallback(UA_Client *client, UA_ClientState clientState) { + switch (clientState) { + case UA_CLIENTSTATE_DISCONNECTED: + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "The client is disconnected"); + break; + case UA_CLIENTSTATE_CONNECTED: + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A TCP connection to the server is open"); + break; + case UA_CLIENTSTATE_SECURECHANNEL: + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A SecureChannel to the server is open"); + break; + case UA_CLIENTSTATE_SESSION: { + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A session with the server is open"); + /* A new session was created. We need to create the subscription. */ + /* Create a subscription */ + UA_CreateSubscriptionRequest request = UA_CreateSubscriptionRequest_default(); + UA_CreateSubscriptionResponse response = UA_Client_Subscriptions_create(client, request, + NULL, NULL, + deleteSubscriptionCallback); + + if (response.responseHeader.serviceResult == UA_STATUSCODE_GOOD) + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "Create subscription succeeded, id %u", + response.subscriptionId); + else + return; + + /* Add a MonitoredItem */ + + char *node_id = const_cast("v1"); + + UA_MonitoredItemCreateRequest monRequest = + UA_MonitoredItemCreateRequest_default(UA_NODEID_STRING(2, node_id)); + + UA_MonitoredItemCreateResult monResponse = + UA_Client_MonitoredItems_createDataChange(client, response.subscriptionId, + UA_TIMESTAMPSTORETURN_BOTH, + monRequest, NULL, handler_valueChanged, NULL); + if (monResponse.statusCode == UA_STATUSCODE_GOOD) + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, + "Monitoring ns=2;s=v1', id %u", monResponse.monitoredItemId); + } + break; + case UA_CLIENTSTATE_SESSION_RENEWED: + UA_LOG_INFO(logger, UA_LOGCATEGORY_USERLAND, "A session with the server is open (renewed)"); + /* The session was renewed. We don't need to recreate the subscription. */ + break; + } + return; +} + +int main(void) { + signal(SIGINT, stopHandler); /* catches ctrl-c */ + + UA_ClientConfig config = UA_ClientConfig_default; + /* Set stateCallback */ + config.stateCallback = stateCallback; + config.subscriptionInactivityCallback = subscriptionInactivityCallback; + + UA_Client *client = UA_Client_new(config); + + /* Endless loop runAsync */ + while (running) { + /* if already connected, this will return GOOD and do nothing */ + /* if the connection is closed/errored, the connection will be reset and then reconnected */ + /* Alternatively you can also use UA_Client_getState to get the current state */ + UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4855"); + if (retval != UA_STATUSCODE_GOOD) { + UA_LOG_ERROR(logger, UA_LOGCATEGORY_USERLAND, "Not connected. Retrying to connect in 1 second"); + /* The connect may timeout after 5 seconds (default timeout) or it may fail immediately on network errors */ + /* E.g. name resolution errors or unreachable network. Thus there should be a small sleep here */ + UA_sleep_ms(1000); + continue; + } + + UA_Client_runAsync(client, 1000); + }; + + /* Clean up */ + UA_Client_delete(client); /* Disconnects the client internally */ + return UA_STATUSCODE_GOOD; } \ No newline at end of file diff --git a/3rd-party/open62541/server.cpp b/3rd-party/open62541/server.cpp new file mode 100644 index 000000000..c1c36ea6e --- /dev/null +++ b/3rd-party/open62541/server.cpp @@ -0,0 +1,6 @@ +#include + +int main() { + std::cout << "Implement me\n"; + return 0; +} \ No newline at end of file From 0f10a3521eec2079424049e573872715522e21b5 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Sat, 6 Apr 2019 11:30:57 +0100 Subject: [PATCH 3/6] Clean up overly verbose code in crypto using .map, .map_err to transform return types --- core/src/crypto/aeskey.rs | 17 ++++---- core/src/crypto/certificate_store.rs | 10 ++--- core/src/crypto/hash.rs | 24 ++++++++---- core/src/crypto/pkey.rs | 26 ++++++------- core/src/crypto/security_policy.rs | 2 +- core/src/crypto/thumbprint.rs | 2 +- core/src/crypto/x509.rs | 58 ++++++++++++---------------- 7 files changed, 68 insertions(+), 71 deletions(-) diff --git a/core/src/crypto/aeskey.rs b/core/src/crypto/aeskey.rs index 9aef60edd..b76b1d218 100644 --- a/core/src/crypto/aeskey.rs +++ b/core/src/crypto/aeskey.rs @@ -72,14 +72,15 @@ impl AesKey { crypter.pad(false); let result = crypter.update(src, dst); if let Ok(count) = result { - let result = crypter.finalize(&mut dst[count..]); - if let Ok(rest) = result { - trace!("do cipher size {}", count + rest); - Ok(count + rest) - } else { - error!("Encryption error during finalize {:?}", result.unwrap_err()); - Err(StatusCode::BadUnexpectedError) - } + crypter.finalize(&mut dst[count..]) + .map(|rest| { + trace!("do cipher size {}", count + rest); + count + rest + }) + .map_err(|e| { + error!("Encryption error during finalize {:?}", e); + StatusCode::BadUnexpectedError + }) } else { error!("Encryption error during update {:?}", result.unwrap_err()); Err(StatusCode::BadUnexpectedError) diff --git a/core/src/crypto/certificate_store.rs b/core/src/crypto/certificate_store.rs index 4465aa386..6b09e2000 100644 --- a/core/src/crypto/certificate_store.rs +++ b/core/src/crypto/certificate_store.rs @@ -178,7 +178,7 @@ impl CertificateStore { builder.build() }; - Ok((X509::wrap(cert), PrivateKey::wrap_private_key(pkey))) + Ok((X509::from(cert), PrivateKey::wrap_private_key(pkey))) } /// Reads a private key from a path on disk. @@ -265,12 +265,12 @@ impl CertificateStore { /// or the test is assumed to fail. fn ensure_cert_and_file_are_the_same(cert: &X509, cert_path: &Path) -> bool { if !cert_path.exists() { - trace!("Can't find cert on disk"); + trace!("Cannot find cert on disk"); false } else { let cert2 = CertificateStore::read_cert(cert_path); if cert2.is_err() { - trace!("Can't read cert from disk {:?} - {}", cert_path, cert2.unwrap_err()); + trace!("Cannot read cert from disk {:?} - {}", cert_path, cert2.unwrap_err()); // No cert2 to compare to false } else { @@ -544,7 +544,7 @@ impl CertificateStore { return Err(format!("Could not read cert from cert file {}", path.display())); } - Ok(X509::wrap(cert.unwrap())) + Ok(X509::from(cert.unwrap())) } /// Makes a path @@ -560,7 +560,7 @@ impl CertificateStore { Ok(path) } - /// Writes to file or prints an error for the reason why it can't. + /// Writes to file or prints an error for the reason why it cannot. /// /// # Errors /// diff --git a/core/src/crypto/hash.rs b/core/src/crypto/hash.rs index 240dcc061..9b5ada22e 100644 --- a/core/src/crypto/hash.rs +++ b/core/src/crypto/hash.rs @@ -79,13 +79,17 @@ pub fn hmac_sha1(key: &[u8], data: &[u8], signature: &mut [u8]) -> Result<(), St /// Verify that the HMAC for the data block matches the supplied signature pub fn verify_hmac_sha1(key: &[u8], data: &[u8], signature: &[u8]) -> bool { - let mut tmp_signature = vec![0u8; SHA1_SIZE]; - if hmac_sha1(key, data, &mut tmp_signature).is_err() { + if signature.len() != SHA1_SIZE { false } else { - trace!("Original signature = {:?}", signature); - trace!("Calculated signature = {:?}", tmp_signature); - signature == &tmp_signature[..] + let mut tmp_signature = [0u8; SHA1_SIZE]; + if hmac_sha1(key, data, &mut tmp_signature).is_err() { + false + } else { + trace!("Original signature = {:?}", signature); + trace!("Calculated signature = {:?}", tmp_signature); + signature == &tmp_signature[..] + } } } @@ -103,10 +107,14 @@ pub fn hmac_sha256(key: &[u8], data: &[u8], signature: &mut [u8]) -> Result<(), /// Verify that the HMAC for the data block matches the supplied signature pub fn verify_hmac_sha256(key: &[u8], data: &[u8], signature: &[u8]) -> bool { - let mut tmp_signature = vec![0u8; SHA256_SIZE]; - if hmac_sha256(key, data, &mut tmp_signature).is_err() { + if signature.len() != SHA256_SIZE { false } else { - signature == &tmp_signature[..] + let mut tmp_signature = [0u8; SHA256_SIZE]; + if hmac_sha256(key, data, &mut tmp_signature).is_err() { + false + } else { + signature == &tmp_signature[..] + } } } diff --git a/core/src/crypto/pkey.rs b/core/src/crypto/pkey.rs index f426e161e..f6430e9ea 100644 --- a/core/src/crypto/pkey.rs +++ b/core/src/crypto/pkey.rs @@ -76,7 +76,6 @@ pub trait KeySize { } } - impl KeySize for PrivateKey { /// Length in bits fn bit_length(&self) -> usize { @@ -99,21 +98,18 @@ impl PrivateKey { } pub fn from_pem(pem: &[u8]) -> Result { - if let Ok(value) = pkey::PKey::private_key_from_pem(pem) { - Ok(PKey { value }) - } else { - error!("Cannot produce a private key from the data supplied"); - Err(()) - } + pkey::PKey::private_key_from_pem(pem) + .map(|value| PKey { value }) + .map_err(|_| { + error!("Cannot produce a private key from the data supplied"); + }) } pub fn private_key_to_pem(&self) -> Result, ()> { - if let Ok(pem) = self.value.private_key_to_pem_pkcs8() { - Ok(pem) - } else { - error!("Cannot turn private key to PEM"); - Err(()) - } + self.value.private_key_to_pem_pkcs8() + .map_err(|_| { + error!("Cannot turn private key to PEM"); + }) } /// Creates a message digest from the specified block of data and then signs it to return a signature @@ -128,7 +124,7 @@ impl PrivateKey { signature.copy_from_slice(&result); return Ok(result.len()); } else { - debug!("Can't sign data - error = {:?}", result.unwrap_err()); + debug!("Cannot sign data - error = {:?}", result.unwrap_err()); } } } @@ -195,7 +191,7 @@ impl PublicKey { trace!("Key verified = {:?}", result); return Ok(result); } else { - debug!("Can't verify key - error = {:?}", result.unwrap_err()); + debug!("Cannot verify key - error = {:?}", result.unwrap_err()); } } } diff --git a/core/src/crypto/security_policy.rs b/core/src/crypto/security_policy.rs index 9b8251309..837654e01 100644 --- a/core/src/crypto/security_policy.rs +++ b/core/src/crypto/security_policy.rs @@ -334,7 +334,7 @@ impl SecurityPolicy { SecurityPolicy::Basic256 | SecurityPolicy::Basic256Sha256 => ByteString::random(self.symmetric_key_size()), _ => { - panic!("Can't make a nonce because key size is unknown"); + panic!("Cannot make a nonce because key size is unknown"); } } } diff --git a/core/src/crypto/thumbprint.rs b/core/src/crypto/thumbprint.rs index e56267712..e66833a5c 100644 --- a/core/src/crypto/thumbprint.rs +++ b/core/src/crypto/thumbprint.rs @@ -24,7 +24,7 @@ impl Thumbprint { if digest.len() != Thumbprint::THUMBPRINT_SIZE { panic!("Thumbprint is not the right length"); } - let mut value: [u8; Thumbprint::THUMBPRINT_SIZE] = Default::default(); + let mut value = [0u8; Thumbprint::THUMBPRINT_SIZE]; value.clone_from_slice(digest); Thumbprint { value } } diff --git a/core/src/crypto/x509.rs b/core/src/crypto/x509.rs index d3620ee7d..55d9a42d6 100644 --- a/core/src/crypto/x509.rs +++ b/core/src/crypto/x509.rs @@ -98,7 +98,6 @@ impl X509Data { } } - /// This is a wrapper around the `OpenSSL` `X509` cert #[derive(Clone)] pub struct X509 { @@ -118,28 +117,29 @@ unsafe impl Send for X509 {} unsafe impl std::marker::Sync for X509 {} -impl X509 { - pub fn wrap(value: x509::X509) -> X509 { - X509 { value } +impl From for X509 { + fn from(value: x509::X509) -> Self { + Self { value } } +} +impl X509 { pub fn from_der(der: &[u8]) -> Result { - if let Ok(value) = x509::X509::from_der(der) { - Ok(X509 { value }) - } else { - error!("Cannot produce an x509 cert from the data supplied"); - Err(()) - } + x509::X509::from_der(der) + .map(|value| X509::from(value)) + .map_err(|_| { + error!("Cannot produce an x509 cert from the data supplied"); + }) } pub fn from_byte_string(data: &ByteString) -> Result { if data.is_null() { - error!("Can't make certificate from null bytestring"); + error!("Cannot make certificate from null bytestring"); Err(StatusCode::BadCertificateInvalid) } else if let Ok(cert) = x509::X509::from_der(&data.value.as_ref().unwrap()) { - Ok(X509::wrap(cert)) + Ok(X509::from(cert)) } else { - error!("Can't make certificate, does bytestring contain .der?"); + error!("Cannot make certificate, does bytestring contain .der?"); Err(StatusCode::BadCertificateInvalid) } } @@ -151,13 +151,12 @@ impl X509 { } pub fn public_key(&self) -> Result { - if let Ok(pkey) = self.value.public_key() { - let pkey = PublicKey::wrap_public_key(pkey); - Ok(pkey) - } else { - error!("Can't obtain public key from certificate"); - Err(StatusCode::BadCertificateInvalid) - } + self.value.public_key() + .map(|pkey| PublicKey::wrap_public_key(pkey)) + .map_err(|_| { + error!("Cannot obtain public key from certificate"); + StatusCode::BadCertificateInvalid + }) } fn get_subject_entry(&self, nid: Nid) -> Result { @@ -298,12 +297,9 @@ impl X509 { } pub fn to_der(&self) -> Result, ()> { - if let Ok(der) = self.value.to_der() { - Ok(der) - } else { - error!("Cannot turn X509 cert to DER"); - Err(()) - } + self.value.to_der().map_err(|e| { + error!("Cannot turn X509 cert to DER, err = {:?}", e); + }) } fn parse_asn1_date(date: &str) -> Result, ()> { @@ -315,13 +311,9 @@ impl X509 { } else { &date }; - let result = Utc.datetime_from_str(date, "%b %d %H:%M:%S %Y"); - if result.is_err() { - error!("Error = {:?}", result.unwrap_err()); - Err(()) - } else { - Ok(result.unwrap()) - } + Utc.datetime_from_str(date, "%b %d %H:%M:%S %Y").map_err(|e| { + error!("Cannot parse ASN1 date, err = {:?}", e); + }) } } From b8cb15e6250aa1e3a59c34edafbdc984e72df5c1 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Sat, 6 Apr 2019 11:32:40 +0100 Subject: [PATCH 4/6] Make service fault debug info slightly more informative of the cause of failure. It should got further and name the precise service call rather than the set though. --- server/src/services/attribute.rs | 4 +++- server/src/services/discovery.rs | 4 +++- server/src/services/method.rs | 4 +++- server/src/services/mod.rs | 5 ++++- server/src/services/monitored_item.rs | 4 +++- server/src/services/node_management.rs | 4 +++- server/src/services/session.rs | 4 +++- server/src/services/subscription.rs | 4 +++- server/src/services/view.rs | 4 +++- 9 files changed, 28 insertions(+), 9 deletions(-) diff --git a/server/src/services/attribute.rs b/server/src/services/attribute.rs index af9b07247..dc161a782 100644 --- a/server/src/services/attribute.rs +++ b/server/src/services/attribute.rs @@ -12,7 +12,9 @@ use crate::{ /// The attribute service. Allows attributes to be read and written from the address space. pub(crate) struct AttributeService; -impl Service for AttributeService {} +impl Service for AttributeService { + fn name(&self) -> String { String::from("AttributeService") } +} impl AttributeService { pub fn new() -> AttributeService { diff --git a/server/src/services/discovery.rs b/server/src/services/discovery.rs index 5a812ac16..615f79ec3 100644 --- a/server/src/services/discovery.rs +++ b/server/src/services/discovery.rs @@ -9,7 +9,9 @@ use crate::{state::ServerState, services::Service}; /// The discovery service. Allows a server to return the endpoints that it supports. pub(crate) struct DiscoveryService; -impl Service for DiscoveryService {} +impl Service for DiscoveryService { + fn name(&self) -> String { String::from("DiscoveryService") } +} impl DiscoveryService { pub fn new() -> DiscoveryService { diff --git a/server/src/services/method.rs b/server/src/services/method.rs index 27b2cb549..8a4e766d2 100644 --- a/server/src/services/method.rs +++ b/server/src/services/method.rs @@ -14,7 +14,9 @@ use crate::{ /// The method service. Allows a client to call a method on the server. pub(crate) struct MethodService; -impl Service for MethodService {} +impl Service for MethodService { + fn name(&self) -> String { String::from("MethodService") } +} impl MethodService { pub fn new() -> MethodService { diff --git a/server/src/services/mod.rs b/server/src/services/mod.rs index 2d0577248..7ba4ce7a8 100644 --- a/server/src/services/mod.rs +++ b/server/src/services/mod.rs @@ -4,9 +4,12 @@ use opcua_types::status_code::StatusCode; pub mod message_handler; +/// The implementation of a service, or a set of services will implement this trait trait Service { + fn name(&self) -> String; + fn service_fault(&self, request_header: &RequestHeader, service_result: StatusCode) -> SupportedMessage { - warn!("Service fault with status code {} is being created", service_result); + warn!("Service {}, request {} generated a service fault with status code {}", self.name(), request_header.request_handle, service_result); ServiceFault::new_supported_message(request_header, service_result) } } diff --git a/server/src/services/monitored_item.rs b/server/src/services/monitored_item.rs index 3799c1af7..1ab19e80f 100644 --- a/server/src/services/monitored_item.rs +++ b/server/src/services/monitored_item.rs @@ -9,7 +9,9 @@ use crate::{session::Session, services::Service}; /// The monitored item service. Allows client to create, modify and delete monitored items on a subscription. pub(crate) struct MonitoredItemService; -impl Service for MonitoredItemService {} +impl Service for MonitoredItemService { + fn name(&self) -> String { String::from("MonitoredItemService") } +} impl MonitoredItemService { pub fn new() -> MonitoredItemService { diff --git a/server/src/services/node_management.rs b/server/src/services/node_management.rs index f48dabfe1..9b5f44aa9 100644 --- a/server/src/services/node_management.rs +++ b/server/src/services/node_management.rs @@ -20,7 +20,9 @@ use crate::{ pub(crate) struct NodeManagementService; -impl Service for NodeManagementService {} +impl Service for NodeManagementService { + fn name(&self) -> String { String::from("NodeManagementService") } +} impl NodeManagementService { pub fn new() -> NodeManagementService { diff --git a/server/src/services/session.rs b/server/src/services/session.rs index 460992f69..6702b5a1e 100644 --- a/server/src/services/session.rs +++ b/server/src/services/session.rs @@ -18,7 +18,9 @@ use crate::{ /// The session service. Allows the client to create, activate and close an authenticated session with the server. pub(crate) struct SessionService; -impl Service for SessionService {} +impl Service for SessionService { + fn name(&self) -> String { String::from("SessionService") } +} impl SessionService { pub fn new() -> SessionService { diff --git a/server/src/services/subscription.rs b/server/src/services/subscription.rs index 7fe9957de..c2ceace02 100644 --- a/server/src/services/subscription.rs +++ b/server/src/services/subscription.rs @@ -15,7 +15,9 @@ use crate::{ /// on the server and to request publish of notifications. pub(crate) struct SubscriptionService; -impl Service for SubscriptionService {} +impl Service for SubscriptionService { + fn name(&self) -> String { String::from("SubscriptionService") } +} impl SubscriptionService { pub fn new() -> SubscriptionService { diff --git a/server/src/services/view.rs b/server/src/services/view.rs index 86df95284..4602f5009 100644 --- a/server/src/services/view.rs +++ b/server/src/services/view.rs @@ -30,7 +30,9 @@ bitflags! { /// The view service. Allows the client to browse the address space of the server. pub(crate) struct ViewService; -impl Service for ViewService {} +impl Service for ViewService { + fn name(&self) -> String { String::from("ViewService") } +} impl ViewService { pub fn new() -> ViewService { From 0994fdbb5585b116d79510b47b0cb4e735cee540 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Sat, 6 Apr 2019 11:33:57 +0100 Subject: [PATCH 5/6] Separate mandatory attributes into a variable - perhaps will make a function on the trait next. --- server/src/address_space/method.rs | 3 +- server/src/address_space/object.rs | 4 ++- server/src/address_space/object_type.rs | 3 +- server/src/address_space/reference_type.rs | 3 +- server/src/address_space/variable.rs | 5 ++-- server/src/address_space/variable_type.rs | 5 ++-- server/src/address_space/view.rs | 35 ++++++++++------------ 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/server/src/address_space/method.rs b/server/src/address_space/method.rs index ad0ae1d5f..5bb003622 100644 --- a/server/src/address_space/method.rs +++ b/server/src/address_space/method.rs @@ -41,8 +41,9 @@ impl Method { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: MethodAttributes) -> Result where S: Into { + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::EXECUTABLE | AttributesMask::USER_EXECUTABLE; let mask = AttributesMask::from_bits(attributes.specified_attributes).ok_or(())?; - if mask.contains(AttributesMask::DISPLAY_NAME | AttributesMask::EXECUTABLE | AttributesMask::USER_EXECUTABLE) { + if mask.contains(mandatory_attributes) { let mut node = Self::new(node_id, browse_name, attributes.display_name, attributes.executable, attributes.user_executable); if mask.contains(AttributesMask::DESCRIPTION) { node.set_description(attributes.description); diff --git a/server/src/address_space/object.rs b/server/src/address_space/object.rs index 6a275ae66..b1c1ce92b 100644 --- a/server/src/address_space/object.rs +++ b/server/src/address_space/object.rs @@ -26,8 +26,10 @@ impl Object { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: ObjectAttributes) -> Result where S: Into { + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::EVENT_NOTIFIER; + let mask = AttributesMask::from_bits(attributes.specified_attributes).ok_or(())?; - if mask.contains(AttributesMask::DISPLAY_NAME | AttributesMask::EVENT_NOTIFIER) { + if mask.contains(mandatory_attributes) { let mut node = Self::new(node_id, browse_name, attributes.display_name, attributes.event_notifier); if mask.contains(AttributesMask::DESCRIPTION) { node.set_description(attributes.description); diff --git a/server/src/address_space/object_type.rs b/server/src/address_space/object_type.rs index 93e77cc99..d4a203b57 100644 --- a/server/src/address_space/object_type.rs +++ b/server/src/address_space/object_type.rs @@ -26,8 +26,9 @@ impl ObjectType { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: ObjectTypeAttributes) -> Result where S: Into { + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::IS_ABSTRACT; let mask = AttributesMask::from_bits(attributes.specified_attributes).ok_or(())?; - if mask.contains(AttributesMask::DISPLAY_NAME | AttributesMask::IS_ABSTRACT) { + if mask.contains(mandatory_attributes) { let mut node = Self::new(node_id, browse_name, attributes.display_name, attributes.is_abstract); if mask.contains(AttributesMask::DESCRIPTION) { node.set_description(attributes.description); diff --git a/server/src/address_space/reference_type.rs b/server/src/address_space/reference_type.rs index 8ea48d560..567e89c76 100644 --- a/server/src/address_space/reference_type.rs +++ b/server/src/address_space/reference_type.rs @@ -31,8 +31,9 @@ impl ReferenceType { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: ReferenceTypeAttributes) -> Result where S: Into { + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::IS_ABSTRACT | AttributesMask::SYMMETRIC; let mask = AttributesMask::from_bits(attributes.specified_attributes).ok_or(())?; - if mask.contains(AttributesMask::DISPLAY_NAME | AttributesMask::IS_ABSTRACT | AttributesMask::SYMMETRIC) { + if mask.contains(mandatory_attributes) { let mut node = Self::new(node_id, browse_name, attributes.display_name, None, false, false); if mask.contains(AttributesMask::DESCRIPTION) { node.set_description(attributes.description); diff --git a/server/src/address_space/variable.rs b/server/src/address_space/variable.rs index ce00d2c87..b18463ae5 100644 --- a/server/src/address_space/variable.rs +++ b/server/src/address_space/variable.rs @@ -152,9 +152,10 @@ impl Variable { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: VariableAttributes) -> Result where S: Into { + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::ACCESS_LEVEL | AttributesMask::USER_ACCESS_LEVEL | + AttributesMask::DATA_TYPE | AttributesMask::HISTORIZING | AttributesMask::VALUE | AttributesMask::VALUE_RANK; let mask = AttributesMask::from_bits(attributes.specified_attributes).ok_or(())?; - if mask.contains(AttributesMask::DISPLAY_NAME | AttributesMask::ACCESS_LEVEL | AttributesMask::USER_ACCESS_LEVEL | - AttributesMask::DATA_TYPE | AttributesMask::HISTORIZING | AttributesMask::VALUE | AttributesMask::VALUE_RANK) { + if mask.contains(mandatory_attributes) { let mut node = Self::new_data_value(node_id, browse_name, attributes.display_name, attributes.data_type, attributes.value.into()); node.set_value_rank(attributes.value_rank); node.set_historizing(attributes.historizing); diff --git a/server/src/address_space/variable_type.rs b/server/src/address_space/variable_type.rs index 72dd2061f..81d7952c9 100644 --- a/server/src/address_space/variable_type.rs +++ b/server/src/address_space/variable_type.rs @@ -32,9 +32,10 @@ impl VariableType { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: VariableTypeAttributes) -> Result where S: Into { + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::IS_ABSTRACT | + AttributesMask::DATA_TYPE | AttributesMask::VALUE_RANK; let mask = AttributesMask::from_bits(attributes.specified_attributes).ok_or(())?; - if mask.contains(AttributesMask::DISPLAY_NAME | AttributesMask::IS_ABSTRACT | - AttributesMask::DATA_TYPE | AttributesMask::VALUE_RANK) { + if mask.contains(mandatory_attributes) { let mut node = Self::new(node_id, browse_name, attributes.display_name, attributes.data_type, attributes.is_abstract, attributes.value_rank); if mask.contains(AttributesMask::DESCRIPTION) { diff --git a/server/src/address_space/view.rs b/server/src/address_space/view.rs index f30478b93..22be33a8f 100644 --- a/server/src/address_space/view.rs +++ b/server/src/address_space/view.rs @@ -27,27 +27,24 @@ impl View { pub fn from_attributes(node_id: &NodeId, browse_name: S, attributes: ViewAttributes) -> Result where S: Into { - let mut node = Self::new(node_id, browse_name, "", 0u8, false); + let mandatory_attributes = AttributesMask::DISPLAY_NAME | AttributesMask::EVENT_NOTIFIER | AttributesMask::CONTAINS_NO_LOOPS; let mask = AttributesMask::from_bits_truncate(attributes.specified_attributes); - if mask.contains(AttributesMask::DISPLAY_NAME) { - node.set_display_name(attributes.display_name); + if mask.contains(mandatory_attributes) { + let mut node = Self::new(node_id, browse_name, attributes.display_name, attributes.event_notifier, attributes.contains_no_loops); + if mask.contains(AttributesMask::DESCRIPTION) { + node.set_description(attributes.description); + } + if mask.contains(AttributesMask::WRITE_MASK) { + node.set_write_mask(WriteMask::from_bits_truncate(attributes.write_mask)); + } + if mask.contains(AttributesMask::USER_WRITE_MASK) { + node.set_user_write_mask(WriteMask::from_bits_truncate(attributes.user_write_mask)); + } + Ok(node) + } else { + error!("View cannot be created from attributes - missing mandatory values"); + Err(()) } - if mask.contains(AttributesMask::DESCRIPTION) { - node.set_description(attributes.description); - } - if mask.contains(AttributesMask::WRITE_MASK) { - node.set_write_mask(WriteMask::from_bits_truncate(attributes.write_mask)); - } - if mask.contains(AttributesMask::USER_WRITE_MASK) { - node.set_user_write_mask(WriteMask::from_bits_truncate(attributes.user_write_mask)); - } - if mask.contains(AttributesMask::EVENT_NOTIFIER) { - node.set_event_notifier(attributes.event_notifier); - } - if mask.contains(AttributesMask::CONTAINS_NO_LOOPS) { - node.set_contains_no_loops(attributes.contains_no_loops); - } - Ok(node) } pub fn event_notifier(&self) -> bool { From 3f2549a9194db3d4701f29d7088f9028bd1d9167 Mon Sep 17 00:00:00 2001 From: Adam Lock Date: Sat, 6 Apr 2019 11:34:50 +0100 Subject: [PATCH 6/6] Replace "can't" with "cannot" in some logging --- client/src/client.rs | 6 +++--- core/src/comms/chunker.rs | 2 +- core/src/comms/message_chunk_info.rs | 4 ++-- samples/discovery-client/src/main.rs | 2 +- server/src/comms/tcp_transport.rs | 2 +- server/src/subscriptions/monitored_item.rs | 2 +- server/src/subscriptions/subscriptions.rs | 4 ++-- types/src/variant.rs | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/client/src/client.rs b/client/src/client.rs index 1664f3e60..f0df1d019 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -160,7 +160,7 @@ impl Client { // Ask the server associated with the default endpoint for its list of endpoints let endpoints = match self.get_server_endpoints() { Result::Err(status_code) => { - error!("Can't get endpoints for server, error - {}", status_code); + error!("Cannot get endpoints for server, error - {}", status_code); return Err(status_code); } Result::Ok(endpoints) => endpoints @@ -449,7 +449,7 @@ impl Client { Err(StatusCode::BadUnexpectedError) } } else { - error!("Can't find an endpoint that we call register server on"); + error!("Cannot find an endpoint that we call register server on"); Err(StatusCode::BadUnexpectedError) } } @@ -535,7 +535,7 @@ impl Client { security_mode: MessageSecurityMode) -> Option { if security_policy == SecurityPolicy::Unknown { - panic!("Can't match against unknown security policy"); + panic!("Cannot match against unknown security policy"); } let matching_endpoint = endpoints.iter().find(|e| { diff --git a/core/src/comms/chunker.rs b/core/src/comms/chunker.rs index a9d395072..e5f76e4a9 100644 --- a/core/src/comms/chunker.rs +++ b/core/src/comms/chunker.rs @@ -195,7 +195,7 @@ impl Chunker { } } Err(err) => { - debug!("Can't decode message {:?}, err = {:?}", object_id, err); + debug!("Cannot decode message {:?}, err = {:?}", object_id, err); Err(StatusCode::BadServiceUnsupported) } } diff --git a/core/src/comms/message_chunk_info.rs b/core/src/comms/message_chunk_info.rs index 88751c8bc..8c6d0198d 100644 --- a/core/src/comms/message_chunk_info.rs +++ b/core/src/comms/message_chunk_info.rs @@ -46,7 +46,7 @@ impl ChunkInfo { let security_header = if chunk.is_open_secure_channel(&decoding_limits) { let result = AsymmetricSecurityHeader::decode(&mut stream, &decoding_limits); if result.is_err() { - error!("chunk_info() can't decode asymmetric security_header, {}", result.unwrap_err()); + error!("chunk_info() cannot decode asymmetric security_header, {}", result.unwrap_err()); return Err(StatusCode::BadCommunicationError); } let security_header = result.unwrap(); @@ -67,7 +67,7 @@ impl ChunkInfo { } else { let result = SymmetricSecurityHeader::decode(&mut stream, &decoding_limits); if result.is_err() { - error!("chunk_info() can't decode symmetric security_header, {}", result.unwrap_err()); + error!("chunk_info() cannot decode symmetric security_header, {}", result.unwrap_err()); return Err(StatusCode::BadCommunicationError); } SecurityHeader::Symmetric(result.unwrap()) diff --git a/samples/discovery-client/src/main.rs b/samples/discovery-client/src/main.rs index 5ecd986f9..ce1a7aa68 100644 --- a/samples/discovery-client/src/main.rs +++ b/samples/discovery-client/src/main.rs @@ -43,7 +43,7 @@ fn main() { // Ask the server associated with the default endpoint for its list of endpoints match client.get_server_endpoints_from_url(discovery_url.as_ref()) { Result::Err(status_code) => { - println!(" ERROR: Can't get endpoints for this server url, error - {}", status_code); + println!(" ERROR: Cannot get endpoints for this server url, error - {}", status_code); continue; } Result::Ok(endpoints) => { diff --git a/server/src/comms/tcp_transport.rs b/server/src/comms/tcp_transport.rs index a4761d681..043b02512 100644 --- a/server/src/comms/tcp_transport.rs +++ b/server/src/comms/tcp_transport.rs @@ -637,7 +637,7 @@ impl TcpTransport { if let Some(publish_responses) = session.subscriptions.take_publish_responses() { match subscription_tx.unbounded_send(SubscriptionEvent::PublishResponses(publish_responses)) { Err(error) => { - error!("Can't send publish responses, err = {}", error); + error!("Cannot send publish responses, err = {}", error); } Ok(_) => { trace!("Sent publish responses to session task"); diff --git a/server/src/subscriptions/monitored_item.rs b/server/src/subscriptions/monitored_item.rs index b52b54ede..cbf8547c6 100644 --- a/server/src/subscriptions/monitored_item.rs +++ b/server/src/subscriptions/monitored_item.rs @@ -255,7 +255,7 @@ impl MonitoredItem { false } } else { - trace!("Can't find item to monitor, node {:?}", self.item_to_monitor.node_id); + trace!("Cannot find item to monitor, node {:?}", self.item_to_monitor.node_id); false } } diff --git a/server/src/subscriptions/subscriptions.rs b/server/src/subscriptions/subscriptions.rs index e36579522..ea211f58e 100644 --- a/server/src/subscriptions/subscriptions.rs +++ b/server/src/subscriptions/subscriptions.rs @@ -282,11 +282,11 @@ impl Subscriptions { debug!("Removing subscription {} sequence number {} from retransmission queue", subscription_id, sequence_number); StatusCode::Good } else { - error!("Can't find acknowledged notification with sequence number {}", sequence_number); + error!("Cannot find acknowledged notification with sequence number {}", sequence_number); StatusCode::BadSequenceNumberUnknown } } else { - error!("Can't find acknowledged notification subscription id {}", subscription_id); + error!("Cannot find acknowledged notification subscription id {}", subscription_id); StatusCode::BadSubscriptionIdInvalid } }) diff --git a/types/src/variant.rs b/types/src/variant.rs index 68f863928..6b3dd47a1 100644 --- a/types/src/variant.rs +++ b/types/src/variant.rs @@ -912,7 +912,7 @@ impl Variant { Variant::DataValue(_) => Some(DataTypeId::DataValue), Variant::Array(ref values) => { if values.is_empty() { - error!("Can't get the data type of an empty array"); + error!("Cannot get the data type of an empty array"); None } else { values[0].data_type() @@ -920,7 +920,7 @@ impl Variant { } Variant::MultiDimensionArray(ref mda) => { if mda.values.is_empty() { - error!("Can't get the data type of an empty array"); + error!("Cannot get the data type of an empty array"); None } else { mda.values[0].data_type()