[FFmpeg-devel] [PATCH v2 1/1] avformat: Add IPFS protocol support.

[Tomas Härdin]

tis 2022-02-01 klockan 22:58 +0100 skrev Mark Gaiser:

> 
> +typedef struct Context {
> +    AVClass *class;
> +    URLContext *inner;
> +    char *gateway;

Is there not a maximum length that an HTTP URL can be? At least without
query parameters. That way you avoid dynamic allocations. You'd have to
separate the AVOption from such a buffer in that case, but I think you
have to anyway.

> +    if (!ipfs_full_data_folder) {
> +        av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> +
> +        // Try via the home folder.
> +        home_folder = getenv("HOME");
> +        ipfs_full_data_folder = av_asprintf("%s/.ipfs/",
> home_folder);

Memory leak. This applies to most if not all av_asprintf() calls.

> +
> +        // Stat the folder. It should exist in a default IPFS setup
> when run as local user.
> +#ifndef _WIN32
> +        stat_ret = stat(ipfs_full_data_folder, &st);
> +#else
> +        stat_ret = win32_stat(ipfs_full_data_folder, &st);
> +#endif

Why bother with stat() when you can just check whether fopen()
succeeded?

> +// For now just makes sure that the gateway ends in url we expect.
> Like http://localhost:8080/.
> +// Explicitly with the traling slash.
> +static void ff_sanitize_ipfs_gateway(URLContext *h)
> +{
> +    Context *c = h->priv_data;
> +    const char last_gateway_char = c->gateway[strlen(c->gateway) -
> 1];

Can strlen(c->gateway) be zero here?

> +static int translate_ipfs_to_http(URLContext *h, const char *uri,
> int flags, AVDictionary **options)
> +{
> +    const char *ipfs_cid;
> +    const char *protocol_path_suffix = "ipfs/";
> +    char *fulluri;
> +    int ret;
> +    Context *c = h->priv_data;
> +    int is_ipfs = (av_strstart(uri, "ipfs://", &ipfs_cid) ||
> av_strstart(uri, "ipfs:", &ipfs_cid));
> +    int is_ipns = (av_strstart(uri, "ipns://", &ipfs_cid) ||
> av_strstart(uri, "ipns:", &ipfs_cid));

https://docs.ipfs.io/concepts/ipfs-gateway/ claims ipfs:// is the
canonical form. No mentioned is made of any ipfs:{CID} form. Incorrect
URLs should be rejected, not silently patched.

Also what happens if c->gateway is "ipfs://[...]"? Infinite recursion?

/Tomas