[FFmpeg-devel] [PATCH] Add support for digest auth in the http and rtsp protocols

[Ronald S. Bultje]

Hi,

On Wed, Mar 24, 2010 at 4:17 PM, Martin Storsj? <martin at martin.st> wrote:
> New series attached, hopefully taking all comments into account.

Let's optimize that value-parsing a bit. Let me know if I'm going too
far or if it's ugly.

> +static char *parse_value(char *value) {
> +    char *inptr  = value;
> +    char *outptr = value;
> +    if (*inptr != '"') {
> +        while (*inptr && *inptr != ' ' && *inptr != ',')
> +            inptr++;
> +        if (*inptr) {
> +            *inptr = '\0';
> +            return inptr + 1;
> +        }
> +        return inptr;
> +    }
> +    inptr++;
> +    while (*inptr && *inptr != '"') {
> +        if (*inptr == '\\') {
> +            if (!inptr[1]) {
> +                inptr++;
> +                break;
> +            }
> +            *outptr++ = inptr[1];
> +            inptr += 2;
> +        } else {
> +            *outptr++ = *inptr++;
> +        }
> +    }
> +    if (*inptr == '"')
> +        inptr++;
> +    *outptr = '\0';
> +    return inptr;
> +}
[..]
> +    /* Parse key=value pairs. */
> +    while (*ptr) {
> +        char *key;
> +        char *value;
> +        char *equals;
> +
> +        /* Skip whitespace and potential commas. */
> +        while (*ptr && (isspace(*ptr) || *ptr == ','))
> +            ptr++;
> +
> +        if (!*ptr)
> +            break;
>
> +        key = ptr;
> +
> +        equals = strchr(ptr, '=');
> +        if (!equals)
> +            break;
> +        *equals = '\0';
> +
> +        value = equals + 1;
> +        ptr = parse_value(value);
> +
> +        callback(ctx, key, value);
> +    }

Let's see? Can you use something like this to prevent all the dups?
Perhaps a little ugly and buggy but you'll get the point.

for (;;) { <- you already check if (*ptr) in the loop
    char *dest = NULL, *dest_end;
    int dest_len, len;

    while (*ptr && (isspace(*value) || *value == ','))
        ptr++;
    if (!*ptr)
        break;
    key = ptr;
    if (!(ptr = strchr(key, '=;)))
        break;
    ptr++;
    len = ptr - key;

    /* find dest */
    callback_get_buf(&dest, &dest_len);
    dest_end = dest + dest_len - 1; // leave one value for terminating '\0'
    assert(dest_end > dest); // assure enough space is available for
at least a '\0'

    /* put value */
#define put_escaped_char(dst, end, src) \
    do { \
        if (ptr[0] == '\\' && ptr[1]) { \
            if (dest && dest_end < dest) \
                *dest++ = ptr[1]; \
            ptr += 2; \
        } else { \
            if (dest && dest_end < dest) \
                *dest++ = *ptr; \
            ptr++; \
        } \
    } while (0)

    if (*ptr == '\"') {
        ptr++;
        while (!(ptr[-1] != '\\' && ptr[0] == '\"'))
            put_escaped_char(dest, dest_end, ptr);
    } else {
        while (!(ptr[-1] != '\\' && (isspace(ptr[0]) || ptr[0] == ',')))
            put_escaped_char(dest, dest_end, ptr);
    }
}

The implementation of callback_dest_buf() should be self-evident.

> +void ff_http_auth_handle_header(HTTPAuthState *state, const char *key,
> +                                const char *value)
[..]
> +        if (av_stristart(value, "Basic ", &p)) {
> +            char *copy;
> +            if (state->auth_type > HTTP_AUTH_BASIC)
> +                return;

if (av_stristart() && state->auth_type <= HTTP_AUTH_BASIC)

> +            state->auth_type = HTTP_AUTH_BASIC;
> +            state->realm[0] = '\0';
> +            copy = av_strdup(p);
> +            if (!copy)
> +                return;
> +            parse_key_value(copy, handle_basic_params, state);

Using the above code, no copy should be necessary.

> +static void handle_basic_params(void *ctx, char *key, char *value)
> +{
> +    HTTPAuthState *state = ctx;

Why void?

> +static void parse_key_value(char *params,
> +                            void (*callback)(void *ctx, char *key, char *value),
> +                            void *ctx)

Again void.

Ronald