Bug #84171

\TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() Request-Header format changed from array of strings to associative array in GuzzleHttp conversion

Added by Felix Buenemann over 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Extbase
Target version:
-
Start date:
2018-03-07
Due date:
% Done:

100%

TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
medium
Is Regression:
Yes
Sprint Focus:

Description

In TYPO3 8 the cURL based implementation \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl() was replaced with GuzzleHttp in #70056.

This introduced a breaking change in how http headers are passed.

In the old implementation HTTP request headers where passed as an array of strings:

$url = 'https://typo3.org';
$requestHeaders = ['Accept: application/json', 'X-Foo: Bar', 'X-Foo: Baz'];
GeneralUtility::getUrl($url, 0, $requestHeaders);

In the new implementation the $requestHeaders are passed through to guzzle as part of the headers option, which wants the following format:

$url = 'https://typo3.org';
$requestHeaders = ['Accept' => 'application/json', 'X-Foo' => ['Bar', 'Baz']];
GeneralUtility::getUrl($url, 0, $requestHeaders);

Due to the different format code that uses GeneralUtility::getUrl() and requires request headers breaks when upgrading from TYPO3 7 to 8 or 9.

I think this is a bug and not an intentional change, since all the changes made as part of #70056 try to emulate the old cURL behavior of getUrl().


Related issues

Related to TYPO3 Core - Feature #70056: Use guzzle Closed 2015-09-23

Associated revisions

Revision 662fb9ae (diff)
Added by Felix Buenemann over 1 year ago

[BUGFIX] Restore getUrl support for list of headers

The change of \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl()
from cURL to GuzzleHttp the format of the $requestHeaders param was
implicitly changed from an array of header strings to an associative
array where the key is the header name and the value is either a single
or an array of values for that header.

This adds back support for the old list of headers format by detecting a
non-associative array and converting it to the Guzzle key/value(s) style.

At the same time the 'old' way is deprecated.

Resolves: #84171
Related: #70056
Releases: master, 8.7
Change-Id: I41b23993957288dfd5294129fa8039aab717461d
Reviewed-on: https://review.typo3.org/56046
Tested-by: TYPO3com <>
Reviewed-by: Frank Naegler <>
Tested-by: Frank Naegler <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

Revision 284411c1 (diff)
Added by Felix Buenemann over 1 year ago

[BUGFIX] Restore getUrl support for list of headers

The change of \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl()
from cURL to GuzzleHttp the format of the $requestHeaders param was
implicitly changed from an array of header strings to an associative
array where the key is the header name and the value is either a single
or an array of values for that header.

This adds back support for the old list of headers format by detecting a
non-associative array and converting it to the Guzzle key/value(s) style.

At the same time the 'old' way is deprecated.

Resolves: #84171
Related: #70056
Releases: master, 8.7
Change-Id: I41b23993957288dfd5294129fa8039aab717461d
Reviewed-on: https://review.typo3.org/56149
Tested-by: TYPO3com <>
Reviewed-by: Frank Naegler <>
Tested-by: Frank Naegler <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

History

#1 Updated by Felix Buenemann over 1 year ago

#2 Updated by Felix Buenemann over 1 year ago

  • Is Regression set to Yes

#3 Updated by Wouter Wolters over 1 year ago

This breaking change is IMO intentional because that is the new format the Guzzle RequestFactory uses. The new implementation is described in the RST file in the patch how to use the new headers.

#4 Updated by Gerrit Code Review over 1 year ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56046

#5 Updated by Felix Buenemann over 1 year ago

I proposed a change in Gerrit that restores compatibility with the old style without breaking the new style.

#6 Updated by Gerrit Code Review over 1 year ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56046

#7 Updated by Felix Buenemann over 1 year ago

Wouter Wolters: The bad thing is that this currently breaks code silently, no exception is thrown for code that uses the old list of headers format, but the headers will not be sent.

For example I was using it to do conditional GET requests with an If-None-Match Header and after upgrading to 8.7 ist startet always fetching the full response, because the header was not correctly added.

So it would be great if someone could take a look at my Gerrit change that adds backwards compatibility.

#8 Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56046

#9 Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56046

#10 Updated by Gerrit Code Review over 1 year ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56046

#11 Updated by Gerrit Code Review over 1 year ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56046

#12 Updated by Gerrit Code Review over 1 year ago

Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56149

#13 Updated by Felix Buenemann over 1 year ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#14 Updated by Gerrit Code Review over 1 year ago

  • Status changed from Resolved to Under Review

Patch set 2 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56149

#15 Updated by Felix Buenemann over 1 year ago

  • Status changed from Under Review to Resolved

#16 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF