resolve BUGFIX remove invalid restriction of intid keys order

They had to be ordered previously, that is no longer true
because RFC does not specify such requirement anywhere.

Fixes #360

Author: michalvasko

src/resolve.c

@@ -3910,142 +3910,160 @@ resolve_schema_leafref(const char *path, struct lys_node *parent, const struct l
  *
  * @param[in] prev_mod Previous module to use in case there is no prefix.
  * @param[in] pred Predicate to use.
- * @param[in,out] node_match Nodes matching the restriction without
- *                           the predicate. Nodes not satisfying
- *                           the predicate are removed.
+ * @param[in,out] node Node matching the restriction without
+ *                     the predicate. If it does not satisfy the predicate,
+ *                     it is set to NULL.
  *
  * @return Number of characters successfully parsed,
  *         positive on success, negative on failure.
  */
 static int
-resolve_instid_predicate(const struct lys_module *prev_mod, const char *pred, struct unres_data *node_match)
+resolve_instid_predicate(const struct lys_module *prev_mod, const char *pred, struct lyd_node **node, int cur_idx)
 {
-    /* ... /node[target = value] ... */
-    struct lyd_node *target;
+    /* ... /node[key=value] ... */
+    struct lyd_node_leaf_list *key;
+    struct lys_node_leaf **list_keys = NULL;
+    struct lys_node_list *slist;
     const char *model, *name, *value;
-    int mod_len, nam_len, val_len, i, has_predicate, cur_idx, idx, parsed, pred_iter, k;
-    uint32_t j;
+    int mod_len, nam_len, val_len, i, has_predicate, parsed;
 
-    assert(pred && node_match->count);
+    assert(pred && node && *node);
 
-    idx = -1;
     parsed = 0;
-
-    pred_iter = -1;
     do {
-        if ((i = parse_predicate(pred, &model, &mod_len, &name, &nam_len, &value, &val_len, &has_predicate)) < 1) {
-            return -parsed+i;
+        if ((i = parse_predicate(pred + parsed, &model, &mod_len, &name, &nam_len, &value, &val_len, &has_predicate)) < 1) {
+            return -parsed + i;
         }
         parsed += i;
-        pred += i;
 
-        if (isdigit(name[0])) {
-            /* pos */
-            assert(!value);
-            idx = atoi(name);
-        } else if (name[0] != '.') {
-            /* list keys */
-            if (pred_iter < 0) {
-                pred_iter = 1;
-            } else {
-                ++pred_iter;
+        /* target */
+        if (name[0] == '.') {
+            /* leaf-list value */
+            if ((*node)->schema->nodetype != LYS_LEAFLIST) {
+                LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects leaf-list, but have %s \"%s\".",
+                       strnodetype((*node)->schema->nodetype), (*node)->schema->name);
+                parsed = -1;
+                goto cleanup;
             }
-        }
 
-        for (cur_idx = 1, j = 0; j < node_match->count; ++cur_idx) {
-            /* target */
-            if (name[0] == '.') {
-                /* leaf-list value */
-                if (node_match->node[j]->schema->nodetype != LYS_LEAFLIST) {
-                    goto remove_instid;
-                }
+            /* check the value */
+            if (strncmp(((struct lyd_node_leaf_list *)*node)->value_str, value, val_len)
+                    || ((struct lyd_node_leaf_list *)*node)->value_str[val_len]) {
+                *node = NULL;
+                goto cleanup;
+            }
 
-                target = node_match->node[j];
-                /* check the value */
-                if (strncmp(((struct lyd_node_leaf_list *)target)->value_str, value, val_len)
-                    || ((struct lyd_node_leaf_list *)target)->value_str[val_len]) {
-                    goto remove_instid;
-                }
+        } else if (isdigit(name[0])) {
+            assert(!value);
 
-            } else if (!value) {
-                /* keyless list position */
-                if ((node_match->node[j]->schema->nodetype != LYS_LIST)
-                        || ((struct lys_node_list *)node_match->node[j]->schema)->keys) {
-                    goto remove_instid;
-                }
+            /* keyless list position */
+            if ((*node)->schema->nodetype != LYS_LIST) {
+                LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects list, but have %s \"%s\".",
+                       strnodetype((*node)->schema->nodetype), (*node)->schema->name);
+                parsed = -1;
+                goto cleanup;
+            }
 
-                if (idx != cur_idx) {
-                    goto remove_instid;
-                }
+            if (((struct lys_node_list *)(*node)->schema)->keys) {
+                LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects list without keys, but have list \"%s\".",
+                       (*node)->schema->name);
+                parsed = -1;
+                goto cleanup;
+            }
 
-            } else {
-                /* list key value */
-                if (node_match->node[j]->schema->nodetype != LYS_LIST) {
-                    goto remove_instid;
-                }
+            /* check the index */
+            if (atoi(name) != cur_idx) {
+                *node = NULL;
+                goto cleanup;
+            }
 
-                /* find the key leaf */
-                for (k = 1, target = node_match->node[j]->child; target && (k < pred_iter); k++, target = target->next);
-                if (!target) {
-                    goto remove_instid;
-                }
-                if ((struct lys_node_leaf *)target->schema !=
-                        ((struct lys_node_list *)node_match->node[j]->schema)->keys[pred_iter - 1]) {
-                    goto remove_instid;
+        } else {
+            /* list key value */
+            if ((*node)->schema->nodetype != LYS_LIST) {
+                LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects list, but have %s \"%s\".",
+                       strnodetype((*node)->schema->nodetype), (*node)->schema->name);
+                parsed = -1;
+                goto cleanup;
+            }
+            slist = (struct lys_node_list *)(*node)->schema;
+
+            /* prepare key array */
+            if (!list_keys) {
+                list_keys = malloc(slist->keys_size * sizeof *list_keys);
+                LY_CHECK_ERR_RETURN(!list_keys, LOGMEM, -1);
+                for (i = 0; i < slist->keys_size; ++i) {
+                    list_keys[i] = slist->keys[i];
                 }
+            }
 
-                /* check name */
-                if (strncmp(target->schema->name, name, nam_len) || target->schema->name[nam_len]) {
-                    goto remove_instid;
+            /* find the schema key leaf */
+            for (i = 0; i < slist->keys_size; ++i) {
+                if (list_keys[i] && !strncmp(list_keys[i]->name, name, nam_len) && !list_keys[i]->name[nam_len]) {
+                    break;
                 }
+            }
+            if (i == slist->keys_size) {
+                /* this list has no such key */
+                LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects list with the key \"%.*s\","
+                       " but list \"%s\" does not define it.", nam_len, name, slist->name);
+                parsed = -1;
+                goto cleanup;
+            }
 
-                /* check module */
-                if (model) {
-                    if (strncmp(target->schema->module->name, model, mod_len)
-                            || target->schema->module->name[mod_len]) {
-                        goto remove_instid;
-                    }
-                } else {
-                    if (target->schema->module != prev_mod) {
-                        goto remove_instid;
-                    }
+            /* check module */
+            if (model) {
+                if (strncmp(list_keys[i]->module->name, model, mod_len) || list_keys[i]->module->name[mod_len]) {
+                    LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects key \"%s\" from module \"%.*s\", not \"%s\".",
+                           list_keys[i]->name, model, mod_len, list_keys[i]->module->name);
+                    parsed = -1;
+                    goto cleanup;
+                }
+            } else {
+                if (list_keys[i]->module != prev_mod) {
+                    LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier expects key \"%s\" from module \"%s\", not \"%s\".",
+                           list_keys[i]->name, prev_mod->name, list_keys[i]->module->name);
+                    parsed = -1;
+                    goto cleanup;
                 }
+            }
 
-                /* check the value */
-                if (strncmp(((struct lyd_node_leaf_list *)target)->value_str, value, val_len)
-                    || ((struct lyd_node_leaf_list *)target)->value_str[val_len]) {
-                    goto remove_instid;
+            /* find the actual data key */
+            for (key = (struct lyd_node_leaf_list *)(*node)->child; key; key = (struct lyd_node_leaf_list *)key->next) {
+                if (key->schema == (struct lys_node *)list_keys[i]) {
+                    break;
                 }
             }
+            if (!key) {
+                /* list instance is missing a key? definitely should not happen */
+                LOGINT;
+                parsed = -1;
+                goto cleanup;
+            }
 
-            /* instid is ok, continue check with the next one */
-            ++j;
-            continue;
+            /* check the value */
+            if (strncmp(key->value_str, value, val_len) || key->value_str[val_len]) {
+                *node = NULL;
+                goto cleanup;
+            }
 
-remove_instid:
-            /* does not fulfill conditions, remove instid record */
-            unres_data_del(node_match, j);
+            /* everything is fine, mark this key as resolved */
+            list_keys[i] = NULL;
         }
     } while (has_predicate);
 
     /* check that all list keys were specified */
-    if ((pred_iter > 0) && node_match->count) {
-        j = 0;
-        while (j < node_match->count) {
-            assert(node_match->node[j]->schema->nodetype == LYS_LIST);
-            if (pred_iter < ((struct lys_node_list *)node_match->node[j]->schema)->keys_size) {
-                /* not enough predicates, just remove the list instance */
-                unres_data_del(node_match, j);
-            } else {
-                ++j;
+    if (list_keys) {
+        for (i = 0; i < slist->keys_size; ++i) {
+            if (list_keys[i]) {
+                LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier is missing list key \"%s\".", list_keys[i]->name);
+                parsed = -1;
+                goto cleanup;
             }
         }
-
-        if (!node_match->count) {
-            LOGVAL(LYE_SPEC, LY_VLOG_NONE, NULL, "Instance identifier is missing some list keys.");
-        }
     }
 
+cleanup:
+    free(list_keys);
     return parsed;
 }
 
@@ -7260,10 +7278,10 @@ check_instid_ext_dep(const struct lys_node *sleaf, const char *json_instid)
 static int
 resolve_instid(struct lyd_node *data, const char *path, int req_inst, struct lyd_node **ret)
 {
-    int i = 0, j;
+    int i = 0, j, parsed, cur_idx;
     const struct lys_module *mod, *prev_mod = NULL;
     struct ly_ctx *ctx = data->schema->module->ctx;
-    struct lyd_node *root;
+    struct lyd_node *root, *node;
     const char *model, *name;
     char *str;
     int mod_len, name_len, has_predicate;
@@ -7323,17 +7341,27 @@ resolve_instid(struct lyd_node *data, const char *path, int req_inst, struct lyd
 
         if (has_predicate) {
             /* we have predicate, so the current results must be list or leaf-list */
-            j = resolve_instid_predicate(mod, &path[i], &node_match);
-            if (j < 1) {
-                LOGVAL(LYE_INPRED, LY_VLOG_LYD, data, &path[i-j]);
-                goto error;
-            }
-            i += j;
+            j = 0;
+            /* index of the current node (for lists with position predicates) */
+            cur_idx = 1;
+            while (j < (signed)node_match.count) {
+                node = node_match.node[j];
+                parsed = resolve_instid_predicate(mod, &path[i], &node, cur_idx);
+                if (parsed < 1) {
+                    LOGVAL(LYE_INPRED, LY_VLOG_LYD, data, &path[i - parsed]);
+                    goto error;
+                }
 
-            if (!node_match.count) {
-                /* no instance exists */
-                break;
+                if (!node) {
+                    /* current node does not satisfy the predicate */
+                    unres_data_del(&node_match, j);
+                } else {
+                    ++j;
+                }
+                ++cur_idx;
             }
+
+            i += parsed;
         } else if (node_match.count) {
             /* check that we are not addressing lists */
             for (j = 0; (unsigned)j < node_match.count; ++j) {

tests/data/files/all-data.json

@@ -15,7 +15,7 @@
     "leaf13": "aGVsbG8h",
     "leaf14": "ident2",
     "leaf15": [null],
-    "leaf16": "/all:cont1/all:leaf2",
+    "leaf16": "/all:cont1/leaf2",
     "leaf17": "ident2",
     "list1": [
       {
@@ -34,6 +34,13 @@
       "aaa",
       "aaaa",
       "aaaaaaaaa"
-    ]
+    ],
+    "list2": [
+      {
+        "leaf27": 3,
+        "leaf28": 4
+      }
+    ],
+    "leaf29": "/all:cont1/list2[leaf28='4'][leaf27='3']"
   }
 }

tests/data/files/all-data.xml

@@ -33,4 +33,9 @@
   <llist1>aaa</llist1>
   <llist1>aaaa</llist1>
   <llist1>aaaaaaaaa</llist1>
+  <list2>
+    <leaf27>3</leaf27>
+    <leaf28>4</leaf28>
+  </list2>
+  <leaf29 xmlns:all_mod="urn:all">/all_mod:cont1/all_mod:list2[all_mod:leaf28='4'][all_mod:leaf27='3']</leaf29>
 </cont1>

tests/data/files/all.yang

@@ -199,6 +199,21 @@ module all {
       ordered-by user;
     }
 
+    list list2 {
+      key "leaf27 leaf28";
+      leaf leaf27 {
+        type uint8;
+      }
+
+      leaf leaf28 {
+        type uint8;
+      }
+    }
+
+    leaf leaf29 {
+      type instance-identifier;
+    }
+
     leaf leaf23 {
       type empty;
     }

tests/data/files/all.yin

@@ -175,6 +175,18 @@
       <type name="tdef1"/>
       <ordered-by value="user"/>
     </leaf-list>
+    <list name="list2">
+      <key value="leaf27 leaf28"/>
+      <leaf name="leaf27">
+        <type name="uint8"/>
+      </leaf>
+      <leaf name="leaf28">
+        <type name="uint8"/>
+      </leaf>
+    </list>
+    <leaf name="leaf29">
+      <type name="instance-identifier"/>
+    </leaf>
     <leaf name="leaf23">
       <type name="empty"/>
     </leaf>

tests/data/test_parse_print.c

@@ -227,7 +227,7 @@ test_parse_print_xml(void **state)
     assert_int_equal(read(fd, st->str1, s.st_size), s.st_size);
     st->str1[s.st_size] = '\0';
 
-    st->dt = lyd_parse_path(st->ctx, data, LYD_XML, LYD_OPT_CONFIG);
+    st->dt = lyd_parse_path(st->ctx, data, LYD_XML, LYD_OPT_CONFIG | LYD_OPT_STRICT);
     assert_ptr_not_equal(st->dt, NULL);
     lyd_print_mem(&(st->str2), st->dt, LYD_XML, LYP_FORMAT);