Skip to content

Imported typedef pattern inherits the first consumer's regexp-posix flavor #2555

Description

@karowski

Imported typedef pattern inherits the first consumer's regexp-posix flavor

Version: libyang v5.8.6 (47351e59e) and upstream devel project version 6.1.7 (commit 68263438),
verified present on both. Line numbers are pristine v5.8.6.

A typedef with a pattern, defined in a module that does not carry openconfig-extensions:regexp-posix,
is compiled as a POSIX regex for an equally-unmarked consumer — purely because another, regexp-posix
module happened to compile the shared typedef first. Loading the consumers in the other order makes the
same leaf XML-Schema. The regexp-posix marker is a per-module statement and should only affect the
patterns defined in that module; here it leaks into an unrelated module through a shared typedef.

The defect is in the flavor selection, independent of how a pattern of either flavor is
subsequently matched: POSIX ERE and XML-Schema regex are different dialects (see Impact), so a
pattern handed the wrong one is validated against a different language than its author wrote.

Minimal reproducer (pure libyang, self-contained)

Neither bugtypes (which defines the typedef) nor bugconsumer (which uses it) carries regexp-posix,
so /bugconsumer:c/dst must be XML-Schema in every order. The program compiles the two consumers in both
orders and reads the compiled pattern's format flag from the public schema tree — so it demonstrates the
defect directly, without relying on any matching behaviour.

Build/run with gcc ly_pattern_flavor_load_order.c -lyang -o repro && ./repro (pass a module directory or
set LY_MODULES_DIR if libyang's internal modules are not on the default path). Exit: 0 = XML-Schema in
both orders (correct), 1 = POSIX when the regexp-posix module compiled the shared typedef first
(defect), 2 = setup failure.

#include <libyang/libyang.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#ifndef LY_MODULES_DIR_DEFAULT
# define LY_MODULES_DIR_DEFAULT NULL
#endif

static const char *mod_oc_ext =
        "module openconfig-extensions {\n"
        "  namespace \"http://openconfig.net/yang/openconfig-ext\";\n"
        "  prefix oc-ext;\n"
        "  extension regexp-posix { description \"POSIX regex patterns.\"; }\n"
        "}\n";

/* UNMARKED module (no regexp-posix) that DEFINES the patterned typedef. */
static const char *mod_types =
        "module bugtypes {\n"
        "  namespace \"urn:bug:types\";\n"
        "  prefix bt;\n"
        "  typedef ipv4-address {\n"
        "    type string {\n"
        "      pattern '([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])"
        "(\\.([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])){3}';\n"
        "    }\n"
        "  }\n"
        "}\n";

/* regexp-posix consumer of the shared typedef. */
static const char *mod_posix =
        "module bugposix {\n"
        "  namespace \"urn:bug:posix\";\n"
        "  prefix bp;\n"
        "  import openconfig-extensions { prefix oc-ext; }\n"
        "  import bugtypes { prefix bt; }\n"
        "  oc-ext:regexp-posix;\n"
        "  container c { leaf a { type bt:ipv4-address; } }\n"
        "}\n";

/* UNMARKED consumer of the same shared typedef. */
static const char *mod_consumer =
        "module bugconsumer {\n"
        "  namespace \"urn:bug:consumer\";\n"
        "  prefix bc;\n"
        "  import bugtypes { prefix bt; }\n"
        "  container c { leaf dst { type bt:ipv4-address; } }\n"
        "}\n";

/* Return the `format` flag (0 = XML-Schema, 1 = POSIX) of the first pattern of
 * /bugconsumer:c/dst, or -1 on any unexpected shape. */
static int
consumer_pattern_format(struct ly_ctx *ctx)
{
    const struct lysc_node *node;
    const struct lysc_node_leaf *leaf;
    const struct lysc_type_str *st;

    node = lys_find_path(ctx, NULL, "/bugconsumer:c/dst", 0);
    if (!node || !(node->nodetype & LYS_LEAF)) {
        return -1;
    }
    leaf = (const struct lysc_node_leaf *)node;
    if (!leaf->type || (leaf->type->basetype != LY_TYPE_STRING)) {
        return -1;
    }
    st = (const struct lysc_type_str *)leaf->type;
    if (!st->patterns || (LY_ARRAY_COUNT(st->patterns) < 1)) {
        return -1;
    }
    return (int)st->patterns[0]->format;
}

/* Build a context, parse the modules with the two consumers in the chosen order,
 * and return the flavor of the unmarked consumer's pattern (or -1 on error). */
static int
run_order(const char *searchdir, int posix_first)
{
    struct ly_ctx *ctx = NULL;
    const char *order[4];
    int i, fmt;

    order[0] = mod_oc_ext;
    order[1] = mod_types;
    order[2] = posix_first ? mod_posix : mod_consumer;
    order[3] = posix_first ? mod_consumer : mod_posix;

    if (ly_ctx_new(searchdir, 0, &ctx)) {
        return -1;
    }
    for (i = 0; i < 4; i++) {
        if (lys_parse_mem(ctx, order[i], LYS_IN_YANG, NULL)) {
            ly_ctx_destroy(ctx);
            return -1;
        }
    }
    fmt = consumer_pattern_format(ctx);
    ly_ctx_destroy(ctx);
    return fmt;
}

int
main(int argc, char **argv)
{
    const char *searchdir = getenv("LY_MODULES_DIR");
    int fa, fb;

    if (argc > 1) {
        searchdir = argv[1];
    }
    if (!searchdir) {
        searchdir = LY_MODULES_DIR_DEFAULT;
    }

    fa = run_order(searchdir, 1);   /* regexp-posix consumer compiled first */
    fb = run_order(searchdir, 0);   /* unmarked consumer compiled first */
    if ((fa < 0) || (fb < 0)) {
        fprintf(stderr, "setup: could not inspect the compiled pattern\n");
        return 2;
    }

    printf("flavor of /bugconsumer:c/dst (an unmarked leaf via an unmarked typedef):\n");
    printf("    regexp-posix module compiled first : %s\n", fa ? "POSIX" : "XML-Schema");
    printf("    unmarked module compiled first     : %s\n", fb ? "POSIX" : "XML-Schema");

    /* Neither bugconsumer nor bugtypes carries regexp-posix, so the consumer's
     * pattern must be XML-Schema in BOTH orders. Assert both explicitly. */
    if ((fa == 0) && (fb == 0)) {
        printf("OK: the consumer pattern is XML-Schema regardless of order.\n");
        return 0;
    }
    if ((fa == 1) && (fb == 0)) {
        printf("BUG: order-dependent flavor -- the unmarked consumer's pattern is POSIX when the "
                "regexp-posix module compiles the shared typedef first, XML-Schema otherwise.\n");
        return 1;
    }
    /* fa==0,fb==1 or fa==1,fb==1: an unmarked consumer still came out POSIX in at
     * least one order -- wrong flavor, though not the canonical order-dependent
     * shape this test expects. */
    printf("BUG (unexpected shape): an unmarked consumer's pattern is POSIX in at least one order "
            "(regexp-posix-first=%s, unmarked-first=%s).\n",
            fa ? "POSIX" : "XML-Schema", fb ? "POSIX" : "XML-Schema");
    return 1;
}

Output (stock v5.8.6)

flavor of /bugconsumer:c/dst (an unmarked leaf via an unmarked typedef):
    regexp-posix module compiled first : POSIX
    unmarked module compiled first     : XML-Schema
BUG: order-dependent flavor -- the unmarked consumer's pattern is POSIX when the regexp-posix module compiles the shared typedef first, XML-Schema otherwise.

Cause

lys_compile_type_patterns() (src/schema_compile_node.c :1150) picks the flavor from the module being
compiled — lys_compile_type_patterns_has_oc_posix_ext(ctx->pmod) — i.e. the first consumer, not the
module that defines the pattern. The typedef's compiled type is then cached on tpdf->type.compiled
and reused by later consumers rather than recompiled: the cached type is taken as base around
:2066-2110, and it is only freed/recompiled on the refcount == 1 path at :1986. So the first consumer to
compile the shared typedef bakes its module's flavor into the shared compiled type for everyone.

This is not the pattern cache: ly_ctx_shared_data_pattern_get() (src/ly_common.c :569) keys its
hash on both the pattern text and the format, with ly_ctx_ht_pattern_equal_cb() (:139) comparing both —
so it never confuses a POSIX and an XML-Schema compilation of the same string.

Impact

Selecting POSIX instead of XML-Schema is a real correctness defect, not just wrong metadata: the two are
different regex dialects with different syntax and semantics, so the same pattern string can define a
different accepted language under each, changing compilation and/or validation results. For example,
XML-Schema regex treats ^ and $ as literal characters (XSD patterns are implicitly whole-value
anchored), whereas POSIX ERE treats them as anchors — so a pattern that uses ^/$ matches a
different set of values depending on the flavor it was handed. An unmarked module's data is therefore
validated against the wrong language, and which flavor a shared typedef receives depends purely on module
compile order (the reproducer shows the same leaf coming out POSIX or XML-Schema by order alone).

It defeats OpenConfig's own RFC 7950 fix, with unmodified upstream models

This is not a synthetic concern. openconfig-inet-types.yang revision 2021-01-07 removed
module-level oc-ext:regexp-posix precisely to get RFC 7950 semantics back — "Remove module extension
oc-ext:regexp-posix by making pattern regexes conform to RFC7950. Types impacted: ipv4-address,
ipv4-address-zoned, ipv6-address, domain-name"
— and its ipv4-address now carries an anchorless
standard pattern that depends on whole-value matching.

openconfig-inet-types is not alone: 9 modules carry that same removal note, including
openconfig-bgp-types, openconfig-packet-match-types and openconfig-vlan-types (also 2021-01-07),
and openconfig-isis-types, whose 2021-08-12 revision adds "This change is backwards incompatible
because it changes the area address regex."
None of the 9 declares the extension any more. So
OpenConfig has been deliberately migrating modules off regexp-posix toward RFC 7950 conformance
since 2021.

That migration does not survive this defect, because the flavor leaks in from a module that was not
migrated:

  • 102 modules under release/models/ still declare oc-ext:regexp-posix;
  • 23 of those also import openconfig-inet-types, among them openconfig-if-ip.yang (which uses
    oc-inet:ipv4-address directly), openconfig-system-logging.yang, openconfig-snmp.yang and
    openconfig-telemetry.yang.

Importing is not sufficient on its own — the marked module has to actually use the shared typedef,
so that compiling the marked module is what compiles it (openconfig-module-catalog.yang, for
instance, imports openconfig-inet-types but only uses oc-inet:uri). Loading a marked module that
does compile the relevant typedef — such as the openconfig-telemetry or openconfig-snmp modules
tested below, both of which use oc-inet:ip-address, a union over ipv4-address/ipv6-address
before an unmarked consumer leaves oc-inet:ipv4-address compiled POSIX for everyone.

ly_flavor_leak_openconfig.c in this directory demonstrates it against a --depth 1 clone of
github.com/openconfig/public, with a two-line probe module that merely imports openconfig-inet-types
and declares leaf addr { type oci:ipv4-address; }:

############ libyang v5.8.6 (stock) ############
  probe alone (unmarked)           10.20.30.1 -> accept     10.20.30.1/33 -> reject
  openconfig-telemetry.yang        10.20.30.1 -> accept     10.20.30.1/33 -> ACCEPT  <-- constraint lost
libyang probe alone openconfig-telemetry.yang loaded first
2.1.111 (libyang.so.2.38.14) reject reject — no regexp-posix support at all
v5.8.6 (libyang.so.5.5.5) reject accepted
devel 6.1.7 (68263438, libyang.so.5.6.11) reject accepted

openconfig-snmp.yang behaves identically. So in a stock OpenConfig deployment the validation of
ipv4-address depends on which module the application happened to load first, and OpenConfig's
deliberate 2021 correction is silently undone.

Counting note: a plain grep for oc-ext:regexp-posix reports 103 files, but one of those —
openconfig-isis-types.yang — matches only because the phrase appears in the prose of the revision
description that removed it. Declarations (oc-ext:regexp-posix ;) number 102.

Reproducing it

The OpenConfig models are not vendored here, so this demo takes a clone; it discovers the search dirs
itself. Modules that pull in openconfig-network-instance fail to load for an unrelated reason (an
identityref default referencing a non-implemented module), so it defaults to a light marked module.

git clone --depth 1 https://github.com/openconfig/public.git
gcc ly_flavor_leak_openconfig.c -lyang -o ocleak

LY=<libyang-prefix>            # e.g. ../toolchain/libyang
export LY_MODULES_DIR=$LY/share/yang/modules/libyang

# the clone is walked for directories holding .yang files, so no search-path plumbing
./ocleak public

# a different marked module -- note the path is taken as given, so include the clone dir
./ocleak public public/release/models/system/openconfig-snmp.yang

Source (ly_flavor_leak_openconfig.c)

#include <libyang/libyang.h>
#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>

#ifndef LY_MODULES_DIR_DEFAULT
# define LY_MODULES_DIR_DEFAULT NULL
#endif

#define MARKED_DEFAULT "release/models/telemetry/openconfig-telemetry.yang"

static const char *probe =
        "module probe {\n"
        "  namespace \"urn:probe\"; prefix p;\n"
        "  import openconfig-inet-types { prefix oci; }\n"
        "  leaf addr { type oci:ipv4-address; }\n"
        "}\n";

/* libyang search dirs are not recursive and there is no "add tree" call, so walk the
 * clone and register every directory that actually holds .yang files. */
static void
add_searchdirs(struct ly_ctx *ctx, const char *dir)
{
    DIR *d = opendir(dir);
    struct dirent *e;
    char path[4096];
    struct stat sb;
    int has_yang = 0;

    if (!d) {
        return;
    }
    while ((e = readdir(d))) {
        if (!strcmp(e->d_name, ".") || !strcmp(e->d_name, "..")) {
            continue;
        }
        if (snprintf(path, sizeof path, "%s/%s", dir, e->d_name) >= (int)sizeof path) {
            continue;
        }
        if (stat(path, &sb)) {
            continue;
        }
        if (S_ISDIR(sb.st_mode)) {
            add_searchdirs(ctx, path);
        } else if (!has_yang) {
            const char *dot = strrchr(e->d_name, '.');

            if (dot && !strcmp(dot, ".yang")) {
                has_yang = 1;
            }
        }
    }
    closedir(d);

    if (has_yang) {
        ly_ctx_set_searchdir(ctx, dir);
    }
}

/* 1 = accepted, 0 = rejected by the pattern, -1 = other */
static int
try_val(struct ly_ctx *ctx, const char *v)
{
    struct lyd_node *t = NULL;
    LY_ERR r = lyd_new_path(NULL, ctx, "/probe:addr", v, 0, &t);

    if (!r) {
        lyd_free_all(t);
        return 1;
    }
    return (r == LY_EVALID) ? 0 : -1;
}

/* Returns 0 on success, 2 if the run could not be set up. */
static int
run(const char *label, const char *marked, const char *lymod, const char *ocroot)
{
    struct ly_ctx *ctx = NULL;
    struct stat sb;
    int v33, vok;

    if (ly_ctx_new(lymod, 0, &ctx)) {
        fprintf(stderr, "setup: ly_ctx_new() failed -- libyang's own modules were not found in\n"
                "       \"%s\".\n"
                "       Set LY_MODULES_DIR to <libyang-prefix>/share/yang/modules/libyang.\n",
                lymod ? lymod : "(none given)");
        return 2;
    }
    ly_log_level(LY_LLERR);
    ly_log_options(LY_LOLOG);

    add_searchdirs(ctx, ocroot);

    if (marked && stat(marked, &sb)) {
        fprintf(stderr, "setup: \"%s\" does not exist.\n"
                "       The marked-module argument is a path as given, not relative to the clone --\n"
                "       prefix it with the clone directory, e.g. \"%s/" MARKED_DEFAULT "\".\n",
                marked, ocroot);
        ly_ctx_destroy(ctx);
        return 2;
    }
    if (marked && lys_parse_path(ctx, marked, LYS_IN_YANG, NULL)) {
        fprintf(stderr, "setup: could not load \"%s\".\n"
                "       Modules importing openconfig-network-instance fail for an unrelated\n"
                "       reason (an identityref default referencing a non-implemented module);\n"
                "       use a light marked module such as " MARKED_DEFAULT " or\n"
                "       release/models/system/openconfig-snmp.yang.\n", marked);
        ly_ctx_destroy(ctx);
        return 2;
    }
    if (lys_parse_mem(ctx, probe, LYS_IN_YANG, NULL)) {
        fprintf(stderr, "setup: could not load the probe module -- is \"%s\" an openconfig/public\n"
                "       clone containing release/models/types/openconfig-inet-types.yang?\n", ocroot);
        ly_ctx_destroy(ctx);
        return 2;
    }

    vok = try_val(ctx, "10.20.30.1");
    v33 = try_val(ctx, "10.20.30.1/33");
    printf("  %-32s 10.20.30.1 -> %-8s   10.20.30.1/33 -> %s\n", label,
            vok == 1 ? "accept" : vok == 0 ? "reject" : "?",
            v33 == 1 ? "ACCEPT  <-- constraint lost" : v33 == 0 ? "reject" : "?");
    ly_ctx_destroy(ctx);
    return 0;
}

int
main(int argc, char **argv)
{
    const char *ocroot = (argc > 1) ? argv[1] : "public";
    const char *lymod = getenv("LY_MODULES_DIR");
    char marked[4096];
    struct stat sb;

    if (!lymod) {
        lymod = LY_MODULES_DIR_DEFAULT;
    }
    if (stat(ocroot, &sb) || !S_ISDIR(sb.st_mode)) {
        fprintf(stderr,
                "usage: %s [path-to-openconfig-public-clone] [marked-module]\n\n"
                "  git clone --depth 1 https://github.com/openconfig/public.git\n"
                "  LY_MODULES_DIR=<libyang-prefix>/share/yang/modules/libyang %s public\n\n"
                "\"%s\" is not a directory.\n", argv[0], argv[0], ocroot);
        return 2;
    }
    if (argc > 2) {
        snprintf(marked, sizeof marked, "%s", argv[2]);
    } else {
        snprintf(marked, sizeof marked, "%s/" MARKED_DEFAULT, ocroot);
    }

    printf("\n");
    if (run("probe alone (unmarked)", NULL, lymod, ocroot)) {
        return 2;
    }
    if (run(strrchr(marked, '/') ? strrchr(marked, '/') + 1 : marked, marked, lymod, ocroot)) {
        return 2;
    }
    return 0;
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions