Is there a way to return the line it is on like when you get an unhandled error?
You can get information about callers, including line numbers, using (e.g.)
debug_backtrace or
apd_callstack (the latter is part of the
APD extension and may not be installed on the X10 servers).
If you use
PDO rather than the old mysql driver, you can use
exceptions, which also record line number and can be used to get a stack trace. Exceptions also let you call your error handler once, rather than at each point an error might happen:
PHP:
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
try {
$query = $db->prepare(...);
$query->execute(...);
// no need to handle errors here,
foreach ($query as $row) {
...
$query = $db->prepare(...);
$query->execute(...);
// or here,
foreach ($query as $row) {
...
} catch (PDOException $exc) {
// because errors are handled here.
writeError($exc);
}
For a tutorial on PDO, read: "
Writing MySQL Scripts with PHP and PDO".
PHP:
<?php
$toUsers = explode(',',$_REQUEST['toUser']);
...
if(trim($toUsers) == '') {
trim works on strings, not arrays. You could try:
PHP:
$toUsers = array_filter(array_map('trim', explode(',',$_REQUEST['toUser'])));
...
if (! $toUsers) {
PHP:
$query = "SELECT id FROM users WHERE username = '$curUser'";
$result = mysql_query($result);
if(!$result)
writeError(mysql_error());
else {
while($row=mysql_fetch_array($result, MYSQL_ASSOC)) {
$senderID = $row['id'];
}
}
There's no need for a loop here as there should be only one row in the result, unless
$curUser might not be a valid username (the result might have 0 or 1 rows) and you're using the loop to conditionally assign
$senderID. However, later code doesn't follow up on the latter, so I'll assume this isn't the purpose of the
while loop. Fetch the row and get the ID without the
while.
PHP:
if(sizeof($toUsers) != 1) {
$query = "SELECT id FROM users WHERE username = '$toUsers'";
Two issues here: first, this branch is entered if
$toUsers has zero items (which shouldn't happen in practice, due to earlier tests) or more than one item, you're running a single query. Change the
!= to a
== to correct this.
Second, these queries are open to
SQL injection. If you switch to PDO and use
prepared statements, you won't have to worry about injection via parameters.
PHP:
$lookupUID = $db->prepare("SELECT id FROM users WHERE username = :to";
foreach ($toUsers as $to) {
$lookupUID->bindValue(':to', $to);
$lookupUID->execute();
..
}
This will also be more performant, as the statement won't need to be parsed each time you execute it.
Alternatively, you can send a message to valid users all at once using the query:
Code:
INSERT INTO mail (`to`, `from`, `subject`, `message`)
SELECT users.id AS `to`, msg.*
FROM users
JOIN (
SELECT :sender AS from, :subject AS subject, :message AS message
) AS msg
WHERE users.username IN ($to);
Note I've explicitly specified which columns to set and dropped the `date` field, as MySQL can automatically set that to the current time if column `date` is defined in a certain way:
Code:
CREATE TABLE `mail` (
`to` int(11) NOT NULL,
`from` int(11) NOT NULL,
...
`date` timestamp DEFAULT CURRENT_TIMESTAMP,
FOREIGN KEY (`to`) REFERENCES `users` (`id`),
FOREIGN KEY (`sender`) REFERENCES `users` (`id`)
) ENGINE=InnoDB;
This shortens the PHP code to send a message:
PHP:
<?php
$msg = array();
$errors = array();
function safifiy_username($name) {
return preg_replace('/^\s+|(\W.*)?\s*)$/g', '', $name);
}
$toUsers = array_filter(array_map('safify_username', explode(',', $_REQUEST['to']));
/* alternate for PHP >= 5.3.
$toUsers = array_filter(array_map(function ($name) {
return preg_replace('/^\s+|(\W.*)\s*$/', '', $name);
}, explode(',', $_REQUEST['to'])));
*/
$msg[':subject'] = isset($_REQUEST['subject']) ? trim($_REQUEST['subject']) : '';
$msg[':message'] = isset($_REQUEST['message']) ? trim($_REQUEST['message']) : '';
// tests should be refactored as a validation function.
if (!$toUsers) {
$errors[] = 'No recipients.';
}
if (empty($msg[':subject'])) {
$errors[] = "Empty subject.";
}
if (empty($msg[':message'])) {
$errors[] = "Empty message.";
}
// don't print $errors here, as we might wish to record errors when adding the message to the mail table.
if (!$errors) {
// $to is a vector for SQL injection, which is why $toUsers is filtered earlier.
$to = "'" . implode("', '", $toUsers) . "'";
$sendMsgQuery = $db->prepare("INSERT INTO mail (`to`, `from`, `subject`, `message`)
SELECT users.id AS `to`, msg.*
FROM users
JOIN (SELECT :sender AS from, :subject AS subject, :message AS message) AS msg
WHERE users.username IN ($to)");
$sendMsgQuery->execute($msg);
}
if ($errors) {
?>Errors: <ul><li><?php
echo implode('</li><li>', $errors);
?></li></ul><?php
}
The one issue with this is it doesn't expose unknown usernames to the script, which could then inform the sender. You can do this by issuing another query in the block that inserts the message:
PHP:
$usersQuery = $db->query("SELECT username FROM users WHERE username IN ($to)");
$unknownUsers = array_diff($toUsers, $usersQuery->fetchAll(PDO::FETCH_COLUMN));
if ($unknownUsers) {
$errors[] = 'Unknown recipients: ' . implode(', ', $unknownUsers);
}
