You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by we...@apache.org on 2019/04/23 19:50:44 UTC

[mynewt-core] branch master updated: sys/log/full: Allow for limiting size of newtmgr log response (#1778)

This is an automated email from the ASF dual-hosted git repository.

wes3 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git


The following commit(s) were added to refs/heads/master by this push:
     new 466b10c  sys/log/full: Allow for limiting size of newtmgr log response (#1778)
466b10c is described below

commit 466b10c5ee5b7dc57a227f674c2eef14a468796c
Author: wes3 <wi...@runtime.io>
AuthorDate: Tue Apr 23 12:50:39 2019 -0700

    sys/log/full: Allow for limiting size of newtmgr log response (#1778)
    
    * sys/log/full: Allow for limiting size of newtmgr log response
    
    Add a syscfg value (LOG_NMGR_MAX_RSP_LEN) which will limit the size of a newtmgr log response. If the response exceeds this limit and there is only one entry in the response the entry type is set to string and the message "error: entry too large (xxx bytes)" is encoded as in the message portion of the response. If the response contains multiple entries the code is unchanged (the additional entry is not added and the encode function returns OS_ENOMEM).
    
    Note that the previous code had a hard-coded limit of 400 bytes for a newtmgr log response but this was not applied to a response with a single log entry in it. This fix applies the same limit to a response with one or more entries in it.
    
    * sys/log/full: Allow for limiting size of newtmgr log response
    
    Address review comments. Fix set but not used variable for LOG_VERSION <= 2 and remove un-needed code.
    
    * sys/log/full: Allow for limiting size of newtmgr log response
    
    Fix unused variable warning for LOG_VERSION <= 2.
---
 sys/log/full/src/log_nmgr.c | 76 +++++++++++++++++++++++++++++++++++----------
 sys/log/full/syscfg.yml     | 16 +++++++++-
 2 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/sys/log/full/src/log_nmgr.c b/sys/log/full/src/log_nmgr.c
index 647c772..ba1f334 100644
--- a/sys/log/full/src/log_nmgr.c
+++ b/sys/log/full/src/log_nmgr.c
@@ -79,6 +79,7 @@ log_nmgr_encode_entry(struct log *log, struct log_offset *log_offset,
     uint8_t data[128];
     int rc;
     int rsp_len;
+    bool too_long;
     CborError g_err = CborNoError;
     struct log_encode_data *ed = log_offset->lo_arg;
     CborEncoder rsp;
@@ -87,6 +88,7 @@ log_nmgr_encode_entry(struct log *log, struct log_offset *log_offset,
 #if MYNEWT_VAL(LOG_VERSION) > 2
     CborEncoder str_encoder;
     int off;
+    uint8_t etype;
 #endif
     rc = OS_OK;
 
@@ -146,6 +148,12 @@ log_nmgr_encode_entry(struct log *log, struct log_offset *log_offset,
         return MGMT_ERR_ECORRUPT;
     }
 
+    /*
+     * Copy the type from the header type. This may get changed to type
+     * string if a single entry is too long.
+     */
+    etype = ueh->ue_etype;
+
     g_err |= cbor_encode_text_stringz(&rsp, "msg");
 
     /*
@@ -179,20 +187,41 @@ log_nmgr_encode_entry(struct log *log, struct log_offset *log_offset,
     g_err |= cbor_encode_uint(&rsp,  ueh->ue_module);
     g_err |= cbor_encoder_close_container(&cnt_encoder, &rsp);
     rsp_len += cbor_encode_bytes_written(&cnt_encoder);
+
     /*
-     * Make sure that at least single entry is returned, even in case response
-     * exceeds this magic value. This is to make sure we can read long log
-     * entries, even if they have to be read one by one.
+     * Check if the response is too long. If more than one entry is in the
+     * response we will not add the current one and will return ENOMEM. If this
+     * is just a single entry we add the generic too long message text.
      */
-    if ((rsp_len > 400) && (ed->counter > 0)) {
-        rc = OS_ENOMEM;
-        goto err;
+    too_long = false;
+    if (rsp_len > MYNEWT_VAL(LOG_NMGR_MAX_RSP_LEN)) {
+        /*
+         * Is this just a single entry? If so, encode the generic error
+         * message in the "msg" field of the response
+         */
+        if (ed->counter == 0) {
+#if MYNEWT_VAL(LOG_VERSION) > 2
+            etype = LOG_ETYPE_STRING;
+#endif
+            too_long = true;
+        } else {
+            rc = OS_ENOMEM;
+            goto err;
+        }
     }
+
+    /*
+     * If a single entry is too long the response length (rsp_len) is not
+     * correct. However, this is not an issue since we only use lo_data_len
+     * when encoding multiple entries. Therefore, there is no need to try
+     * and do a dummy encoding with the generic too long entry message simply
+     * to calculate a response size that will not be used.
+     */
     log_offset->lo_data_len = rsp_len;
 
     g_err |= cbor_encoder_create_map(ed->enc, &rsp, CborIndefiniteLength);
 #if MYNEWT_VAL(LOG_VERSION) > 2
-    switch (ueh->ue_etype) {
+    switch (etype) {
     case LOG_ETYPE_CBOR:
         g_err |= cbor_encode_text_stringz(&rsp, "type");
         g_err |= cbor_encode_text_stringz(&rsp, "cbor");
@@ -220,18 +249,27 @@ log_nmgr_encode_entry(struct log *log, struct log_offset *log_offset,
      * inside.
      */
     g_err |= cbor_encoder_create_indef_byte_string(&rsp, &str_encoder);
-    for (off = 0; off < len && !g_err; ) {
-        rc = log_read_body(log, dptr, data, off, sizeof(data));
-        if (rc < 0) {
-            g_err |= 1;
-            break;
-        }
+    if (too_long) {
+        sprintf((char *)data, "error: entry too large (%d bytes)", rsp_len);
+        rc = strlen((char *)data);
         g_err |= cbor_encode_byte_string(&str_encoder, data, rc);
-        off += rc;
+    } else {
+        for (off = 0; off < len && !g_err;) {
+            rc = log_read_body(log, dptr, data, off, sizeof(data));
+            if (rc < 0) {
+                g_err |= 1;
+                break;
+            }
+            g_err |= cbor_encode_byte_string(&str_encoder, data, rc);
+            off += rc;
+        }
     }
     g_err |= cbor_encoder_close_container(&rsp, &str_encoder);
 #else
     g_err |= cbor_encode_text_stringz(&rsp, "msg");
+    if (too_long) {
+        sprintf((char *)data, "error: entry too large (%d bytes)", rsp_len);
+    }
     g_err |= cbor_encode_text_stringz(&rsp, (char *)data);
 #endif
     g_err |= cbor_encode_text_stringz(&rsp, "ts");
@@ -249,7 +287,13 @@ log_nmgr_encode_entry(struct log *log, struct log_offset *log_offset,
     if (g_err) {
         return MGMT_ERR_ENOMEM;
     }
-    return (0);
+
+    /* If response is too long we want to respond with ENOMEM error code */
+    if (too_long) {
+        rc = OS_ENOMEM;
+    } else {
+        rc = 0;
+    }
 err:
     return (rc);
 }
@@ -284,7 +328,7 @@ log_encode_entries(struct log *log, CborEncoder *cb,
     g_err |= cbor_encoder_close_container(&cnt_encoder, &entries);
     rsp_len = cbor_encode_bytes_written(cb) +
               cbor_encode_bytes_written(&cnt_encoder);
-    if (rsp_len > 400) {
+    if (rsp_len > MYNEWT_VAL(LOG_NMGR_MAX_RSP_LEN)) {
         rc = OS_ENOMEM;
         goto err;
     }
diff --git a/sys/log/full/syscfg.yml b/sys/log/full/syscfg.yml
index 6b93564..103d978 100644
--- a/sys/log/full/syscfg.yml
+++ b/sys/log/full/syscfg.yml
@@ -6,7 +6,7 @@
 # to you under the Apache License, Version 2.0 (the
 # "License"); you may not use this file except in compliance
 # with the License.  You may obtain a copy of the License at
-# 
+#
 #  http://www.apache.org/licenses/LICENSE-2.0
 #
 # Unless required by applicable law or agreed to in writing,
@@ -70,6 +70,20 @@ syscfg.defs:
         description: 'Expose "log" command in newtmgr.'
         value: 0
 
+    LOG_NMGR_MAX_RSP_LEN:
+        description: >
+            The maximum newtmgr log response length. Note that this does not
+            represent the entire length of the response sent as the response
+            error code is added to the response after this check is performed.
+            If a single log entry exceeds this value the msg contents of the
+            log entry will be the following string: error: entry too large
+            (xxx bytes) with xxx being the size of the entry. Note that the
+            value of 400 is a legacy hard-coded value that the code used to
+            limit the size of a response with multiple entries although the
+            previous code did not limit the size of a response with only one
+            entry in it.
+        value: 400
+
     LOG_MAX_USER_MODULES:
         description: 'Maximum number of user modules to register'
         value: 1