Re: [PATCH 3/4] imap-send: remove two string length checks

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 11, 2026, 18:55 UTC
Message-ID: <xmqq4immp56h.fsf@gitster.g>
In-Reply-To: <20260311121107.1122387-4-dev+git@drbeat.li>

Beat Bolli <dev+git@drbeat.li> writes:

Show 5 quoted lines

> 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.

Show 29 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);