Small cleanup #110

reef-actor · 2018-01-29T11:14:36Z

'remove_js' & 'replace_title' didn't seem to belong in the ProxifyPlugin, so have been given their own plugins.

ProxifyPlugin has been reorganised, primary difference is the use of named capture groups in the RegEx's to allow reuse of the callback and therefore reduce duplicated code. Image and Font content has been added to the blacklist to reduce load.

AbstractPlugin has been simplified to directly subscribe to events, removing 2 extra function calls per event per plugin.

Nitpick-CI · 2018-01-29T11:14:42Z

src/Plugin/AbstractPlugin.php

@@ -26,56 +22,16 @@ public function onCompleted(ProxyEvent $event){
 	}

 	final public function subscribe($dispatcher){
+		$event_listeners = [


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:42Z

src/Plugin/AbstractPlugin.php

@@ -26,56 +22,16 @@ public function onCompleted(ProxyEvent $event){
 	}

 	final public function subscribe($dispatcher){
+		$event_listeners = [
+			'request.before_send' => 'onBeforeRequest',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:42Z

src/Plugin/AbstractPlugin.php

@@ -26,56 +22,16 @@ public function onCompleted(ProxyEvent $event){
 	}

 	final public function subscribe($dispatcher){
+		$event_listeners = [
+			'request.before_send' => 'onBeforeRequest',
+			'request.sent' => 'onHeadersReceived',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:43Z

src/Plugin/AbstractPlugin.php

+		$event_listeners = [
+			'request.before_send' => 'onBeforeRequest',
+			'request.sent' => 'onHeadersReceived',
+			'curl.callback.write' => 'onCurlWrite',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:43Z

src/Plugin/AbstractPlugin.php

+			'request.before_send' => 'onBeforeRequest',
+			'request.sent' => 'onHeadersReceived',
+			'curl.callback.write' => 'onCurlWrite',
+			'request.complete' => 'onCompleted',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:44Z

src/Plugin/AbstractPlugin.php

+			'request.sent' => 'onHeadersReceived',
+			'curl.callback.write' => 'onCurlWrite',
+			'request.complete' => 'onCompleted',
+		];


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:44Z

src/Plugin/AbstractPlugin.php

-			case 'request.complete':
-				$this->onCompleted($event);
-			break;
+		foreach($event_listeners as $event => $listener) {


Spaces must be used to indent lines; tabs are not allowed

Expected 1 space after FOREACH keyword; 0 found

Nitpick-CI · 2018-01-29T11:14:44Z

src/Plugin/AbstractPlugin.php

-				$this->onCompleted($event);
-			break;
+		foreach($event_listeners as $event => $listener) {
+			$dispatcher->addListener($event, [$this, $listener]);


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:45Z

src/Plugin/ProxifyPlugin.php

-
-		return str_replace($matches[1], proxify_url($matches[1], $this->base_url), $matches[0]);
-	}
+	private const CONTENT_TYPE_BLACKLIST = ['image', 'font', 'application/javascript', 'application/x-javascript', 'text/javascript', 'text/plain'];


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:45Z

src/Plugin/ProxifyPlugin.php

-		return str_replace($matches[1], proxify_url($matches[1], $this->base_url), $matches[0]);
-	}
+	private const CONTENT_TYPE_BLACKLIST = ['image', 'font', 'application/javascript', 'application/x-javascript', 'text/javascript', 'text/plain'];
+	private const LINK_TYPE_BLACKLIST = ['data:', 'magnet:', 'about:', 'javascript:', 'mailto:', 'tel:', 'ios-app:', 'android-app:'];


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:46Z

src/Plugin/ProxifyPlugin.php

-	}
+	private const CONTENT_TYPE_BLACKLIST = ['image', 'font', 'application/javascript', 'application/x-javascript', 'text/javascript', 'text/plain'];
+	private const LINK_TYPE_BLACKLIST = ['data:', 'magnet:', 'about:', 'javascript:', 'mailto:', 'tel:', 'ios-app:', 'android-app:'];
+	private const CONTENT_PARSERS = [


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:46Z

src/Plugin/ProxifyPlugin.php

+	private const CONTENT_TYPE_BLACKLIST = ['image', 'font', 'application/javascript', 'application/x-javascript', 'text/javascript', 'text/plain'];
+	private const LINK_TYPE_BLACKLIST = ['data:', 'magnet:', 'about:', 'javascript:', 'mailto:', 'tel:', 'ios-app:', 'android-app:'];
+	private const CONTENT_PARSERS = [
+		'@\bcontent=(?<quote>\'|")\d+\s*;\s*url=(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',           // content="X;url=<url>" (meta-refresh)


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:46Z

src/Plugin/ProxifyPlugin.php

+	private const LINK_TYPE_BLACKLIST = ['data:', 'magnet:', 'about:', 'javascript:', 'mailto:', 'tel:', 'ios-app:', 'android-app:'];
+	private const CONTENT_PARSERS = [
+		'@\bcontent=(?<quote>\'|")\d+\s*;\s*url=(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',           // content="X;url=<url>" (meta-refresh)
+		'@\b(?:src|href)\s*=\s*(?<quote>\'|")(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',              // src="<url>" & href="<url>"


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:47Z

src/Plugin/ProxifyPlugin.php

+	private const CONTENT_PARSERS = [
+		'@\bcontent=(?<quote>\'|")\d+\s*;\s*url=(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',           // content="X;url=<url>" (meta-refresh)
+		'@\b(?:src|href)\s*=\s*(?<quote>\'|")(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',              // src="<url>" & href="<url>"
+		'@\burl\s*\((?<quote>\'|")(?<url>.*?)\k<quote>\)@im' => 'self::proxify_url_callback',                       // url(<url>)


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:47Z

src/Plugin/ProxifyPlugin.php

+		'@\bcontent=(?<quote>\'|")\d+\s*;\s*url=(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',           // content="X;url=<url>" (meta-refresh)
+		'@\b(?:src|href)\s*=\s*(?<quote>\'|")(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',              // src="<url>" & href="<url>"
+		'@\burl\s*\((?<quote>\'|")(?<url>.*?)\k<quote>\)@im' => 'self::proxify_url_callback',                       // url(<url>)
+		'@\@import\s+(?<quote>\'|")(?<url>.*?)\k<quote>@im' => 'self::proxify_url_callback',                        // @import '<url>'


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:48Z

src/Plugin/ProxifyPlugin.php

+		'@\b(?:src|href)\s*=\s*(?<quote>\'|")(?<url>.*?)\k<quote>@is' => 'self::proxify_url_callback',              // src="<url>" & href="<url>"
+		'@\burl\s*\((?<quote>\'|")(?<url>.*?)\k<quote>\)@im' => 'self::proxify_url_callback',                       // url(<url>)
+		'@\@import\s+(?<quote>\'|")(?<url>.*?)\k<quote>@im' => 'self::proxify_url_callback',                        // @import '<url>'
+		'@\b(?:srcset)\s*=\s*(?<quote>\'|")(?<value>.*?)\k<quote>@im' => 'self::proxify_srcset_attribute_callback', // srcset="<url> xxx, …"


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:48Z

src/Plugin/ProxifyPlugin.php

+		'@\burl\s*\((?<quote>\'|")(?<url>.*?)\k<quote>\)@im' => 'self::proxify_url_callback',                       // url(<url>)
+		'@\@import\s+(?<quote>\'|")(?<url>.*?)\k<quote>@im' => 'self::proxify_url_callback',                        // @import '<url>'
+		'@\b(?:srcset)\s*=\s*(?<quote>\'|")(?<value>.*?)\k<quote>@im' => 'self::proxify_srcset_attribute_callback', // srcset="<url> xxx, …"
+		'@<\s*form[^>]*action=(?<quote>\'|")(?<url>.*?)\k<quote>[^>]*>@im' => 'self::proxify_form_callback',        // <form action="<url>" …>


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:48Z

src/Plugin/ProxifyPlugin.php

+		'@\@import\s+(?<quote>\'|")(?<url>.*?)\k<quote>@im' => 'self::proxify_url_callback',                        // @import '<url>'
+		'@\b(?:srcset)\s*=\s*(?<quote>\'|")(?<value>.*?)\k<quote>@im' => 'self::proxify_srcset_attribute_callback', // srcset="<url> xxx, …"
+		'@<\s*form[^>]*action=(?<quote>\'|")(?<url>.*?)\k<quote>[^>]*>@im' => 'self::proxify_form_callback',        // <form action="<url>" …>
+	];


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:49Z

src/Plugin/ProxifyPlugin.php

-
-		return str_replace($url, proxify_url($url, $this->base_url), $matches[0]);
-	}
+	private $base_url = '';


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:49Z

src/Plugin/ProxifyPlugin.php

-		return str_replace($url, proxify_url($url, $this->base_url), $matches[0]);
-	}
+	private $base_url = '';
+


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:50Z

src/Plugin/ProxifyPlugin.php

-	}
+	private $base_url = '';
+
+	public function onCompleted(ProxyEvent $event) {


Spaces must be used to indent lines; tabs are not allowed

Opening brace should be on a new line

Nitpick-CI · 2018-01-29T11:14:50Z

src/Plugin/ProxifyPlugin.php

+	private $base_url = '';
+
+	public function onCompleted(ProxyEvent $event) {
+		$response = $event['response'];


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:50Z

src/Plugin/ProxifyPlugin.php

+
+	public function onCompleted(ProxyEvent $event) {
+		$response = $event['response'];
+		$content_type = $response->headers->get('content-type');


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:51Z

src/Plugin/ProxifyPlugin.php

+	public function onCompleted(ProxyEvent $event) {
+		$response = $event['response'];
+		$content_type = $response->headers->get('content-type');
+		if(starts_with($content_type, self::CONTENT_TYPE_BLACKLIST)) return;


Spaces must be used to indent lines; tabs are not allowed

Expected 1 space after IF keyword; 0 found

Inline control structures are not allowed

Nitpick-CI · 2018-01-29T11:14:51Z

src/Plugin/ProxifyPlugin.php

-		}
-
-		return $result;
+		// to be used when proxifying all the relative links


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:51Z

src/Plugin/ProxifyPlugin.php

-
-		return $result;
+		// to be used when proxifying all the relative links
+		$this->base_url = $event['request']->getUri();


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:52Z

src/Plugin/ProxifyPlugin.php

-		return $result;
+		// to be used when proxifying all the relative links
+		$this->base_url = $event['request']->getUri();
+		$proxified_content = preg_replace_callback_array(self::CONTENT_PARSERS, $response->getContent());


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:52Z

src/Plugin/ProxifyPlugin.php

+		// to be used when proxifying all the relative links
+		$this->base_url = $event['request']->getUri();
+		$proxified_content = preg_replace_callback_array(self::CONTENT_PARSERS, $response->getContent());
+		$response->setContent($proxified_content);


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-29T11:14:53Z

src/Plugin/ProxifyPlugin.php

 	}

-	public function onBeforeRequest(ProxyEvent $event){
-
+	public function onBeforeRequest(ProxyEvent $event) {


Spaces must be used to indent lines; tabs are not allowed

Opening brace should be on a new line

Nitpick-CI · 2018-01-29T11:14:53Z

src/Plugin/ProxifyPlugin.php

-
-			$request->prepare();
-		}
+		$this->convertPostToGet($request);


Spaces must be used to indent lines; tabs are not allowed

Script removal is not related to proxification. Class Html removed, no longer referenced.

Title replacement is not related to proxification.

Use regex named capture groups to allow reuse of callbacks. Added 'image' & 'font' to content-type blacklist.

Nitpick-CI · 2018-01-30T10:03:17Z

src/Plugin/AbstractPlugin.php

+abstract class AbstractPlugin
+{
+
+	public function onBeforeRequest(ProxyEvent $event)


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:17Z

src/Plugin/AbstractPlugin.php

+{
+
+	public function onBeforeRequest(ProxyEvent $event)
+	{


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:18Z

src/Plugin/AbstractPlugin.php

-	public function onHeadersReceived(ProxyEvent $event){
+
+	public function onHeadersReceived(ProxyEvent $event)
+	{


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:18Z

src/Plugin/AbstractPlugin.php

-
-	public function onCurlWrite(ProxyEvent $event){
+
+	public function onCurlWrite(ProxyEvent $event)


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:19Z

src/Plugin/AbstractPlugin.php

-	public function onCurlWrite(ProxyEvent $event){
+
+	public function onCurlWrite(ProxyEvent $event)
+	{


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:19Z

src/Plugin/AbstractPlugin.php

-
-	public function onCompleted(ProxyEvent $event){
+
+	public function onCompleted(ProxyEvent $event)


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:20Z

src/Plugin/AbstractPlugin.php

-	public function onCompleted(ProxyEvent $event){
+
+	public function onCompleted(ProxyEvent $event)
+	{


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:20Z

src/Plugin/AbstractPlugin.php

-				$this->onCompleted($event);
-			break;
+
+	final public function subscribe($dispatcher)


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:20Z

src/Plugin/AbstractPlugin.php

-			break;
+
+	final public function subscribe($dispatcher)
+	{


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:21Z

src/Plugin/AbstractPlugin.php

+
+	final public function subscribe($dispatcher)
+	{
+		$event_listeners = [


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:21Z

src/Plugin/AbstractPlugin.php

+	final public function subscribe($dispatcher)
+	{
+		$event_listeners = [
+			'request.before_send' => 'onBeforeRequest',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:22Z

src/Plugin/AbstractPlugin.php

+	{
+		$event_listeners = [
+			'request.before_send' => 'onBeforeRequest',
+			'request.sent' => 'onHeadersReceived',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:22Z

src/Plugin/AbstractPlugin.php

+		$event_listeners = [
+			'request.before_send' => 'onBeforeRequest',
+			'request.sent' => 'onHeadersReceived',
+			'curl.callback.write' => 'onCurlWrite',


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:23Z

src/Plugin/AbstractPlugin.php

+			'request.complete' => 'onCompleted',
+		];
+
+		foreach ($event_listeners as $event => $listener) {


Spaces must be used to indent lines; tabs are not allowed

Nitpick-CI · 2018-01-30T10:03:24Z

src/Plugin/AbstractPlugin.php

+		];
+
+		foreach ($event_listeners as $event => $listener) {
+			$dispatcher->addListener($event, [$this, $listener]);


Spaces must be used to indent lines; tabs are not allowed

Reduced function call overhead.

reef-actor · 2018-01-31T09:59:50Z

I removed the Html class as it is no longer a dependency following the extraction of js_remove and inlining the RegEx. Since some of the plugins in php-proxy-plugin-bundle are dependent on it, it might make sense to merge it with the utils there before removing it here.
Also, is Redis.php still needed in this project? Seems to be unused and superseded by php-proxy-plugin-cache?

reef-actor · 2018-02-01T10:12:25Z

It looks like there is also a dependency on the url_pattern functionality from php-proxy-plugin-bundle so that goes back in.

webaddicto · 2018-02-04T19:14:15Z

Interesting changes @reef-actor

@Athlon1600 what do you think about these changes?

I like the refactor of ProxifyPlugin and that remove_js and replace_title have becomed a plugin.

reef-actor · 2018-02-04T22:26:06Z

It has occurred to me that const visibility modifiers were only introduced in php 7.1 so the use of private const should probably be changed to just const to avoid unnecessarily restricting compatibility.

Athlon1600 · 2018-02-11T23:52:06Z

Some of those could probably go through. I will have more time to review this next week though... I'm looking right now to see what happened to Html class.

Nitpick-CI reviewed Jan 29, 2018

View reviewed changes

Move remove_js into dedicated plugin

46e9ea8

Script removal is not related to proxification. Class Html removed, no longer referenced.

reef-actor force-pushed the master branch from ef9ebc7 to 477a939 Compare January 29, 2018 13:08

reef-actor added 2 commits January 30, 2018 10:01

Move replace_title into dedicated plugin

03eee03

Title replacement is not related to proxification.

Refactor ProxifyPlugin

0affa91

Use regex named capture groups to allow reuse of callbacks. Added 'image' & 'font' to content-type blacklist.

reef-actor force-pushed the master branch from 477a939 to 93c6480 Compare January 30, 2018 10:03

Nitpick-CI reviewed Jan 30, 2018

View reviewed changes

reef-actor force-pushed the master branch from 93c6480 to 5682ba5 Compare January 30, 2018 10:16

Simplify AbstractPlugin

fc6d872

Reduced function call overhead.

reef-actor force-pushed the master branch from 5682ba5 to fc6d872 Compare January 30, 2018 10:26

Return the url_pattern filter to AbstractPlugin

36aae6b

Remove const visibility modifiers froim AbstractPlugin & ProxifyPlugin

4ce5868


		public function onCurlWrite(ProxyEvent $event){

		public function onCurlWrite(ProxyEvent $event)


		public function onCompleted(ProxyEvent $event){

		public function onCompleted(ProxyEvent $event)

Small cleanup #110

Are you sure you want to change the base?

Small cleanup #110

Conversation

reef-actor commented Jan 29, 2018

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

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

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

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

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

reef-actor commented Jan 31, 2018

reef-actor commented Feb 1, 2018

webaddicto commented Feb 4, 2018

reef-actor commented Feb 4, 2018

Athlon1600 commented Feb 11, 2018