Re: [Bug debuginfod/28034] client-side %-escape url characters

[Mark Wielaard <mark@klomp.org>]

Hi Noah,

This looks good, but the description is not really what the patch does.
It is part of the PR28034 fix, but repeating that problem statement
doesn't really make clear what this patch is about. Could explain in
the commit message that curl_easy_escape () also escapes '/' to '%2F'
and that we want to reverse that back to '/'?

Also some (tiny) optimization hints.

This can be done outside/before the loop

  /* Initialize each handle.  */
  for (int i = 0; i < num_urls; i++)

So you only need to escape once. You of course then need to make sure
the escaped_string is freed after the loop.

We already check that the first char is a '/'. It seems silly to curl
escape that one and then unescape it again. So maybe curl_easy_escape
(data[i].handle, filename + 1, 0) and then change the snprintf pattern
to "%s/%s/%s/%s"?
            ^ the slash got readded here.

The strlen inside the while loop can also be done outside and then
calculated instead of running strlen on the tail every time.

size_t escaped_strlen = strlen (escaped_string);
while( (loc = strstr(loc, "%2F")) )
  {
     loc[0] = '/';
     // pull the string back after replacement
     escaped_strlen -= (loc + 3) - escaped_string);
     memmove(loc+1, loc+3, escaped_strlen + 1);
  }

(Note, not even compiled, so I might be wrong)

Lastly I assume there are already testcases that cover this
functionality? Just wanting to know how you tested it.

Thanks,

Mark