Completed
Push — master ( fc1b09...6e5f86 )
by Jacob
06:46
created

CommentSubmitModule::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 7
Code Lines 5

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 7
rs 9.4285
c 0
b 0
f 0
cc 1
eloc 5
nc 1
nop 4
1
<?
0 ignored issues
show
Security Best Practice introduced by
It is not recommend to use PHP's short opening tag <?, better use <?php, or <?= in case of outputting.

Short opening tags are disabled in PHP’s default configuration. In such a case, all content of this file is output verbatim to the browser without being parsed, or executed.

As a precaution to avoid these problems better use the long opening tag <?php.

Loading history...
2
3
Loader::load('collector', 'comment/CommentCollector');
4
5
Loader::load('utility', array(
6
	'Content',
7
	'Cookie',
8
	'Request',
9
	'Validate'));
10
11
final class CommentSubmitModule
12
{
13
14
	private $site;
15
	private $path;
16
	private $full_path;
17
	private $page_title;
18
19
	public function __construct($site, $path, $full_path, $page_title)
20
	{
21
		$this->site = $site;
22
		$this->path = $path;
23
		$this->full_path = $full_path;
24
		$this->page_title = $page_title;
25
	}
26
27
	public function activate()
28
	{
29
		if(!Request::hasPost())
30
			return false;
31
		if(!Request::getPost('submit') == 'Submit Comment')
32
			return false;
33
		
34
		$errors = $this->fetch_errors();
35
		if(count($errors) > 0)
36
			return $errors;
37
		if(Request::getPost('catch') !== '')
38
			return false;
39
		
40
		$page_id = $this->save_comment_page();
41
		$commenter_id = $this->save_commenter();
42
		$comment_id = $this->save_comment();
43
		
44
		$comment_meta_id = $this->save_comment_meta($commenter_id, $comment_id, $page_id);
45
    $comment_service_id = $this->save_to_comment_service(Request::getPost());
0 ignored issues
show
Bug introduced by
Are you sure the assignment to $comment_service_id is correct as $this->save_to_comment_s...ce(\Request::getPost()) (which targets CommentSubmitModule::save_to_comment_service()) seems to always return null.

This check looks for function or method calls that always return null and whose return value is assigned to a variable.

class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
$object = $a->getObject();

The method getObject() can return nothing but null, so it makes no sense to assign that value to a variable.

The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.

Loading history...
Unused Code introduced by
$comment_service_id is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
46
		
47
		$this->send_notifications($page_id);
48
		$this->redirect_to_comment($comment_meta_id);
49
	}
50
51
	private function fetch_errors()
52
	{
53
		$errors = array();
54
		if(!Validate::checkRequest('post', 'name', 'name'))
55
			$errors['name'] = 'You must include a valid name';
56
		if(!Validate::checkRequest('post', 'email', 'email'))
57
			$errors['email'] = 'You must include a valid email';
58
		if(Request::getPost('website') && !Validate::checkRequest('post', 'website', 'url'))
59
			$errors['website'] = 'Please enter a valid website';
60
		if(!Validate::checkRequest('post', 'comment', 'string'))
61
			$errors['comment'] = 'You must enter a comment';
62
		if(Request::getPost('notify') && Request::getPost('notify') != 'check')
63
			$errors['notify'] = 'You entered an invalid notify request';
64
		if(Request::getPost('reply') && !Validate::checkRequest('post', 'reply', 'integer'))
65
			$errors['reply'] = 'You entered an invalid reply request';
66
		
67
		return $errors;
68
	}
69
70
	private function save_comment_page()
71
	{
72
		$page_result = CommentCollector::getCommentPageByURL($this->path, $this->site);
73
		if($page_result !== null)
74
			return $page_result->id;
75
		
76
		$query = "INSERT INTO `jpemeric_comment`.`comment_page` (`site`, `path`) VALUES ('%d', '%s')";
77
		$query = sprintf($query, $this->site, $this->path);
78
		Database::execute($query);
79
		
80
		return Database::lastInsertID();
81
	}
82
83
	private function save_commenter()
84
	{
85
		$cookie_value = array(
86
			'name' => Request::getPost('name'),
87
			'email' => Request::getPost('email'));
88
		
89
		if(Request::getPost('website') != '')
90
			$cookie_value['website'] = Request::getPost('website');
91
		
92
		$cookie_value = json_encode($cookie_value);
93
		
94
		Cookie::instance('Commenter')
95
			->setValue($cookie_value)
96
			->save();
97
		
98
		$commenter_result = CommentCollector::getCommenterByFields(Request::getPost('name'), Request::getPost('email'), Request::getPost('website'));
99
		if($commenter_result !== null)
100
			return $commenter_result->id;
101
		
102
		$query = "INSERT INTO `jpemeric_comment`.`commenter` (`name`,`email`,`url`) VALUES ('%s','%s','%s')";
103
		
104
		$name = Database::escape(Request::getPost('name'));
105
		$email = Database::escape(Request::getPost('email'));
106
		$website = Database::escape(Request::getPost('website'));
107
		
108
		$query = sprintf($query, $name, $email, $website);
109
		Database::execute($query);
110
		return Database::lastInsertID();
111
	}
112
113
	private function save_comment()
114
	{
115
		$comment_result = CommentCollector::getCommentByBody(Request::getPost('comment'));
116
		if($comment_result !== null)
117
			return $comment_result->id;
118
		
119
		$query = "INSERT INTO `jpemeric_comment`.`comment` (`body`, `body_format`) VALUES ('%s', '%s')";
120
		
121
		$body = Database::escape(Request::getPost('comment'));
122
		
123
		$body_format = Request::getPost('comment');
124
		$body_format = Content::instance('CleanComment', $body_format)->activate();
125
		$body_format = Database::escape($body_format);
126
		
127
		$query = sprintf($query, $body, $body_format);
128
		Database::execute($query);
129
		return Database::lastInsertID();
130
	}
131
132
	private function save_comment_meta($commenter, $comment, $page)
133
	{
134
		$query = "INSERT INTO `jpemeric_comment`.`comment_meta` (`commenter`,`comment`,`reply`,`notify`,`comment_page`,`date`,`display`) VALUES ('%d','%d','%d','%d','%d','%s','%d')";
135
		
136
		$reply = Database::escape(Request::getPost('type'));
137
		if($reply == 'new')
138
			$reply = 0;
139
		// else check to make sure value is legit
140
		
141
		$notify = (Request::getPost('notify') == 'check') ? 1 : 0;
142
		$date = date('Y-m-d H:i:s');
143
		$display = 1;
144
		
145
		$query = sprintf($query, $commenter, $comment, $reply, $notify, $page, $date, $display);
146
		Database::execute($query);
147
		return Database::lastInsertID();
148
	}
149
150
	private function send_notifications($page)
151
	{
152
		$email_recipient_array = array();
153
		
154
		$commenter_result = CommentCollector::getCommenterByFields(Request::getPost('name'), Request::getPost('email'), Request::getPost('website'));
155
		
156
		if($commenter_result->trusted == 1)
157
		{
158
			$notification_result = CommentCollector::getNotificationForPage($page);
159
			
160
			foreach($notification_result as $notification_row)
0 ignored issues
show
Bug introduced by
The expression $notification_result of type array|false is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
161
			{
162
				if($notification_row->email == Request::getPost('email'))
163
					continue;
164
				
165
				$email_recipient_array[$notification_row->email] = array(
166
					'email' => $notification_row->email,
167
					'name' => $notification_row->name);
168
			}
169
		}
170
        
171
        $site = URLDecode::getSite();
172
        
173
        if ($site == 'blog') {
174
            $subject = "New Comment on Jacob Emerick's Blog";
175
            $message = "Hello!\nThere has been a new comment on the post '{$this->page_title}' at Jacob Emerick's Blog. You have chosen to be notified of it - please reply to [email protected] if you would like to be removed from these notifications.\n\nOn " . date('F j, Y g:i a') . ", " . Request::getPost('name') . " commented...\n" . Request::getPost('comment') . "\n\nVisit {$this->full_path}#comments to see and reply to all the comments on this post.\nThank you!";
176
        } else if ($site == 'waterfalls') {
177
            $subject = "New Comment on Waterfalls of the Keweenaw";
178
            $message = "Hello!\nThere has been a new comment on the page '{$this->page_title}' at Waterfalls of the Keweenaw. You have chosen to be notified of it - please reply to [email protected] if you would like to be removed from these notifications.\n\nOn " . date('F j, Y g:i a') . ", " . Request::getPost('name') . " commented...\n" . Request::getPost('comment') . "\n\nVisit {$this->full_path}#comments to see and reply to all the comments on this post.\nThank you!";
179
        }
180
181
    global $container;
182
183
		foreach($email_recipient_array as $email_recipient)
184
		{
185
      $sent = $container['mail']
0 ignored issues
show
Unused Code introduced by
$sent is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
186
        ->addTo($email_recipient['email'], $email_recipient['name'])
187
        ->addBCC($container['config']->admin_email)
188
        ->setSubject($subject)
0 ignored issues
show
Bug introduced by
The variable $subject does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
189
        ->setPlainMessage($message)
0 ignored issues
show
Bug introduced by
The variable $message does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
190
        ->send();		
191
		}
192
	}
193
194
	private function redirect_to_comment($comment_id)
195
	{
196
		$url = '';
197
		$url .= $this->full_path;
198
		$url .= "#comment-{$comment_id}";
199
		
200
		Loader::loadNew('controller', 'Error303Controller', array($url))->activate();
201
		exit;
0 ignored issues
show
Coding Style Compatibility introduced by
The method redirect_to_comment() contains an exit expression.

An exit expression should only be used in rare cases. For example, if you write a short command line script.

In most cases however, using an exit expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage.

Loading history...
202
	}
203
204
    private function save_to_comment_service(array $data)
0 ignored issues
show
Coding Style introduced by
save_to_comment_service uses the super-global variable $_SERVER which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
205
    {
206
        $path = $_SERVER['REQUEST_URI'];
207
        $path = explode('/', $path);
208
        $path = array_filter($path);
209
        $path = array_slice($path, 0, 2);
210
        $path = implode('/', $path);
211
212
        $body = [
213
            'commenter' => [
214
                'name' => $data['name'],
215
                'email' => $data['email'],
216
                'website' => $data['website'],
217
            ],
218
            'body' => $data['comment'],
219
            'should_notify' => (isset($data['notify']) && $data['notify'] == 'check'),
220
            'should_display' => true,
221
            'domain' => (URLDecode::getSite() == 'blog' ? 'blog.jacobemerick.com' : 'waterfallsofthekeweenaw.com'),
222
            'path' => $path,
223
            'url' => $this->full_path,
224
            'thread' => 'comments',
225
            'ip_address' => $_SERVER['REMOTE_ADDR'],
226
            'user_agent' => $_SERVER['HTTP_USER_AGENT'],
227
            'referrer' => $_SERVER['HTTP_REFERER'],
228
        ];
229
230
        global $config;
231
        $configuration = new Swagger\Client\Configuration();
232
        $configuration->setUsername($config->comments->user);
233
        $configuration->setPassword($config->comments->password);
234
        $configuration->addDefaultHeader('Content-Type', 'application/json');
235
        $configuration->setHost($config->comments->host);
236
        $configuration->setCurlTimeout($config->comments->timeout);
237
238
        $client = new Swagger\Client\ApiClient($configuration);
239
        $response = $client->callApi('/comments', 'POST', '', $body, '');
0 ignored issues
show
Documentation introduced by
'' is of type string, but the function expects a array.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
Unused Code introduced by
$response is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
240
    }
241
}
242