It's not need to parse a space character separately, Default case parses a space correctly.
Why do you parse it?

Why do you use the clause?

It's useful to verify multibyte characters also. For example: %TESTNUMBER/testapi/test+=+тест+++测试+
What do you think about it?

It's useful to verify 2-byte (not only 3-byte UTF-8 symbols) characters also. For example: GET /%TESTNUMBER/testapi/test===カールロックスкурл рокс!!!=== HTTP/1.1
What do you think about it?

sure why not,
Also I've try to write "curl rocks" in katakana (japanese), which seems to give "kaaru rokusu". but can also be translate as karl rocks also. and I guess "КарлРокс" mean karl rocks in cyrilics isn't it ?
Also google translate seems tu tell me that "керл Рокс" would be a better translation, is that right ?

Let it be "курл рокс" )

strlen_uri() returns the length of the given URI this function is a modified version of strlen_url in urlapi.c

strcpy_uri() copies a url to an output buffer and URL-encodes the URI accordingly this function is a modified version of strcpy_url in urlapi.c

It's "a URL". The leading U in URL is pronounced as "you" with a starting consonant sound, and thus "a URL" is correct.

@bagder, okay, thanks.

What do you think about renaming vars to: canonical_uri and canonical_query_str ?

It is not possible to read these functions now without thinking: why does this file need to almost duplicate these functions?

How do they differ and why isn't tweaking the ones in urlapi.c better?

@outscale-mgo, I think, good idea is taking if(in_query) and default clauses out from case-statement.
case '&' clause should be executing always only in if(in_query) for "without thinking".

something like:

  for(;
      *iptr;         /* until zero byte */
      iptr++) {

    if(in_query) {
      if(*iptr == '=') {
        if(!count_equal++) {
          msnprintf(optr, 4, "%%%02X", *iptr);
          optr += 3;
        } else
          *optr++=*iptr;
        continue;
      } else if (*iptr == '&')
        count_equal = 0;
    }

    switch(*iptr) {
    case '%':
      *optr++ = *iptr;
      if(iptr[1]) {
        iptr++;
        *optr++ = (char)TOUPPER(*iptr);
        if(*iptr != '%' && iptr[1]) {
          iptr++;
          *optr++ = (char)TOUPPER(*iptr);
        }
      }
      break;
    default:
      if(urlchar_needs_escaping(*iptr, in_query)) {
        msnprintf(optr, 4, "%%%02X", *iptr);
        optr += 3;
      }
      else
        *optr++=*iptr;
      break;
    }
  }

?

I don't understand why it is curl's job to encode the equals signs like this. The user has provided a URL to use. Why would the user provide a wrong one for curl to re-encode like this?

https://docs.aws.amazon.com/AmazonS3/latest/API/images/sigV4-using-query-params.png
In this document AWS say that you need to encode every parameter and arguments of the query string. problem is that curl seems to contain the whole query string as a single, so I had to find a way to determine which part need to be encode, and which part of need to be left as it.

@bagder, AWS doesn't recommend using standard UriEncode functions provided by development platform may not work because of differences in implementation and related ambiguity in the underlying RFCs.

2 small details:
the equal is for example, if an user call something like https://api.region.provider/Call&Argument====&Argument2=2= in which case, if as I've understand correctly, all equal but the first after Argument and Argument2 must be encode. (but this is a corner case, which I'm not sure is even possible)

Also I could patch the functions in url API, but it would add a lot of if/else which might be ugly (for examples curl use lovercase for hexa and AWS require upper case), also as @drTr0jan said, AWS advise to reimplement the encoding.

That's borderline crazy since a URL is already URL encoded as otherwise it's not a URL. Does the spec also clearly say what bytes it needs to re-encode?

BTW, doesn't that also imply that you should first URL decode the query string so that you get it truly canonicalized? Otherwise existing URL-encoded characters like %41 might thwart this.

Now I think about it, I do think the AWS doc is about not yet encoded URL, (so not URL).
also the issue is about using "https://${ES_DOMAIN_ENDPOIN}/tempo_data-*/event/_mapping" instead of "https://${ES_DOMAIN_ENDPOIN}/tempo_data-%2A/event/_mapping".

though it might still be useful to add an option to force encoding ? (in which case we won't have to deal with already encoded URL)

I think the issue here is that they want to make the url canonical, and to do that you need to first URL decode and then URL encode because curl always operate on a URL as input and that means it is encoded. So a * could be passed in as a * or as %2a or as %2A. They're all fine according to the URL syntax. But they will then genrate different hashes and that's not what a user wants.

This function doesn't "copy a URI" it (re-)encodes a query string according to the aws sig v4 rules.

How about instead call it encode_query() ?

This function returns the new length of a query string, it's not really a "uri". strlen_encoded_querry() maybe?

BTW, doesn't that also imply that you should first URL decode the query string so that you get it truly canonicalized? Otherwise existing URL-encoded characters like %41 might thwart this.

Why do you compare *iptr with '%'?

so if you have an encoded URL, containing '%2a' for example I can use '%2A' in the canonical header. I detect '%' so I can decode -> encode URL

What about for example %41 present in the original URL. Shouldn't that rather get decoded to A in a canonical URL?

@outscale-mgo, but you've detected '%' while case '%' yet, haven't you?

What about for example %41 present in the original URL. Shouldn't that rather get decoded to A in a canonical URL?

ok I'll do that

@outscale-mgo, but you've detected '%' while case '%' yet, haven't you?

yes, because %% is deferent from %01 which represent an ASCII character, anyway I'm going to change the algo to make a full decode -> encode.

Why do need this code?

good question, I've take inspiration from what is done in urlapi.c, where TOLOWER is use there.
TBH I don't know what's the advantage over using toupper, I've just assume it was there for a reason, maybe it was wrong, @bagder ?

You want to use Curl_raw_toupper() and not toupper() to avoid it being affected by locales.

For better understanding it can be like as:

for(ptr = (unsigned char *)url; *ptr; ptr++) {
  if (in_query) {
    switch (*ptr) {
      case '=':
        if(count_equal++)
          newlen += 2;
        newlen++;
        continue;
      case '&':
        count_equal = 0;
    }
  }
  if(urlchar_needs_escaping(*ptr, in_query))
    newlen += 2;
  newlen++;
 }

in this case, if you go into the case '=': it will bug, because, newlen will be increment twice or 4 times.
so the code should be:

for(ptr = (unsigned char *)url; *ptr; ptr++) {
  if (in_query) {
    switch (*ptr) {
      case '=':
        if(count_equal++)
          newlen += 2;
        newlen++;
        continue; // replace break by continue here.
      case '&':
        count_equal = 0;
    }
  }
  if(urlchar_needs_escaping(*ptr, in_query))
    newlen += 2;
  newlen++;
 }

TBH I don't think it simplify the code a lot, so I don't think I will change that.

Of course, continue should be there. Sorry for my typo.

CWE 197 advisory: potential loss of information in cast
(at-me in a reply with help or ignore)

hello an idea for that ?
would an

if (ascii_code > 255)
    handle_errror()

be enough ?

char is often signed, so that typecast is not limiting the value to 0 - 255. And since you want an unsigned value, maybe strtoul() is actually what you want? The fact that char is also not certain to be signed or unsigned is also a reason to use it unsigned explicitly.

The code in escape.c does this:

      hex = strtoul(hexstr, &ptr, 16);

      in = curlx_ultouc(hex); /* this long is never bigger than 255 anyway */