Take const CURLU* arg in URL observers

[rcombs]

Currently, curl_url_dup and curl_url_get each take a non-const CURLU* arg. In the case of _dup, there seems to be no reason for this; it never actually mutates the input. curl_url_get, though, actually can mutate its input; this is unintuitive and undocumented. This PR ensures that _get never mutates its input (this is a fairly trivial set of changes), and changes the arg type for those two functions to const CURLU*.

This changes the signatures for two public functions, but the result is both API- and ABI-compatible with existing code, so it shouldn't require a major semver bump.

If this change is rejected for some reason, the documentation should be updated to clearly indicate that curl_url_get can mutate its input.

[bagder]

curl_url_get, though, actually can mutate its input; this is unintuitive and undocumented

Can you elaborate on this. When or how would it "mutate" its input?

[rcombs]

See the second commit: it modifies the path member by setting it to strdup("/") if it was previously unset, and modifies the host member by freeing the existing value and replacing it with the same string rewritten to escape %s.

The latter seems like an outright bug; see the output from this test program, linked against the current code:

#include <curl/curl.h>

int main()
{
  CURLU* u = curl_url();
  curl_url_set(u, CURLUPART_URL, "https://test%test:8080/test", 0);

  char *str;
  curl_url_get(u, CURLUPART_URL, &str, 0);
  printf("%s\n", str);
  curl_url_get(u, CURLUPART_URL, &str, 0);
  printf("%s\n", str);
  curl_url_get(u, CURLUPART_URL, &str, 0);
  printf("%s\n", str);

  return 0;
}

https://test%25test:8080/test
https://test%2525test:8080/test
https://test%252525test:8080/test

The function escapes percent signs in host, but then saves the resulting escaped string back into the object. This means that each time we request the CURLUPART_URL, the host gets another 25! This bug would've been much more difficult to introduce had the argument been const to begin with.

[bagder]

That "mutation" is a bug.

[rcombs]

Makes sense; so, this PR fixes that bug, and also makes it more difficult for similar bugs to recur in the future.

[bagder]

Thinking further, I believe test%test is an illegal host name and should be rejected by the parser. I'll take a look at that (that's independent from this PR).

[bagder]

For all I can tell, adding const like this should be fine both API and ABI wise.

[bagder]

Thanks!