Skip to content

When decoding arrays is the "len" a promise or a hint? #212

Open
@laumann

Description

@laumann

I'm working with a somewhat special OPC-UA server, and having some problems decoding arrays returning.

I've messed around with debugging and have managed to produce this log snippet:

2022-06-02T09:54:01.905208721+00:00: DEBUG: /omnioiot/opcua/client/src/message_queue.rs:101: Response to Request 788 has been stored
2022-06-02T09:54:01.905377070+00:00: DEBUG: /omnioiot/opcua/client/src/session/session.rs:2197: session:5 browse, success
2022-06-02T09:54:01.905674093+00:00: DEBUG: /omnioiot/opcua/client/src/message_queue.rs:51: Request 789 was processed by the server
2022-06-02T09:54:01.920895280+00:00: DEBUG: /omnioiot/opcua/client/src/comms/tcp_transport.rs:106: receive chunk intermediate 1789:839
2022-06-02T09:54:01.921120572+00:00: DEBUG: /omnioiot/opcua/core/src/comms/chunker.rs:299: Decoded node id / object id of BrowseResponse_Encoding_DefaultBinary
2022-06-02T09:54:01.921182136+00:00: DEBUG: /omnioiot/opcua/core/src/supported_message.rs:281: decoding object_id BrowseResponse_Encoding_DefaultBinary
2022-06-02T09:54:01.921230660+00:00: DEBUG: /omnioiot/opcua/types/src/encoding.rs:135: decoding array of [0]opcua_types::string::UAString
2022-06-02T09:54:01.921274461+00:00: DEBUG: /omnioiot/opcua/types/src/service_types/browse_response.rs:55: response_header = ResponseHeader { timestamp: DateTime { date_time: 2022-06-02T09:55:19.381129200Z }, request_handle: 789, service_result: HISTORICAL_RAW | Good, service_diagnostics: DiagnosticInfo { symbolic_id: None, namespace_uri: None, locale: None, localized_text: None, additional_info: None, inner_status_code: None, inner_diagnostic_info: None }, string_table: Some([]), additional_header: ExtensionObject { node_id: NodeId { namespace: 0, identifier: Numeric(0) }, body: None } }
2022-06-02T09:54:01.921679179+00:00: DEBUG: /omnioiot/opcua/types/src/encoding.rs:135: decoding array of [1]opcua_types::service_types::browse_result::BrowseResult
2022-06-02T09:54:01.921726744+00:00: DEBUG: /omnioiot/opcua/types/src/encoding.rs:135: decoding array of [1000]opcua_types::service_types::reference_description::ReferenceDescription
2022-06-02T09:54:01.923009814+00:00: DEBUG: /omnioiot/opcua/types/src/encoding.rs:98: Decoding error - Error { kind: UnexpectedEof, message: "failed to fill whole buffer" }
2022-06-02T09:54:01.923120810+00:00: DEBUG: /omnioiot/opcua/types/src/encoding.rs:158: Decoded 891 items (expected 1000), before failure: IS_ERROR | BadUnexpectedError | BadInternalError | BadOutOfMemory | BadResourceUnavailable | BadCommunicationError | BadEncodingError | BadDecodingError
2022-06-02T09:54:02.396905716+00:00: DEBUG: /omnioiot/opcua/types/src/encoding.rs:135: decoding array of [759693312]opcua_types::diagnostic_info::DiagnosticInfo
memory allocation of 60775464960 bytes failed
Aborted (core dumped)

I'm suspecting that the decoded array length is more of a hint than the necessarily the length of the array for two reasons:

  • The fact that 891 items are successfully decoded before EOF is encountered
  • The length of 759693312 just seems outlandish (= 0x2d480000)

A few notes:

  • The patched code is not based on master but the v0.9, but AFAICT the decoding logic for arrays hasn't changed
  • I made a small change where, instead of wanting to decode exactly len array items, any errors caught would just break out of the decoding loop.
  • We run with the max possible decoding limits (i32::MAX as usize)
  • The OPC-UA server in question is Kepware 5

The patch in question:

diff --git a/core/src/comms/chunker.rs b/core/src/comms/chunker.rs
index 15241a6a..b6d088c2 100644
--- a/core/src/comms/chunker.rs
+++ b/core/src/comms/chunker.rs
@@ -296,7 +296,7 @@ impl Chunker {
                     StatusCode::BadUnexpectedError
                 })
                 .map(|object_id| {
-                    trace!("Decoded node id / object id of {:?}", object_id);
+                    debug!("Decoded node id / object id of {:?}", object_id);
                     object_id
                 })
         }
diff --git a/core/src/supported_message.rs b/core/src/supported_message.rs
index 33617e9c..78971fa3 100644
--- a/core/src/supported_message.rs
+++ b/core/src/supported_message.rs
@@ -278,7 +278,7 @@ impl SupportedMessage {
         object_id: ObjectId,
         decoding_options: &DecodingOptions,
     ) -> EncodingResult<Self> {
-        trace!("decoding object_id {:?}", object_id);
+        debug!("decoding object_id {:?}", object_id);
         let decoded_message = match object_id {
             ObjectId::ServiceFault_Encoding_DefaultBinary => {
                 ServiceFault::decode(stream, decoding_options)?.into()
diff --git a/types/src/encoding.rs b/types/src/encoding.rs
index 21d8f39e..a5eb1a1e 100644
--- a/types/src/encoding.rs
+++ b/types/src/encoding.rs
@@ -95,7 +95,7 @@ where
     T: Debug,
 {
     result.map_err(|err| {
-        trace!("Decoding error - {:?}", err);
+        debug!("Decoding error - {:?}", err);
         StatusCode::BadDecodingError
     })
 }
@@ -132,6 +132,7 @@ pub fn read_array<S: Read, T: BinaryEncoder<T>>(
     decoding_options: &DecodingOptions,
 ) -> EncodingResult<Option<Vec<T>>> {
     let len = read_i32(stream)?;
+    debug!("decoding array of [{}]{}", len, std::any::type_name::<T>());
     if len == -1 {
         Ok(None)
     } else if len < -1 {
@@ -145,8 +146,19 @@ pub fn read_array<S: Read, T: BinaryEncoder<T>>(
         Err(StatusCode::BadDecodingError)
     } else {
         let mut values: Vec<T> = Vec::with_capacity(len as usize);
+        let mut count = 0;
         for _ in 0..len {
-            values.push(T::decode(stream, decoding_options)?);
+            //values.push(T::decode(stream, decoding_options)?);
+            match T::decode(stream, decoding_options) {
+                Ok(val) => {
+                    count += 1;
+                    values.push(val);
+                }
+                Err(err) => {
+                    debug!("Decoded {} items (expected {}), before failure: {:?}", count, len, err);
+                    break;
+                }
+            }
         }
         Ok(Some(values))
     }
diff --git a/types/src/service_types/browse_response.rs b/types/src/service_types/browse_response.rs
index 11b24635..bdcc80d0 100644
--- a/types/src/service_types/browse_response.rs
+++ b/types/src/service_types/browse_response.rs
@@ -52,8 +52,11 @@ impl BinaryEncoder<BrowseResponse> for BrowseResponse {
     #[allow(unused_variables)]
     fn decode<S: Read>(stream: &mut S, decoding_options: &DecodingOptions) -> EncodingResult<Self> {
         let response_header = ResponseHeader::decode(stream, decoding_options)?;
+        debug!("response_header = {:?}", response_header);
         let results: Option<Vec<BrowseResult>> = read_array(stream, decoding_options)?;
+        debug!("results = {:?}", results);
         let diagnostic_infos: Option<Vec<DiagnosticInfo>> = read_array(stream, decoding_options)?;
+        debug!("diagnostic_infos = {:?}", diagnostic_infos);
         Ok(BrowseResponse {
             response_header,
             results,
-- 
2.35.1

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions