Insphpect

This tool is currently proof-of-concept. Your feedback and evaluation is valuable in helping to improve it and ensure its reports are meaninful.

Please click here to complete a short survey to tell us what you think. It should take less than 5 minutes and help further this research project!

GuzzleHttp\Handler\CurlHandler

Detected issues

Issue Method Line number
Using `new` in constructor __construct 34

Code

Click highlighted lines for details

<?phpnamespace GuzzleHttp\Handler;use GuzzleHttp\Promise\PromiseInterface;use Psr\Http\Message\RequestInterface;/** * HTTP handler that uses cURL easy handles as a transport layer. * * When using the CurlHandler, custom curl options can be specified as an * associative array of curl option constants mapping to values in the * **curl** key of the "client" key of the request. * * @final */class CurlHandler{    /**     * @var CurlFactoryInterface     */    private $factory;    /**     * Accepts an associative array of options:     *     * - factory: Optional curl factory used to create cURL handles.     *     * @param array $options Array of options to use with the handler     */    public function __construct(array $options = [])    {        $this->factory = $options['handle_factory']

Usage of the new keyword in a constructor

Summary of issues

Background

If a dependency is constructed inside the object that requires it rather than passed in as a reference then flexibility is lost[1][2]

 public class Car {

    private 
Engine engine;
    
    public 
Car() {
        
this.engine = new Engine();
    }
}

Here, the Car constructor creates the Engine instance. This is inflexible as it forces all Car objects to use the exact same Engine type. Instead, it would encourage reuse if the program supported different engine types (e.g. DieselEngine, PetrolEngine or HybridEngine).

The same is true when an instance variable is created when the class is defined:

 public class Car {
    private 
Engine engine = new Engine();
}

By using the new keyword to instantiate a dependency, the specific implementation of that dependency is hardcoded and cannot be substituted.

Instead, the dependency should be constructed outside the class and injected in:

 public class Car {
    private 
Engine engine;

    public 
Car(Engine engine) {
        
this.engine engine;
    }
}

Using dependency injection it is possible to pass in any engine type:

 //Instead of
Car myCar = new Car();

//It's now possible to construct different types of car:
Car petrolCar = new Car(new PetrolEngine);
Car electricCar = new Car(new ElectricEngine);

A secondary advantage to Dependency Injection with regards to flexibility and encapsulation is that the class which has the dependency (Car, in this example) it not aware of the dependencies of the Engine class.

For example, if the Engine class required a Gearbox instance as a constructor argument, the Car class would need to instantiate and pass in the relevant Gearbox instance. And provide any dependencies of the Gearbox class when instantiating it.

If the constructor arguments of any of the classes which need to be instantiated are modified during development, any class which creates an instance of the class must also be modified. A change to the constructor for Engine would require modifying the Car class. Instead, if the fully constructed Engine instance

By loosely coupling the Engine class to the Car class, the author of the Car class does not need to know anything about the implementation of Engine class or have knowledge of what dependencies it has.

 
public Car() {

    
this.engine = new Engine(new Gearbox());
}

Further reading

Additional resources:

References

  1. Hevery, M. (2008) How to Think About the “new” Operator with Respect to Unit Testing [online]. Available from: http://misko.hevery.com/2008/07/08/how-to-think-about-the-new-operator/
  2. Hevery, M. (2008) Code Reviewers Guide [online]. Available from: http://misko.hevery.com/code-reviewers-guide/

Please note: This feature is currently proof-of-concept, the instructions may not always be completely accurate.

\GuzzleHttp\Handler\CurlFactory is instantiated inside the constructor of GuzzleHttp\Handler\CurlHandler

1) Remove the new expression and replace it with a variable:

 

?? new CurlFactory(3);

becomes:

 

            ?? $curlFactory;

2) Add a constructor argument for the new variable: Replace:

 

public function __construct(array $options = [])

with:

 

public function __construct(\GuzzleHttp\Handler\CurlFactory $curlFactory, array $options = [])

3) Find any occurance of new GuzzleHttp\Handler\CurlHandler and provide the new dependency.

\GuzzleHttp\Handler\CurlHandler is being instantiated in src/Utils.php:0

Replace:

 

$handler Proxy::wrapSync(new CurlMultiHandler(), new CurlHandler());

With:

 

$handler Proxy::wrapSync(new CurlMultiHandler(), new CurlHandler(new \GuzzleHttp\Handler\CurlFactory(3)));



\GuzzleHttp\Handler\CurlHandler is being instantiated in src/Utils.php:0

Replace:

 

$handler = new CurlHandler();

With:

 

$handler = new CurlHandler(new \GuzzleHttp\Handler\CurlFactory(3));



Please note: This feature is currently proof-of-concept, this patch may not work, please don't blindly apply it.

diff --git a/src/Client.php b/src/Client.php
index 81f4f6d..8c77d1e 100644
--- a/src/Client.php
+++ b/src/Client.php
@@ -257,7 +257,7 @@ class Client implements ClientInterface, \Psr\Http\Client\ClientInterface
         $this->config = $config + $defaults;

         if (!empty($config['cookies']) && $config['cookies'] === true) {
-            $this->config['cookies'] = new CookieJar();
+            $this->config['cookies'] = new CookieJar(new \GuzzleHttp\Cookie\SetCookie(new SetCookie()));
         }

         // Add the default user-agent header.
diff --git a/src/Cookie/CookieJar.php b/src/Cookie/CookieJar.php
index d6757c6..a6cfe90 100644
--- a/src/Cookie/CookieJar.php
+++ b/src/Cookie/CookieJar.php
@@ -27,13 +27,13 @@ class CookieJar implements CookieJarInterface
      *                           arrays that can be used with the SetCookie
      *                           constructor
      */
-    public function __construct(bool $strictMode = false, array $cookieArray = [])
+    public function __construct(\GuzzleHttp\Cookie\SetCookie $setCookie, bool $strictMode = false, array $cookieArray = [])
     {
         $this->strictMode = $strictMode;

         foreach ($cookieArray as $cookie) {
             if (!($cookie instanceof SetCookie)) {
-                $cookie = new SetCookie($cookie);
+                $cookie = $setCookie;
             }
             $this->setCookie($cookie);
         }
@@ -310,4 +310,4 @@ class CookieJar implements CookieJarInterface
             );
         }
     }
-}
+}
\ No newline at end of file
diff --git a/src/Handler/CurlHandler.php b/src/Handler/CurlHandler.php
index 47e21f0..8d26b67 100644
--- a/src/Handler/CurlHandler.php
+++ b/src/Handler/CurlHandler.php
@@ -28,10 +28,10 @@ class CurlHandler
      *
      * @param array $options Array of options to use with the handler
      */
-    public function __construct(array $options = [])
+    public function __construct(\GuzzleHttp\Handler\CurlFactory $curlFactory, array $options = [])
     {
         $this->factory = $options['handle_factory']
-            ?? new CurlFactory(3);
+            ?? $curlFactory;
     }

     public function __invoke(RequestInterface $request, array $options): PromiseInterface
@@ -46,4 +46,4 @@ class CurlHandler

         return CurlFactory::finish($this, $easy, $this->factory);
     }
-}
+}
\ No newline at end of file
diff --git a/src/Utils.php b/src/Utils.php
index 91591da..dc76214 100644
--- a/src/Utils.php
+++ b/src/Utils.php
@@ -87,9 +87,9 @@ final class Utils
     {
         $handler = null;
         if (\function_exists('curl_multi_exec') && \function_exists('curl_exec')) {
-            $handler = Proxy::wrapSync(new CurlMultiHandler(), new CurlHandler());
+            $handler = Proxy::wrapSync(new CurlMultiHandler(), new CurlHandler(new \GuzzleHttp\Handler\CurlFactory(3)));
         } elseif (\function_exists('curl_exec')) {
-            $handler = new CurlHandler();
+            $handler = new CurlHandler(new \GuzzleHttp\Handler\CurlFactory(3));
         } elseif (\function_exists('curl_multi_exec')) {
             $handler = new CurlMultiHandler();
         }
} public function __invoke(RequestInterface $request, array $options): PromiseInterface { if (isset($options['delay'])) { \usleep($options['delay'] * 1000); } $easy = $this->factory->create($request, $options); \curl_exec($easy->handle); $easy->errno = \curl_errno($easy->handle); return CurlFactory::finish($this, $easy, $this->factory); }}