Stefan Beller <sbeller@xxxxxxxxxx> writes:
> +static struct string_list *read_push_options(void)
> +{
> + struct string_list *ret = xmalloc(sizeof(*ret));
> + string_list_init(ret, 1);
> +
> + while (1) {
> + char *line, *lf;
> + int len;
> +
> + line = packet_read_line(0, &len);
> +
> + if (!line)
> + break;
> +
> + /*
> + * NEEDSWORK: expose the limitations to be configurable;
> + * Once the limit can be lifted, include a way for payloads
> + * larger than one pkt, e.g use last byte to indicate if
> + * the push option continues in the next packet or implement
> + * larger packets.
> + */
> + if (len > LARGE_PACKET_MAX - 1) {
packet_read_line() calls packet_read_line_generic() that calls
packet_read() with the fixed packet_buffer[] that is sized to be
LARGE_PACKET_MAX.
Can this check even trigger?
> + /*
> + * NEEDSWORK: The error message in die(..) is not
> + * transmitted in call cases, so ideally all die(..)
> + * calls are prefixed with rp_error and then we can
> + * combine rp_error && die into one helper function.
> + */
> + rp_error("protocol error: server configuration allows push "
> + "options of size up to %d bytes",
> + LARGE_PACKET_MAX - 1);
> + die("protocol error: push options too large");
> + }
> + lf = strchr(line, '\n');
> + if (lf)
> + *lf = '\0';
packet_read_line() -> packet_read_line_generic() calls packet_read()
with PACKET_READ_CHOMP_NEWLINE flag bit; do we need this check?
> +
> + string_list_append(ret, line);
> + }
> +
> + return ret;
> +}
Other than that, looks good to me.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html