laminas / laminas-http

Provides an easy interface for performing Hyper-Text Transfer Protocol (HTTP) requests
https://docs.laminas.dev/laminas-http/
BSD 3-Clause "New" or "Revised" License
36 stars 27 forks source link

Changed default beauvoir for HTTP Version #59

Closed MOuli90 closed 2 years ago

MOuli90 commented 2 years ago

BC Break Report

Q A
Version 2.15.0

Summary

After upgrading from Verion 2.14.3 to Version 2.15.0 I could record that requests are sent with HTTP 1.0 instead of HTTP 1.1.

The default HTTP version is set to the const value of \Laminas\Http\Request::VERSION_11 wich is the string '1.1'. see \Laminas\Http\Client::$config

     'httpversion'     => Request::VERSION_11,

The Curl Adapter compares this string to the float value 1.1 in a typesafe way and will switch the HTTP version to 1.0. see \Laminas\Http\Client\Adapter\Curl line 445

$curlHttp = $httpVersion === 1.1 ? CURL_HTTP_VERSION_1_1 : CURL_HTTP_VERSION_1_0;

Previous behavior

Requests sended by default with HTTP version 1.1 with the Curl adapter

Current behavior

Requests sended by default with HTTP version 1.0 with the Curl adapter

How to reproduce

Some Code:

<?php

require_once 'vendor/autoload.php';

class CurlWrapper extends \Laminas\Http\Client\Adapter\Curl
{
    public function write($method, $uri, $httpVersion = 1.1, $headers = [], $body = '')
    {
        $result = parent::write($method, $uri, $httpVersion, $headers, $body);

        preg_match(
            '/^.*HTTP\/(.*)\s$/m',
            $result,
            $matches
        );

        // Should be by default 1.1
        $realHttpVersion = $matches[1];

        // $httpVersion is actually a mixed type
        if ($httpVersion == 1.1 && $realHttpVersion != $httpVersion) {
            throw new \LogicException('Unexpected HTTP version sent');
        }

        return $result;
    }

}

$clientDefault = new \Laminas\Http\Client(
    'http://jsonplaceholder.typicode.com/todos/1',
    [
        'adapter' => CurlWrapper::class,
    ]
);
$clientDefault->send(); // throws the "Unexpected HTTP version sent" exception

$clientOverwrittenByConst = new \Laminas\Http\Client(
    'http://jsonplaceholder.typicode.com/todos/1',
    [
        'adapter' => CurlWrapper::class,
        'httpversion' => \Laminas\Http\Request::VERSION_11
    ]
);
$clientOverwrittenByConst->send(); // throws the "Unexpected HTTP version sent" exception

$clientOverwrittenByFloat = new \Laminas\Http\Client(
    'http://jsonplaceholder.typicode.com/todos/1',
    [
        'adapter' => CurlWrapper::class,
        'httpversion' => 1.1
    ]
);
$clientOverwrittenByFloat->send(); // correct behaviour
laminas-bot commented 2 years ago

This package is considered feature-complete, and is now in security-only maintenance mode, following a decision by the Technical Steering Committee. If you have a security issue, please follow our security reporting guidelines. If you wish to take on the role of maintainer, please nominate yourself

You can continue using laminas/laminas-http safely. Its successor will be PSR-7 in a later revision of laminas/laminas-mvc.

MOuli90 commented 2 years ago

Releated to #57

boesing commented 2 years ago

@MOuli90 Is it possible to narrow the problem down to a specific change?

MOuli90 commented 2 years ago

@boesing Sure. The problem can be narrowed by the Change in \Laminas\Http\Client\Adapter\Curl:445

The change from == to ===. But maybe reverting to the none strict compare == is not the proper way to fix it. To be consistent to the other adapters the write method should receive the http version as string and not as float. The AdapterInterface already says that the version should be a string.

Ocramius commented 2 years ago

@MOuli90 if we can have a test case to prevent a further regression by CS, I will gladly integrate it in the code, together with a fix :+1:

boesing commented 2 years ago

Should be fixed with #61 Will release 2.15.1 now.