Eric Sunshine <sunsh...@sunshineco.com> writes:

> ... Can you
> expand the commit message a bit to make it more self-contained? At
> minimum, perhaps show the error message you were experiencing, and
> cite (as Daniel pointed out) RFC 3986 and the bit about a "legal" URL
> not containing brackets.

Thanks for a good suggestion.


>
> Also, a natural question which pops into the head of someone reading
> this patch is whether other parts of the URL (host, user, etc.) also
> need to be handled similarly. It's possible that you audited the code
> and determined that they are handled fine already, but the reader of
> the commit message is unable to infer that. Consequently, it might be
> nice to have a sentence about that, as well ("other parts of the URL
> are already encoded, thus are fine" or "other parts of the URL are not
> subject to this problem because ...").
>
> The patch itself looks okay (from a cursory read).
>
> Thanks.
>
>> Reported-by: Doron Behar <doron.be...@gmail.com>
>> Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemar...@suse.com>
>> ---
>>  imap-send.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/imap-send.c b/imap-send.c
>> index 54e6a80fd..36c7c1b4f 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, 
>> struct credential *cred)
>>  {
>>         CURL *curl;
>>         struct strbuf path = STRBUF_INIT;
>> +       char *uri_encoded_folder;
>>
>>         if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
>>                 die("curl_global_init failed");
>> @@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf 
>> *srvc, struct credential *cred)
>>         strbuf_addstr(&path, server.host);
>>         if (!path.len || path.buf[path.len - 1] != '/')
>>                 strbuf_addch(&path, '/');
>> -       strbuf_addstr(&path, server.folder);
>> +
>> +       uri_encoded_folder = curl_easy_escape(curl, server.folder, 0);
>> +       if (!uri_encoded_folder)
>> +               die("failed to encode server folder");
>> +       strbuf_addstr(&path, uri_encoded_folder);
>> +       curl_free(uri_encoded_folder);
>>
>>         curl_easy_setopt(curl, CURLOPT_URL, path.buf);
>>         strbuf_release(&path);
>> --
>> 2.15.1.272.g8e603414b

Reply via email to