Throw exception if you are using all wrong headers #2916

Nyholm · 2021-07-17T17:05:52Z

This will fix #2911

Note that this only thrown an exception if you are using all wrong headers. Ie, it check if you send a list or a map.

GrahamCampbell

It would be also good to add some additional validation in the psr7 class to ensure headers are actually of type array<string, string>.

Nyholm · 2021-07-17T22:23:20Z

Yeah. I agree. But it is a bit tricky.

Let's discuss that further on the psr7 repo

TimWolla · 2021-10-04T18:46:25Z

src/Client.php

@@ -344,6 +345,9 @@ private function applyOptions(RequestInterface $request, array &$options): Reque
        ];

        if (isset($options['headers'])) {
+            if (array_keys($options['headers']) === range(0, count($options['headers']) - 1)) {
+                throw new RequestException('The headers array must have header name as keys.', $request);


My understanding is that this PR is not yet in any release: Should this exception be changed to InvalidArgumentException instead? I seems to be more fitting.

I think this was chosen for consistency with what similar code here does. I'm open to review this for Guzzle 8.

Looking further below within this method I am just seeing InvalidArgumentException being thrown. That was my primary motivation to suggest the change.

e.g.

guzzle/src/Client.php

Line 437 in e38c668

throw new InvalidArgumentException('sink must not be a boolean');

and

guzzle/src/Client.php

Lines 356 to 362 in e38c668

if (isset($options['multipart'])) {

throw new InvalidArgumentException('You cannot use '

. 'form_params and multipart at the same time. Use the '

. 'form_params option if you want to send application/'

. 'x-www-form-urlencoded requests, and the multipart '

. 'option to send multipart/form-data requests.');

}

@GrahamCampbell Pinging in case you didn't see my response to make sure you see this before an eventual new release.

Thanks for the ping (I had to clear all my notifications coz thousands built up and I was unable to prioritise effectively). This isn't tagged yet, right? What you are saying sounds reasonable to me. Are you able to send a PR over, so we can get this resolved before the 6.4.0 tag? :)

Sure, I will get right to it.

PR is up: #2942

… provided As discussed in PR guzzle#2916 after it was merged an `InvalidArgumentException` is more fitting, as passing an invalid `headers` array is a clear programming error that needs to be fixed and not caught. This is consistent with the validation of the other options, e.g. when using `multipart` and `form_params` at the same time. see guzzle#2916 see a2b8dd1

… provided (#2942) * Throw `InvalidArgumentException` when an incorrect `headers` array is provided As discussed in PR #2916 after it was merged an `InvalidArgumentException` is more fitting, as passing an invalid `headers` array is a clear programming error that needs to be fixed and not caught. This is consistent with the validation of the other options, e.g. when using `multipart` and `form_params` at the same time. see #2916 see a2b8dd1 * Remove obsolete import in Client.php / ClientTest.php

Throw exception if you are using all wrong headers

c7717b8

Nyholm mentioned this pull request Jul 17, 2021

strtolower() expects parameter 1 to be string, int given #2911

Closed

GrahamCampbell approved these changes Jul 17, 2021

View reviewed changes

Nyholm merged commit a2b8dd1 into guzzle:master Jul 17, 2021

TimWolla reviewed Oct 4, 2021

View reviewed changes

TimWolla mentioned this pull request Oct 13, 2021

Throw InvalidArgumentException when an incorrect headers array is provided #2942

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throw exception if you are using all wrong headers #2916

Throw exception if you are using all wrong headers #2916

Nyholm commented Jul 17, 2021

GrahamCampbell left a comment

Nyholm commented Jul 17, 2021

TimWolla Oct 4, 2021

GrahamCampbell Oct 4, 2021

TimWolla Oct 4, 2021

TimWolla Oct 13, 2021

GrahamCampbell Oct 13, 2021 •

edited

TimWolla Oct 13, 2021

TimWolla Oct 13, 2021

	if (isset($options['multipart'])) {
	throw new InvalidArgumentException('You cannot use '
	. 'form_params and multipart at the same time. Use the '
	. 'form_params option if you want to send application/'
	. 'x-www-form-urlencoded requests, and the multipart '
	. 'option to send multipart/form-data requests.');
	}

Throw exception if you are using all wrong headers #2916

Throw exception if you are using all wrong headers #2916

Conversation

Nyholm commented Jul 17, 2021

GrahamCampbell left a comment

Choose a reason for hiding this comment

Nyholm commented Jul 17, 2021

TimWolla Oct 4, 2021

Choose a reason for hiding this comment

GrahamCampbell Oct 4, 2021

Choose a reason for hiding this comment

TimWolla Oct 4, 2021

Choose a reason for hiding this comment

TimWolla Oct 13, 2021

Choose a reason for hiding this comment

GrahamCampbell Oct 13, 2021 • edited

Choose a reason for hiding this comment

TimWolla Oct 13, 2021

Choose a reason for hiding this comment

TimWolla Oct 13, 2021

Choose a reason for hiding this comment

GrahamCampbell Oct 13, 2021 •

edited