Re: [PATCH 3/4] imap-send: remove two string length checks
- From
Beat Bolli <dev+git@drbeat.li>
- Date
- Mar 11, 2026, 22:00 UTC
- Message-ID
- <d9338d30-3e73-4cb7-9f8f-cc1ce21c9de4@drbeat.li>
- In-Reply-To
- <xmqq4immp56h.fsf@gitster.g>
Hi Junio
On 11.03.2026 19:55, Junio C Hamano wrote:
Show 24 quoted lines
> Beat Bolli <dev+git@drbeat.li> writes: > >> At this point, these two checks verify that the ASN1_STRINGs are >> internally consistent. This may have been ok when the fields were >> accessed directly, but now that the API is used, is unnecessary. >> >> Remove the two checks. > > Oswald already gave a similar comment, but > > * I am not sure what you meant by "ok" in "may have been ok". Do > you mean "with raw access to the fields, it may have been made > send to ensure validity of ASN1_STRING"? > > * I am also not sure what you meant by "now that the API is used". > Who in the code uses which API function so that we do not have to > do our sanity checking? > > The call to host_matches() that these extra checks protect are > still passing raw "const char *" in this step, and the change to > pass ASN1_STRING does not happen until [4/4], so you did not mean > host_matches(). I am not sure what it is. > > Thanks.
I didn't realize that the strlen() comparison was meant to check for embedded NULs. I'll send v2 shortly that keeps this check.
Cheers, Beat
Show 28 quoted lines
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>> imap-send.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/imap-send.c b/imap-send.c
>> index 2a904314dd..2bb0003f08 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -253,8 +253,6 @@ static int verify_hostname(X509 *cert, const char *hostname)
>> ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype);
>>
>> if (ntype == GEN_DNS &&
>> - strlen((const char *)ASN1_STRING_get0_data(subj_alt_str)) ==
>> - ASN1_STRING_length(subj_alt_str) &&
>> host_matches(hostname, (const char *)ASN1_STRING_get0_data(subj_alt_str)))
>> found = 1;
>> }
>> @@ -270,8 +268,7 @@ static int verify_hostname(X509 *cert, const char *hostname)
>> (cname_entry = X509_NAME_get_entry(subj, i)) == NULL ||
>> (cname = X509_NAME_ENTRY_get_data(cname_entry)) == NULL)
>> return error("cannot get certificate common name");
>> - if (strlen((const char *)ASN1_STRING_get0_data(cname)) == ASN1_STRING_length(cname) &&
>> - host_matches(hostname, (const char *)ASN1_STRING_get0_data(cname)))
>> + if (host_matches(hostname, (const char *)ASN1_STRING_get0_data(cname)))
>> return 0;
>> return error("certificate owner '%s' does not match hostname '%s'",
>> ASN1_STRING_get0_data(cname), hostname);