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] - Fix JHTTP socket transport http version #43002

Merged
merged 4 commits into from
Mar 11, 2024

Conversation

SniperSister
Copy link
Contributor

@SniperSister SniperSister commented Mar 11, 2024

Pull Request for Issue #38963 and #42973

Summary of Changes

In #35568 a change was merged into the JHTTP socket driver, increasing the accepted HTTP version for the client from 1.0 to 1.1.

That change introduce the issue described in #38963: HTTP 1.1 defines the chunked transfer mode which is mandatory for all clients implementing HTTP 1.1 - as our socket-based client however does not support chunked responses, a chunked response causes an infinite loop.

Testing Instructions

  • Create a socket-based JHTTP request to a server responding with a chunked response, i.e. by using this code block:
    $http = \Joomla\CMS\Http\HttpFactory::getHttp([], 'socket'); $response = $http->get('https://update.joomla.org/cms/root.json');

Actual result BEFORE applying this Pull Request

  • Infinite loop

Expected result AFTER applying this Pull Request

  • Response returned

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

@HLeithner
Copy link
Member

I have tested this item ✅ successfully on f54a18f

Works now


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

@alikon
Copy link
Contributor

alikon commented Mar 11, 2024

still
when no curl
no luck
image

it's my faulty settings or ?

@SniperSister
Copy link
Contributor Author

@alikon did you follow the test instructions above (abstract code block) or did you try to fetch a Joomla update? The later one is blocked by the changes in George's PR

@alikon
Copy link
Contributor

alikon commented Mar 11, 2024

i've applyed this pr on an already not working sceanrio as per #42973

@SniperSister
Copy link
Contributor Author

In that case it won't work as the TUF updater is still pinned to the curl driver, that's being fixed in #42900

This PR is not specifically fixing the TUF issue but solving the issue generically that blocks TUF from using socket transports.

@alikon
Copy link
Contributor

alikon commented Mar 11, 2024

so it could be tested after #42900 has been merged or with #42900

right ?

@SniperSister
Copy link
Contributor Author

... or without TUF by executing the code snippet mentioned in the test instructions :)

@alikon
Copy link
Contributor

alikon commented Mar 11, 2024

me quite confused #43004 ?

@SniperSister
Copy link
Contributor Author

me quite confused #43004 ?

Totally different topic.

@LadySolveig LadySolveig merged commit e3b6e7e into joomla:5.1-dev Mar 11, 2024
3 checks passed
@LadySolveig
Copy link
Contributor

Thanks

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.

6 participants