Make Curl Client HTTP/2 compatible #1137

Schrank · 2020-08-06T21:33:46Z

Refactoring Mage_HTTP_Client_Curl::parseHeaders

Manual testing scenarios (*)

Before this fails with Exception: Invalid response line returned from server: HTTP/2 301 after the change it works.

$c = new Mage_HTTP_Client_Curl();
$c->get('https://facebook.com');

Questions or comments

I'm not sure wether HTTP/2 allows Status Codes (line 473)
this is PHP 7.1 code (not sure what openmage is expecting), list shortcut and return void

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All automated tests passed successfully (all builds are green)

sreichel

Thanks fior PR.

Again ... what about short array syntax? I'd prefer it and see a huge PR incoming ... :P.

theroch · 2020-08-07T12:21:32Z

Thanks for your PR.
Should Openmage support PHP 7.0 anymore? PHP 7.0 is EOL since over a year and most linux distributions supports newer php versions. PHP 7.1 is availabe since december 2016.

theroch

Approved for PHP 7.1 and up

theroch

Code is now compatible to PHP 7.0 and newer

sreichel · 2020-08-07T22:41:43Z

Questions or comments

this is PHP 7.1 code (not sure what openmage is expecting), list shortcut and return void

I'd like to make use of it, but not in the main release line. Guess dropping php-7.0 support would break a lot installations.

@colinmollenhour @Flyingmana not sure about planned release strategy, but setting min requirements to php-7.3 for develop/future branch could be a start.

lib/Mage/HTTP/Client/Curl.php

tmotyl

add support for http 1.0

tmotyl

code should be made 7.0 compatible

tmotyl · 2021-05-07T12:38:39Z

hmm, I see it's already 7.0 compatible.
Alternative solution would be to force http 1.1 by adding:

        $this->curlOption(CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);

here:
https://github.com/openmage/magento-lts/blob/1.9.4.x/lib/Mage/HTTP/Client/Curl.php#L354

I've tested the patch on php 7.4 and 7.0 and it works

@sreichel would you mind taking a look again?

tmotyl · 2021-05-07T13:31:57Z

some related links from community:
https://community.magento.com/t5/Magento-2-x-Version-Upgrades/Invalid-response-line-returned-from-server-HTTP-2-200/td-p/113833/page/2
https://magento.stackexchange.com/questions/249737/magento-2-invalid-response-line-returned-from-server-http-2-200

- I'm not sure wether HTTP/2 allows Status Codes (line 473) - this is PHP 7.1 code (not sure what openmage is expecting), list shortcut and return void

Remove list shortcut and void return type

Co-authored-by: sv3n <[email protected]>

fballiano · 2022-07-18T11:30:14Z

Sorry guys I rebased the PR and made a small change to 2 "if" statements (since the notation that was used is almost never used in our code)

* Merge PR #2342 * Revert "Add basic text for Ukraine (#2074)" (#2325) This reverts commit 33dfa26. * Mage_Catalog_Model_Product_Attribute_Backend_Groupprice_Abstract: avoid loading all websites when using only the current one (#2351) * Added support for HTTP2 to Mage_HTTP_Client_Curl (#1137) * Blocked access to all dot files (#2349) * Capitalization Adjustment Regarding CamelCase in Method Names (#2365) * refactor: Adjusted capitalization of two public methods. * refactor: Adjusted capitalization of one protected method. * refactor: Adjusted capitalization of where call. * chore: Removed fixed error from phpstan baseline. * Some microoptimization (#2335) * Avoid duplicate method calls * Replaced array_push() * Changed substr() third parameter * Use array_key_exists() * php7 opcode - internal functions * Enclosed error with <pre> tag for prettier error print (if developer mode is enabled). (#2368) * Updated phpstan to 1.8.2 (#2367) * Escape product titles in MSRP JavaScript (#2366) * Product names were not escaped. If contained a double quote, would break the JavaScript for MSRP/MAP * update contribution list * Update boxes.css (#2330) * Force describeTable() to use read DB adapter (#2371) * Do not install n98/n98_layouthelper (#2373) * Add apt update to XML validation workflow (#2376) * Merged PR #2375 * Replace remaining "sizeof" calls with "count" (#2369) * Remove DISCLAIMER and change Magento -> OpenMage in header (#2297) * Added label for phpstan cosmetic changes (#2384) * Added weight to salesOrderShipmentAddTrack API (#1377) * PHPStan/DOCBlock fixes (#2336) * Updated docs for email addTo() (#2382) * Updated phpstan experimental (#2386) * Cosmetic changes to Mage_Payment_Model_Method_Abstract::validate() (#2388) * Replaced join() calls with implode() (#2389) * Hidden empty sub menu from backend (#2391) * Remove Thumbs.db file (#2394) * Support PHP 8.1 in composer.json (#2378) * php condition in composer.json This solves the issue related to php versions > 8.1 * Reduced condition for PHP requirement * Changed PHP requirement * Updated version in Ubuntu 22.04 based on PHP 8.1.2 * Composer.lock updated in Ubuntu 20.04 (PHP 8.1.2) * Update composer.lock * Update composer.lock * Blocked various file types in .htaccess (#2359) * Color swatches work with disparate product IDs (#2390) * Move Credit Memo at the end of the buttons list (#2392) * Version bump (#2387) * Minor fixes on 'filter_condition_callback' method _filterStoreCondition() (#2362) * add ReturnTypeWillChange to various Files catched by code style checker #2302 * Phpstan fixes (#2396) * Fixed addCrumb() * Fixed initForm() and _needToAddDummy() * Fixed addLink() * Fixed addLinkRel() * Fixed canUseCanonicalTag() * Fixed getAddUrl...() * Fixed rollBack() camelCase error reported by phpstan (#2403) * Changes default root dir in composer.json (#2401) * Fixed targetNamespace for WS-I Compliant SOAP APIs (#2405) * Updated phpstan baseline Co-authored-by: sv3n <[email protected]> Co-authored-by: Fabian Blechschmidt <[email protected]> Co-authored-by: Colin Mollenhour <[email protected]> Co-authored-by: Kevin Jakob <[email protected]> Co-authored-by: Ng Kiat Siong <[email protected]> Co-authored-by: Scott Moore <[email protected]> Co-authored-by: ADDISON <[email protected]> Co-authored-by: Justin Beaty <[email protected]> Co-authored-by: luigifab <[email protected]> Co-authored-by: Daniel Fahlke <[email protected]> Co-authored-by: leissbua <[email protected]>

colinmollenhour previously approved these changes Aug 6, 2020

View reviewed changes

github-actions bot added Component: lib/Mage Relates to lib/Mage Component: lib/* Relates to lib/* labels Aug 6, 2020

sreichel reviewed Aug 7, 2020

View reviewed changes

theroch approved these changes Aug 7, 2020

View reviewed changes

theroch previously approved these changes Aug 7, 2020

View reviewed changes

sreichel added the enhancement label Aug 7, 2020

tmotyl reviewed Aug 9, 2020

View reviewed changes

lib/Mage/HTTP/Client/Curl.php Outdated Show resolved Hide resolved

tmotyl requested changes Dec 31, 2020

View reviewed changes

sreichel added the PHP7 label Jan 10, 2021

tmotyl dismissed stale reviews from theroch and colinmollenhour via 808d1ee May 7, 2021 11:42

tmotyl approved these changes May 7, 2021

View reviewed changes

tmotyl requested changes May 7, 2021

View reviewed changes

tmotyl previously approved these changes May 7, 2021

View reviewed changes

Schrank and others added 3 commits July 18, 2022 13:27

Make Curl Client HTTP/2 compatible

9561c97

- I'm not sure wether HTTP/2 allows Status Codes (line 473) - this is PHP 7.1 code (not sure what openmage is expecting), list shortcut and return void

Make PHP 7.0 compatible

e16c6b8

Remove list shortcut and void return type

Allow HTTP/1.0

6b05a34

Co-authored-by: sv3n <[email protected]>

fballiano force-pushed the patch-4 branch from 808d1ee to 6b05a34 Compare July 18, 2022 11:27

switched the "if" notation

7129a85

fballiano dismissed tmotyl’s stale review via 7129a85 July 18, 2022 11:28

fballiano requested review from sreichel, colinmollenhour and tmotyl August 3, 2022 15:13

colinmollenhour approved these changes Aug 3, 2022

View reviewed changes

fballiano approved these changes Aug 3, 2022

View reviewed changes

fballiano merged commit 7f352b6 into OpenMage:1.9.4.x Aug 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Curl Client HTTP/2 compatible #1137

Make Curl Client HTTP/2 compatible #1137

Schrank commented Aug 6, 2020 •

edited by sreichel

Loading

sreichel left a comment

theroch commented Aug 7, 2020

theroch left a comment

theroch left a comment

sreichel commented Aug 7, 2020

Questions or comments

tmotyl left a comment

tmotyl left a comment

tmotyl commented May 7, 2021 •

edited

Loading

tmotyl commented May 7, 2021

fballiano commented Jul 18, 2022

Make Curl Client HTTP/2 compatible #1137

Make Curl Client HTTP/2 compatible #1137

Conversation

Schrank commented Aug 6, 2020 • edited by sreichel Loading

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

sreichel left a comment

Choose a reason for hiding this comment

theroch commented Aug 7, 2020

theroch left a comment

Choose a reason for hiding this comment

theroch left a comment

Choose a reason for hiding this comment

sreichel commented Aug 7, 2020

Questions or comments

tmotyl left a comment

Choose a reason for hiding this comment

tmotyl left a comment

Choose a reason for hiding this comment

tmotyl commented May 7, 2021 • edited Loading

tmotyl commented May 7, 2021

fballiano commented Jul 18, 2022

Schrank commented Aug 6, 2020 •

edited by sreichel

Loading

tmotyl commented May 7, 2021 •

edited

Loading