Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.1] Change redirect url (from headers['Location']) from array to string in CurlTransport #42769

Merged
merged 3 commits into from
May 20, 2024

Conversation

sergeytolkachyov
Copy link
Contributor

@sergeytolkachyov sergeytolkachyov commented Feb 7, 2024

The problem was discovered when using HTTP Factory for an external request to the server that performs redirection. The request method executes a request to the specified url of the string type. If the response code indicates a redirect, then the redirect url is obtained from the Location header. After that, another request is executed with a new url.

image

The redirect url is an instance of the Uri class. During the operation of this class, the UrlHelper class is called, in which the url is parsed. When making the first request, the url type is a string. However, in the case of redirection, the url is an array with one element, since $response->headers['Location'] is passed immediately as an argument. Therefore, when trying to make a second request for a new URl, the error parse_url() occurs: Argument #1 ($url) must be of type string, array given in the UriHelper class line 38.

image

This screenshot shows that the content of the Location header is an array, not a string. Which is the reason for the error.
image

The problem was found only when using the CURLOPT_FOLLOWLOCATION = false parameter. In this case, Joomla executes the redirect url request on its own.

Summary of Changes

Change a $redirect_uri = new Uri($response->headers['Location']); to $redirect_uri = new Uri($response->headers['Location'][0]);

Testing Instructions

It may not be the most correct, but the fastest way to check the result of this change is to run the query directly from the template index.php file.

  1. Open it index.php Your frontend template
  2. Add the following code to it
use Joomla\CMS\Http\HttpFactory;

$options = new Registry();
$options->set(
	'transport.curl',
	[
		CURLOPT_FOLLOWLOCATION   => false, 
	]
);

$http = (new HttpFactory)->getHttp($options, ['curl', 'stream']);

$url = 'enter here any URL with redirect ';

$response = $http->get($url);
echo '<pre>';
print_r($response->body);
echo '</pre>';
  1. Run the request (refresh the page) and you will see an error parse_url(): Argument #1 ($url) must be of type string, array given
  2. Apply this PR (download the build and update Joomla 5.0.2)

image

or in the file libraries/src/Http/Transport/CurlTransport.php find the line $redirect_uri = new Uri($response->headers['Location']) and replace it to $redirect_uri = new Uri($response->headers['Location'][0]);
5. Run the request again (refresh the page again)
6. Error has gone

Actual result BEFORE applying this Pull Request

An error parse_url(): Argument #1 ($url) must be of type string, array given in ROOT\libraries\vendor\joomla\uri\src\UriHelper.php:38

image

Expected result AFTER applying this Pull Request

Error has gone

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@gug2
Copy link

gug2 commented Feb 8, 2024

I have tested this item ✅ successfully on d1032db

Tested successfully


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42769.

@progreccor
Copy link

I have tested. Work ok

@Quy Quy removed the PR-5.1-dev label Feb 12, 2024
@Quy
Copy link
Contributor

Quy commented Feb 12, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42769.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 12, 2024
@Quy Quy added PR-5.1-dev RTC This Pull Request is Ready To Commit and removed RTC This Pull Request is Ready To Commit labels Feb 12, 2024
@sergeytolkachyov
Copy link
Contributor Author

@Quy

@sergeytolkachyov
Copy link
Contributor Author

@Quy Joomla 5.1.0 Alpha 4 has been released at 20 February 2024. This pr was RTC 2 weeks ago but stll not merged.

@HLeithner
Copy link
Member

@Quy Joomla 5.1.0 Alpha 4 has been released at 20 February 2024. This pr was RTC 2 weeks ago but stll not merged.

RTC doesn't mean that it get automatically merged, it in only means that the pr has 2 tests. But still a maintainer has to check the pr and consider side effects. So it could take some time till a maintainer has time to validate the issue and check if the fix is the correct way.

@HLeithner HLeithner added the bug label Apr 24, 2024
@HLeithner HLeithner changed the title Change redirect url (from headers['Location']) from array to string in CurlTransport [5.1] Change redirect url (from headers['Location']) from array to string in CurlTransport Apr 24, 2024
@LadySolveig LadySolveig added this to the Joomla! 5.1.1 milestone May 20, 2024
@LadySolveig LadySolveig merged commit a406543 into joomla:5.1-dev May 20, 2024
2 of 3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 20, 2024
@LadySolveig
Copy link
Contributor

Thank you @sergeytolkachyov and also for testing @gug2 @progreccor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants