[systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

[Lennart Poettering]

On Fri, 10.04.15 15:17, Tomasz Bursztyka (tomasz.bursztyka at linux.intel.com) wrote:

> +struct PAC {
> +        duk_context *ctx;
> +};
> +
> +static int get_addresses_from_interface(int ifindex, union in_addr_union *address) {
> +        struct ifreq ifr = {};
> +        int sk;
> +
> +        sk = socket(AF_INET, SOCK_DGRAM, 0);
> +        if (sk < 0)
> +                return -1;

No made up errors please! Return -errno or so.

> +
> +        ifr.ifr_ifindex = ifindex;
> +
> +        if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 0) {
> +                close(sk);
> +                return -1;

Same here...

Also, please don't use the old ioctls for querying network
information. Use netlink through sd-rtnl. You can look at the
systemd-resolved sources, they do this already, and this code should
probably do it very similar from that.

> +static int pac_dns_resolve(duk_context *ctx) {
> +        _cleanup_free_ char *address = NULL;
> +        struct addrinfo hints = {};
> +        struct addrinfo *res, *addr;
> +        const char *hostname;
> +        int r;
> +
> +        hostname = duk_require_string(ctx, 0);
> +
> +        hints.ai_family = AF_INET;
> +
> +        r = getaddrinfo(hostname, NULL, &hints, &res);
> +        if (r != 0)
> +                return 0;

Hm, synchronous getaddrinfo() is nasty... Please use sd-resolve for
this, which adds asynchronous getaddrinfo() for cases like this...

> +
> +        for (addr = res; addr; addr = addr->ai_next) {
> +                union in_addr_union a;
> +
> +                if (addr->ai_family != AF_INET ||
> +                                addr->ai_addrlen != sizeof(struct sockaddr_in))
> +                        continue;
> +
> +                a.in = ((struct sockaddr_in *) addr->ai_addr)->sin_addr;
> +                in_addr_to_string(AF_INET, &a, &address);

This can actually fail need to check for OOM...

> +        r = local_gateways(NULL, 0, AF_INET, &gateways);
> +        if (r <= 0)
> +                return 0;
> +
> +        r = get_addresses_from_interface(gateways->ifindex, &addr);
> +        if (r < 0)
> +                return 0;

BTW; local_addresses() can do this for you...

> +        in_addr_to_string(AF_INET, &addr, &address);

Can fail, due to OOM, needs OOM check...

> +static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
> +        duk_context *ctx;
> +
> +        ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
> +        if (!ctx)
> +                return -ENOMEM;
> +
> +        duk_push_global_object(ctx);
> +        duk_push_c_function(ctx, pac_dns_resolve, 1);
> +        duk_put_prop_string(ctx, -2, "dnsResolve");
> +        duk_push_c_function(ctx, pac_my_ip_address, 0);
> +        duk_put_prop_string(ctx, -2, "myIpAddress");
> +
> +        duk_push_pointer(ctx, user_data);
> +        duk_put_prop_string(ctx, -2, "_user_data_");
> +
> +        duk_pop(ctx);
> +
> +        if (duk_peval_file(ctx, pac_file) != 0) {
> +                duk_destroy_heap(ctx);
> +                return -EINVAL;
> +        }

How is error handling done in duktape? The individual functions cannot
fail? And are any errors returned?

> +int pac_execute(struct PAC *pac, const char *url, const char *host, char **ret) {
> +        duk_push_global_object(pac->ctx);
> +        duk_get_prop_string(pac->ctx, -1, "FindProxyForURL");
> +        duk_push_string(pac->ctx, url);
> +        duk_push_string(pac->ctx, host);
> +
> +        if (duk_pcall(pac->ctx, 2) != DUK_EXEC_SUCCESS)
> +                 return -1;
> +
> +        *ret = strdup(duk_to_string(pac->ctx, -1));

Missing OOM check...

Lennart

-- 
Lennart Poettering, Red Hat