Add support for httplug #112

aaa2000 · 2017-06-25T10:44:09Z

Close #107

Fix failed tests and skipped test testImageFile
Update composer dependencies to a stable version
Check on a wallabag instance

coveralls · 2017-06-25T10:56:03Z

Coverage increased (+1.3%) to 98.42% when pulling 0394fc6 on aaa2000:support-httplug into 9018618 on j0k3r:master.

jtojnar · 2017-06-25T11:16:36Z

composer.json

+        "php-http/client-implementation": "^1.0",
+        "php-http/httplug": "^1.0",
+        "php-http/message-factory": "^1.0",
+        "php-http/discovery": "^1.0",


This will require PHP 5.5.

This is fine to me.
Jumping to httplug will force a new major release (ie 2.0.0) so we can increase the minimum version of PHP in that case.

Oh, did not notice Wallabag is switching to PHP 5.6, at selfoss we still support 5.4 at least until Debian wheezy EOL.

Add PHP 5.5 requirement

I see that j0k3r/safecurl will no longer be used, I must analyse his behaviour to reimplement with httplug

j0k3r

Really good job!

j0k3r · 2017-06-26T07:44:23Z

README.md

-    composer require j0k3r/graby
+    composer require j0k3r/graby php-http/guzzle5-adapter
+
+Why `php-http/guzzle6-adapter`? Because Graby is decoupled form any HTTP messaging client with help by [HTTPlug](http://httplug.io/).


Shouldn't this be php-http/guzzle5-adapter?
Or the line above is wrong and should be php-http/guzzle6-adapter

j0k3r · 2017-06-26T07:45:41Z

src/Extractor/HttpClient.php

-                        // add referer for picky sites
-                        'Referer' => $this->getReferer($url, $httpHeader),
-                    ],
-                    'timeout' => $this->config['timeout'],


We can't configure timeout anymore?

Unfortunately, not easily. Each adapter has a different configuration. We must configure a client manually and inject it

I think this should be defined somewhere in the README

There is a paragraph Timeout configuration in the README

j0k3r · 2017-06-26T07:49:23Z

tests/Extractor/HttpClientTest.php

-        $client->expects($this->any())
-            ->method('get')
-            ->willReturn($response);
+        for ($i = 0; $i < 10; $i++) {


Why will it be called 10 times?

The configuration of redirection by httplug was incorrect and the test was not well implemented, I changed it. Is it better ?

j0k3r · 2017-06-26T07:50:58Z

tests/GrabyTest.php

-            ->method('get')
-            ->willReturn($response);
+            ],
+            <<<"HTML"


coveralls · 2017-06-26T22:34:50Z

Coverage increased (+1.3%) to 98.406% when pulling eb2d4d9 on aaa2000:support-httplug into 9018618 on j0k3r:master.

coveralls · 2017-07-01T09:46:47Z

Coverage increased (+1.3%) to 98.406% when pulling a75cefd on aaa2000:support-httplug into 9018618 on j0k3r:master.

aaa2000 · 2017-10-12T21:29:19Z

@j0k3r I wish to test temporarily with a fork of httplug but I have the error

Failed to clone the git@github.com:aaa2000/client-common.git repository, try running in interactive mode so that you can enter your GitHub credentials
                                                                                                                                                               
  [RuntimeException]                                                                                                                                            

  Failed to execute git clone --mirror 'git@github.com:aaa2000/client-common.git' '/home/travis/.composer/cache/vcs/git-gh.neting.cc-aaa2000-client-common.git/'

Do you know how to correct ?

j0k3r · 2017-10-13T06:08:18Z

Looks like it was a temporary hit limit I've relaunch build and it works fine now.

j0k3r · 2017-10-13T09:46:10Z

@aaa2000 if you want to join Gitter we could talk about that

coveralls · 2017-10-19T22:10:41Z

Coverage decreased (-0.3%) to 98.528% when pulling 0135e00 on aaa2000:support-httplug into c467f10 on j0k3r:master.

coveralls · 2017-10-19T22:18:18Z

Coverage decreased (-0.5%) to 98.296% when pulling a9aa6a0 on aaa2000:support-httplug into c467f10 on j0k3r:master.

j0k3r

~~You should remove your "php-http/client-common": "dev-cookie-date as 1.4.x-dev", from composer.json~~
OK I reviewed only the first new commit 😓

j0k3r · 2017-10-20T05:32:37Z

src/HttpClient/Plugin/ServerSideRequestForgeryProtection/Options.php

+    }
+
+    /**
+     * Sets a list to the passed in array.


to the passed what?

Copy paste from safecurl, I will fix it later

j0k3r · 2017-10-20T05:39:10Z

tests/GrabyTest.php

     * @dataProvider dataForSinglePage
     */
    public function testSinglePage($url, $expectedUrl, $singlePageUrl)
    {
+        DnsMock::withMockedHosts([
+            'singlepage1.com' => [['type' => 'A', 'ip' => '93.184.216.34']],


Is it some random IP?

IP from example.com

Maybe you should put it in a constant to avoid XX updates if example.com changes its dns

Others are random ip. 93.184.216.34 is example.com. The test use a http mock client, so a change doesn't impact the test. I will add a constant for the IP, it improves readability

coveralls · 2017-10-21T11:06:21Z

Coverage decreased (-0.3%) to 98.529% when pulling 8fd760e on aaa2000:support-httplug into c467f10 on j0k3r:master.

coveralls · 2017-10-21T12:33:11Z

Coverage decreased (-0.3%) to 98.529% when pulling 0299d30 on aaa2000:support-httplug into c467f10 on j0k3r:master.

coveralls · 2017-10-21T13:21:20Z

Coverage decreased (-0.3%) to 98.529% when pulling 595999a on aaa2000:support-httplug into c467f10 on j0k3r:master.

coveralls · 2017-10-21T14:23:39Z

Coverage decreased (-0.3%) to 98.525% when pulling 06ff069 on aaa2000:support-httplug into c467f10 on j0k3r:master.

aaa2000 · 2017-10-21T14:29:04Z

...lient/Plugin/ServerSideRequestForgeryProtection/ServerSideRequestForgeryProtectionPlugin.php

+    {
+        $this->options = $options ?: new Options();
+        $this->uriFactory = $uriFactory ?: UriFactoryDiscovery::find();
+        //TODO force using IPV4 ? @see SafeCurl::init()


I don't think that It is possible to do that with php-http

Maybe we don't need it 🤔

coveralls · 2017-10-21T14:30:26Z

Coverage decreased (-0.3%) to 98.525% when pulling e23ab8a on aaa2000:support-httplug into c467f10 on j0k3r:master.

aaa2000 · 2017-10-21T14:31:38Z

...lient/Plugin/ServerSideRequestForgeryProtection/ServerSideRequestForgeryProtectionPlugin.php

+ *
+ * @see https://github.com/j0k3r/safecurl
+ * @see https://whitton.io/articles/safecurl-ssrf-protection-and-a-capture-the-bitcoins/
+ */


Maybe, the plugin should be extract in his own repository

Might a good idea.

coveralls · 2017-10-29T12:16:07Z

Coverage decreased (-0.4%) to 98.424% when pulling 90eb474 on aaa2000:support-httplug into 19c8a37 on j0k3r:2.0.

j0k3r · 2017-10-30T06:41:02Z

src/Extractor/HttpClient.php

-                        // add referer for picky sites
-                        'Referer' => $this->getReferer($url, $httpHeader),
-                    ],
-                    'timeout' => $this->config['timeout'],


I think this should be defined somewhere in the README

j0k3r · 2017-10-30T06:41:22Z

composer.json

+        "php-http/message-factory": "^1.0",
+        "php-http/discovery": "^1.0",
+        "php-http/client-common": "^1.6@dev",
+        "php-http/message": "^1.6@dev",


Do you really need the @dev?

Not anymore, fixed.

coveralls · 2017-10-31T21:03:14Z

Coverage decreased (-0.4%) to 98.424% when pulling 0fb652a on aaa2000:support-httplug into 19c8a37 on j0k3r:2.0.

coveralls · 2017-11-01T17:35:33Z

Coverage decreased (-0.4%) to 98.424% when pulling 6003896 on aaa2000:support-httplug into 19c8a37 on j0k3r:2.0.

This reverts commit 588bd5f.

…-fixer

In order to void endless head request

coveralls · 2017-11-01T20:10:44Z

Coverage decreased (-0.4%) to 98.424% when pulling bdf4208 on aaa2000:support-httplug into 19c8a37 on j0k3r:2.0.

j0k3r · 2017-11-03T06:40:07Z

👏 !

jtojnar reviewed Jun 25, 2017

View reviewed changes

j0k3r reviewed Jun 26, 2017

View reviewed changes

j0k3r added the enhancement label Jun 26, 2017

jtojnar mentioned this pull request Jun 27, 2017

Do you use PHP 5.4? fossar/selfoss#962

Closed

j0k3r mentioned this pull request Oct 5, 2017

Support for Guzzle 6? #121

Closed

aaa2000 force-pushed the support-httplug branch 2 times, most recently from f40a16c to 9d0c9ad Compare October 12, 2017 20:55

j0k3r reviewed Oct 20, 2017

View reviewed changes

aaa2000 commented Oct 21, 2017

View reviewed changes

j0k3r changed the base branch from master to 2.0 October 23, 2017 13:42

j0k3r added this to the 2.0 milestone Oct 23, 2017

j0k3r reviewed Oct 30, 2017

View reviewed changes

aaa2000 force-pushed the support-httplug branch from 6003896 to 253e615 Compare November 1, 2017 17:33

aaa2000 added 24 commits November 1, 2017 21:01

Documents timeout configuration

68b8b75

Fix rebase error

fa939af

Fix testWith404ResponseWithResponse

ea5596b

Reenable httplug skipped test

4bec73a

Use php-http/client-common fork while waiting

aaa57cc

Try to run test in travis with the php-http/client-common fork

fba8dbb

Revert "Try to run test in travis with the php-http/client-common fork"

ba5a098

This reverts commit 588bd5f.

Protects against Server-Side Request Forgery attacks

1c51b2e

SSRF Plugin throws RequestException

bc222b1

Removes code that is no longer useful

f2bbf89

Remove fork repository

7e02657

Fix phpcs

c332ac3

Fix port validation after the addition of strict comparison of php-cs…

fcfcab9

…-fixer

Use PHPUnit 4.8 syntaxe for PHP5.5

e9f610c

Use a constant for example.com IP

97051b7

Fix phpcs

181d1dd

Update php-http/guzzle5-adapter dev dependencies

788c8f9

In order to void endless head request

Fix scrutinizer issues

efa4177

Fix description in Phpdoc

393ad3b

Use repository j0k3r/httplug-ssrf-plugin

4e98fff

Use php-http stable depedencies

d41c2ff

Remove fork repository

3596267

Use stable version for httplug-ssrf-plugin

e5c1a4d

Interface not useful

bdf4208

aaa2000 force-pushed the support-httplug branch from 253e615 to bdf4208 Compare November 1, 2017 20:03

j0k3r merged commit ebb3d80 into j0k3r:2.0 Nov 3, 2017

j0k3r mentioned this pull request Nov 3, 2017

Tracking 2.0 release #128

Merged

jtojnar mentioned this pull request Mar 2, 2021

Fetch sources in parallel fossar/selfoss#1249

Open

Add support for httplug #112

Add support for httplug #112

Conversation

aaa2000 commented Jun 25, 2017 • edited Loading

coveralls commented Jun 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j0k3r left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jun 26, 2017

coveralls commented Jul 1, 2017

aaa2000 commented Oct 12, 2017

j0k3r commented Oct 13, 2017

j0k3r commented Oct 13, 2017

coveralls commented Oct 19, 2017

coveralls commented Oct 19, 2017

j0k3r left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 21, 2017

coveralls commented Oct 21, 2017

coveralls commented Oct 21, 2017

coveralls commented Oct 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaa2000 Oct 31, 2017 • edited Loading

Choose a reason for hiding this comment

coveralls commented Oct 31, 2017

coveralls commented Nov 1, 2017

coveralls commented Nov 1, 2017

j0k3r commented Nov 3, 2017

aaa2000 commented Jun 25, 2017 •

edited

Loading

j0k3r left a comment •

edited

Loading

aaa2000 Oct 31, 2017 •

edited

Loading