Repository: valkey-io/valkey
File: src/cluster.c (C)

Code before:
    }
    return C_OK;
}

int isValidAuxChar(int c) {
    /* Return true if the character is alphanumeric */
    if (isalnum(c)) {
        return 1;
    }

    /* List of invalid characters */
    static const char *invalid_charset = "!#$%&()*+;<>?@[]^{|}~";

    /* Return true if the character is NOT in the invalid charset */
    return strchr(invalid_charset, c) == NULL;
}

int isValidAuxString(char *s, unsigned int length) {
    for (unsigned i = 0; i < length; i++) {
        if (!isValidAuxChar(s[i])) return 0;
    }
    return 1;
}

int parseAndValidateConnectionPorts(client *c, long long *port, long long *cport) {
    if (getLongLongFromObject(c->argv[3], port) != C_OK) {
        addReplyErrorFormat(c, "Invalid base port specified: %s", (char *)c->argv[3]->ptr);
        return C_ERR;
    }
    if (*port <= 0 || *port > 65535) {
        addReplyErrorFormat(c, "Port number is out of range");
        return C_ERR;
    }

    if (c->argc == 5) {
        if (getLongLongFromObject(c->argv[4], cport) != C_OK) {
            addReplyErrorFormat(c, "Invalid bus port specified: %s", (char *)c->argv[4]->ptr);
            return C_ERR;
        }
    } else {
        *cport = *port + CLUSTER_PORT_INCR;
    }

    if (*cport <= 0 || *cport > 65535) {
        addReplyErrorFormat(c, "Cport number is out of range");
        return C_ERR;
    }

    return C_OK;
}


Review comment (refactor):
This function has a strong coupling with the CLUSTER MEET command. The `c->argv[3]` and `c->argv[4]` are hard-coded to match exactly the args of that command. It is actually a helper function that's only usable for custer meet.

I think it's better just keep this code directly in clusterCommandSpecial where cluster meet is handled. If we keep it there, it is clear from the surrounding context what argv[3] and argv[4] are.

We can do some other simplification of the validation, like function to parse and validate a port, such as `int getPortFromObject(robj *arg, int *port)` that just parses the number and does a port range check.

Code after:
    return C_OK;
}

int isValidAuxChar(int c) {
    /* Return true if the character is alphanumeric */
    if (isalnum(c)) {
        return 1;
    }

    /* List of invalid characters */
    static const char *invalid_charset = "!#$%&()*+;<>?@[]^{|}~";

    /* Return true if the character is NOT in the invalid charset */
    return strchr(invalid_charset, c) == NULL;
}

int isValidAuxString(char *s, unsigned int length) {
    for (unsigned i = 0; i < length; i++) {
        if (!isValidAuxChar(s[i])) return 0;
    }
    return 1;
}

void clusterCommandMyId(client *c) {
    char *name = clusterNodeGetName(getMyClusterNode());
    if (name) {
        addReplyBulkCBuffer(c, name, CLUSTER_NAMELEN);
    } else {
        addReplyError(c, "No ID yet");
    }
}

int clusterNodeIsMyself(clusterNode *n) {
    return n == getMyClusterNode();
}

void clusterCommandMyShardId(client *c) {
    char *sid = clusterNodeGetShardId(getMyClusterNode());
    if (sid) {
        addReplyBulkCBuffer(c, sid, CLUSTER_NAMELEN);
    } else {
        addReplyError(c, "No shard ID yet");
    }
}

/* When a cluster command is called, we need to decide whether to return TLS info or
 * non-TLS info by the client's connection type. However if the command is called by
 * a Lua script or RM_call, there is no connection in the fake client, so we use
 * server.current_client here to get the real client if available. And if it is not
 * available (modules may call commands without a real client), we return the default
 * info, which is determined by server.tls_cluster. */