Fix #16929: curl_getinfo($ch, CURLINFO_CONTENT_TYPE) returns false when Content-Type header is not set

[MarcusXavierr]

This PR addresses an inconsistency in how curl_getinfo($ch, CURLINFO_CONTENT_TYPE) handles cases where the Content-Type header is missing or empty. Reported on issue #16929

Previously, curl_getinfo could return false when using the specific CURLINFO_CONTENT_TYPE flag, but NULL when accessing the content_type key from the associative array returned by curl_getinfo($ch).

Now, it'll return NULL in both situations.

[ondrejcech]

@MarcusXavierr I believe that returning NULL values is better because false is returned in case of error as described in php docs.

If option is given, returns its value. Otherwise, returns an associative array with the following elements (which correspond to option), or false on failure:

Returning false would mean the call ended with an error, but curl_error() is empty.

[MarcusXavierr]

@ondrejcech yep, it seems it should return NULL when someone calls curl_getinfo($ch, CURLINFO_CONTENT_TYPE)

But should this function call return NULL just to this option? Because all string responses from curl_getinfo seem to have false as a fallback

php-src/ext/curl/interface.c

Lines 2663 to 2670 in ba8e3e1

	case CURLINFO_STRING:
	{
	char *s_code = NULL;
	if (curl_easy_getinfo(ch->cp, option, &s_code) == CURLE_OK && s_code) {
	RETURN_STRING(s_code);
	} else {
	RETURN_FALSE;

Edit: I think adding a new case on the switch is the cleanest solution 🤔

[ondrejcech]

I think returning false is confusing because it makes it difficult to distinguish between a fallback value and an actual error.

I agree, that adding a new case on the switch is the cleanest for my problem.

[cmb69]

Looks like this is already pretty inconsistent. In some cases false is returned, in other cases the array element is not there. Now, returning null only in this case might not be the best idea. I'm not sure, though, what's the best way forward.

[ondrejcech]

OK, I understand. Can we at least make curl_getinfo($ch, CURLINFO_CONTENT_TYPE) consistent with the result of curl_getinfo($ch)['content_type']? The first will return false, the second NULL.

[MarcusXavierr]

@cmb69 that's the point. from what I've read on the code, if the request was successful, but the Content-Type was missing, the only possible response from now on is NULL. And wouldn't the content_type array key not be set only when there's an error on curl_easy_getinfo(ch->cp, CURLINFO_CONTENT_TYPE, &s_code)?

php-src/ext/curl/interface.c

Lines 2496 to 2504 in ba8e3e1

	if (curl_easy_getinfo(ch->cp, CURLINFO_CONTENT_TYPE, &s_code) == CURLE_OK) {
	if (s_code != NULL) {
	CAAS("content_type", s_code);
	} else {
	zval retnull;
	ZVAL_NULL(&retnull);
	CAAZ("content_type", &retnull);
	}
	}

[cmb69]

I had a closer look: as is, curl_getinfo() with $option either returns the value or false. curl_getinfo() without $option only set array elements which have a value. Now changing curl_getinfo($ch, CURLINFO_CONTENT_TYPE) to return null if there is no Content-Type header would be inconsistent with the other options. Changing curl_getinfo($ch) to set the content_type to false would likely do more harm than good (code using isset() works fine for missing elements as well as null elements).

Thinking about that further, the different behavior regarding returning false and not setting the array element is not really inconsistent; think of curl_getinfo() without $option as curl_getinfo_all(); a separate function could easily justify the behavior.

The only real inconsistency I see is that the content_type element is always set. That is the result of fixing https://bugs.php.net/46739, which, in my opinion, should have been fixed in the docs. Now the docs are still wrong (not all these elements are necessarily set in the array). "Unfixing" https://bugs.php.net/46739 after 16 years might not be the best idea, though.