Skip to content

Small cleanup#110

Open
reef-actor wants to merge 6 commits intoAthlon1600:masterfrom
reef-actor:master
Open

Small cleanup#110
reef-actor wants to merge 6 commits intoAthlon1600:masterfrom
reef-actor:master

Conversation

@reef-actor
Copy link
Copy Markdown

'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.

Comment thread src/Plugin/AbstractPlugin.php Outdated
}

final public function subscribe($dispatcher){
$event_listeners = [
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
final public function subscribe($dispatcher){
$event_listeners = [
'request.before_send' => 'onBeforeRequest',
'request.sent' => 'onHeadersReceived',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
$event_listeners = [
'request.before_send' => 'onBeforeRequest',
'request.sent' => 'onHeadersReceived',
'curl.callback.write' => 'onCurlWrite',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
'request.before_send' => 'onBeforeRequest',
'request.sent' => 'onHeadersReceived',
'curl.callback.write' => 'onCurlWrite',
'request.complete' => 'onCompleted',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
'request.sent' => 'onHeadersReceived',
'curl.callback.write' => 'onCurlWrite',
'request.complete' => 'onCompleted',
];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
case 'request.complete':
$this->onCompleted($event);
break;
foreach($event_listeners as $event => $listener) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Spaces must be used to indent lines; tabs are not allowed
  • Expected 1 space after FOREACH keyword; 0 found

Comment thread src/Plugin/AbstractPlugin.php Outdated
$this->onCompleted($event);
break;
foreach($event_listeners as $event => $listener) {
$dispatcher->addListener($event, [$this, $listener]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated

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'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
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:'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
}
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 = [
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
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>"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
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>)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
'@\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>'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
'@\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, …"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
'@\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>" …>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
'@\@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>" …>
];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
return str_replace($url, proxify_url($url, $this->base_url), $matches[0]);
}
private $base_url = '';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
}
private $base_url = '';

public function onCompleted(ProxyEvent $event) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Spaces must be used to indent lines; tabs are not allowed
  • Opening brace should be on a new line

Comment thread src/Plugin/ProxifyPlugin.php Outdated
private $base_url = '';

public function onCompleted(ProxyEvent $event) {
$response = $event['response'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
}

return $result;
// to be used when proxifying all the relative links
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
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());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated
// 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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/ProxifyPlugin.php Outdated

public function onBeforeRequest(ProxyEvent $event){

public function onBeforeRequest(ProxyEvent $event) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Spaces must be used to indent lines; tabs are not allowed
  • Opening brace should be on a new line

Comment thread src/Plugin/ProxifyPlugin.php Outdated

$request->prepare();
}
$this->convertPostToGet($request);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Script removal is not related to proxification.
Class Html removed, no longer referenced.
reef-actor added 2 commits January 30, 2018 10:01
Title replacement is not related to proxification.
Use regex named capture groups to allow reuse of callbacks.
Added 'image' & 'font' to content-type blacklist.
Comment thread src/Plugin/AbstractPlugin.php Outdated
abstract class AbstractPlugin
{

public function onBeforeRequest(ProxyEvent $event)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
{

public function onBeforeRequest(ProxyEvent $event)
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
public function onHeadersReceived(ProxyEvent $event){

public function onHeadersReceived(ProxyEvent $event)
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated

public function onCurlWrite(ProxyEvent $event){

public function onCurlWrite(ProxyEvent $event)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
public function onCurlWrite(ProxyEvent $event){

public function onCurlWrite(ProxyEvent $event)
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated

public function onCompleted(ProxyEvent $event){

public function onCompleted(ProxyEvent $event)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
public function onCompleted(ProxyEvent $event){

public function onCompleted(ProxyEvent $event)
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
$this->onCompleted($event);
break;

final public function subscribe($dispatcher)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
break;

final public function subscribe($dispatcher)
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated

final public function subscribe($dispatcher)
{
$event_listeners = [
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
final public function subscribe($dispatcher)
{
$event_listeners = [
'request.before_send' => 'onBeforeRequest',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
{
$event_listeners = [
'request.before_send' => 'onBeforeRequest',
'request.sent' => 'onHeadersReceived',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
$event_listeners = [
'request.before_send' => 'onBeforeRequest',
'request.sent' => 'onHeadersReceived',
'curl.callback.write' => 'onCurlWrite',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
'request.complete' => 'onCompleted',
];

foreach ($event_listeners as $event => $listener) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Plugin/AbstractPlugin.php Outdated
];

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Reduced function call overhead.
@reef-actor
Copy link
Copy Markdown
Author

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
Copy link
Copy Markdown
Author

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

@webaddicto
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Author

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
Copy link
Copy Markdown
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants