openwrt / luci

LuCI - OpenWrt Configuration Interface
Apache License 2.0
6.43k stars 2.55k forks source link

Should the `portrange` data type accept a single port? #7242

Open heiher opened 3 months ago

heiher commented 3 months ago

I'm confused because, In natmap.js, the portrange data type accepts both a port range and a single port. However, In natmap.init, portrange does not accept a single port.

https://github.com/openwrt/luci/blob/ca57a08eb9176f51a978cb2adbaa96b6c5dcb470/applications/luci-app-natmap/htdocs/luci-static/resources/view/natmap.js#L93-L95

https://github.com/openwrt/packages/blob/1019631a5a81a4b44035133364de2293bcc75d9d/net/natmap/files/natmap.init#L18-L33

validate_section_natmap() {
    uci_load_validate "${NAME}" natmap "$1" "$2" \
        'enable:bool:0' \
        'family:string' \
        'udp_mode:bool:0' \
        'interface:string' \
        'interval:uinteger' \
        'stun_server:host' \
        'http_server:host' \
        'port:portrange' \
        'forward_target:host' \
        'forward_port:port' \
        'notify_script:file' \
        'log_stdout:bool:1' \
        'log_stderr:bool:1'
}

Should the portrange data type accept a single port?

systemcrash commented 3 months ago

https://github.com/openwrt/openwrt/blob/b72c4b53860ef7a65f486212c7393e2c2b57344b/package/system/procd/files/procd.sh#L621-L634

systemcrash commented 3 months ago

I would suggest that the ubox verify here also tries a 'port' type if the 'portrange' type check fails https://git.openwrt.org/?p=project/ubox.git;a=blob;f=validate/validate.c;h=e72b8117ecd8b680778b0f5c7637ed6546a7736b;hb=refs/heads/master#l593

or

https://github.com/openwrt/ubox/blob/85f1053019caf4cd333795760950235ee4529ba7/validate/validate.c#L593

ping @jow- @robimarko @Ansuel

from e.g.

static bool
dt_type_port(struct dt_state *s, int nargs)
{
    int n;
    char *e;

    n = strtoul(s->value, &e, 10);

    return (e > s->value && *e == 0 && n <= 65535);
}

static bool
dt_type_portrange(struct dt_state *s, int nargs)
{
    int n, m;
    char *e;

    n = strtoul(s->value, &e, 10);

    if (e == s->value || *e != '-')
        return false;

    m = strtoul(e + 1, &e, 10);

    return (*e == 0 && n <= 65535 && m <= 65535 && n <= m);
}

to

static bool
dt_type_portrange(struct dt_state *s, int nargs)
{
    int n, m;
    char *e;

    n = strtoul(s->value, &e, 10);

-   if (e == s->value || *e != '-')
-       return false;
+   if (e == s->value || *e != '-') {
+       // If parsing as portrange fails, try parsing as a single port
+       return dt_type_port(s, nargs);
+   }

    m = strtoul(e + 1, &e, 10);

    return (*e == 0 && n <= 65535 && m <= 65535 && n <= m);
}
systemcrash commented 2 months ago

Sent a patch to the dev mailing list...