914 lines
30 KiB
Plaintext
914 lines
30 KiB
Plaintext
========================
|
|
Horde Coding Standards
|
|
========================
|
|
|
|
:Contact: dev@lists.horde.org
|
|
|
|
.. contents:: Contents
|
|
.. section-numbering::
|
|
|
|
|
|
Basics
|
|
======
|
|
|
|
Horde Coding Standards are following PSR-0, PSR-1 and PSR-2 of the `PHP
|
|
Framework Interop Group`_
|
|
|
|
.. _`PHP Framework Interop Group`: http://www.php-fig.org/
|
|
|
|
|
|
Indenting/Whitespace
|
|
====================
|
|
|
|
Use an indent of 4 spaces, with no tabs. Remove all trailing whitespace. If
|
|
using vim, the following .vimrc code will highlight trailing whitespace in
|
|
red::
|
|
|
|
highlight WhitespaceEOL ctermbg=red guibg=red
|
|
match WhitespaceEOL /\s\+$/
|
|
|
|
|
|
Language Case
|
|
=============
|
|
|
|
When working with PHP statements, constructs or keywords, lowercase text is
|
|
required. This does not apply to classes provided by PHP, which should be
|
|
written as they are in the PHP manual (e.g. `DOMDocument::createCDATASection`_)
|
|
|
|
.. _DOMDocument::createCDATASection: http://us.php.net/manual/en/domdocument.createcdatasection.php
|
|
|
|
|
|
Control Structures
|
|
==================
|
|
|
|
These include ``if``, ``for``, ``while``, ``switch``, etc. Here is an example
|
|
``if`` statement, since it is the most complicated of them::
|
|
|
|
if ((condition1) || (condition2)) {
|
|
action1;
|
|
} elseif ((condition3) && (condition4)) {
|
|
action2;
|
|
} else {
|
|
defaultaction;
|
|
}
|
|
|
|
Multi-line if conditions are braced this way::
|
|
|
|
if ((condition1) || (condition2) || (condition3) ||
|
|
(condition4)) {
|
|
action1;
|
|
}
|
|
|
|
Control statements should have one space between the control keyword and
|
|
opening parenthesis, to distinguish them from function calls.
|
|
|
|
Do not omit the curly braces under any circumstance. In the case of a large
|
|
number of short tests and actions, the following is acceptable::
|
|
|
|
if (condition) { action; }
|
|
if (condition 2) { action 2; }
|
|
...
|
|
|
|
For switch statements::
|
|
|
|
switch (condition) {
|
|
case 1:
|
|
action1;
|
|
break;
|
|
|
|
case 2:
|
|
action2;
|
|
break;
|
|
|
|
default:
|
|
defaultaction;
|
|
break;
|
|
}
|
|
|
|
|
|
Function Calls
|
|
==============
|
|
|
|
Functions should be called with no spaces between the function name, the
|
|
opening parenthesis, and the first parameter; spaces between commas and each
|
|
parameter, and no space between the last parameter, the closing parenthesis,
|
|
and the semicolon. Here's an example::
|
|
|
|
$var = foo($bar, $baz, $quux);
|
|
|
|
As displayed above, there should be one space on either side of an equals sign
|
|
used to assign the return value of a function to a variable. In the case of a
|
|
block of related assignments, more space may be inserted to promote
|
|
readability::
|
|
|
|
$short = foo($bar);
|
|
$long_variable = foo($baz);
|
|
|
|
The "@" operator can be used to silence any errors that a function call may
|
|
generate. This should be used with caution, as any fatal errors will be
|
|
completely transparent to the user. Only use "@" to silence functions that,
|
|
e.g., emit warnings when the data input is incorrect (such as unserialize()).
|
|
|
|
|
|
Function Definitions
|
|
====================
|
|
|
|
Function declarations follow the "BSD/Allman" convention::
|
|
|
|
function fooFunction($arg1, $arg2 = '')
|
|
{
|
|
if (condition) {
|
|
statement;
|
|
}
|
|
return $val;
|
|
}
|
|
|
|
Arguments with default values go at the end of the argument list. Always
|
|
attempt to return a meaningful value from a function if one is appropriate.
|
|
|
|
Functions used only in the current script/class (e.g. private or protected)
|
|
should begin with a ``_`` character (e.g. ``_exampleLibrary``). This helps
|
|
distinguish these private functions from other, publicly available functions.
|
|
|
|
|
|
Class Definitions
|
|
=================
|
|
|
|
Class definitions also follow the "BSD/Allman" convention::
|
|
|
|
class Some_Class
|
|
{
|
|
/**
|
|
*
|
|
*/
|
|
var $_variable;
|
|
|
|
public function fooFunction()
|
|
{
|
|
statement;
|
|
}
|
|
|
|
}
|
|
|
|
Note the blank line at the end of the class definition.
|
|
|
|
|
|
Naming Libraries
|
|
================
|
|
|
|
Libraries (any file located in the ``lib/`` directory of the application)
|
|
should be named with capital letters at the beginning of each word. Use
|
|
studlycaps for naming; a session cache class would be stored in
|
|
``lib/SessionCache.php``.
|
|
|
|
If the library/class is extended, the extending files should be stored in a
|
|
directory under ``lib/`` with the same name as the original library.
|
|
Subclasses follow the exact same naming requirements.
|
|
|
|
Example
|
|
-------
|
|
|
|
The "Example Library" library should be saved as ``lib/ExampleLibrary.php``.
|
|
Any file extending the library/class should be stored in the directory
|
|
``lib/ExampleLibrary/``.
|
|
|
|
|
|
Comments
|
|
========
|
|
|
|
Inline documentation for classes should follow the `phpDocumentor conventions`_.
|
|
|
|
.. _phpDocumentor conventions: http://phpdoc.org/
|
|
|
|
Quick example for private variable definition for Horde::
|
|
|
|
/**
|
|
* Variable description.
|
|
*
|
|
* @var datatype
|
|
*/
|
|
|
|
Quick example function definition for Horde::
|
|
|
|
/**
|
|
* The description of the function goes here.
|
|
*
|
|
* @param datatype $variablename Description of variable.
|
|
* @param datatype $variable2name Description of variable2.
|
|
* ...
|
|
* [Insert 2 spaces after the longest $variable definition, and then line
|
|
* up all descriptions with this description]
|
|
*
|
|
* @return datatype Description of return value.
|
|
* [Once again, insert 2 spaces after the datatype, and line up all
|
|
* subsequent lines, if any, with this character.]
|
|
*
|
|
* @since Horde x.x.x [Only if necessary - use if function is added to the
|
|
* current release versions to indicate that the function has not been
|
|
* available in previous versions. @since is only needed if the function
|
|
* has been added after the last major point release - e.g. x.0.]
|
|
*/
|
|
|
|
|
|
Including Code
|
|
==============
|
|
|
|
You should not explicitly include or require classes. Horde provides an
|
|
autoloader library, so the class should be autoloaded. This will allow
|
|
maximum flexibility in managing class libraries.
|
|
|
|
For all other includes (configuration files, data files, etc.) use `include`_.
|
|
|
|
.. _autoload: http://php.net/autoload
|
|
.. _include: http://php.net/include
|
|
|
|
|
|
PHP Code Tags
|
|
=============
|
|
|
|
Always use ``<?php`` to delimit PHP code, not the ``<?`` shorthand.
|
|
This is required for PEAR compliance and is also the most portable way to
|
|
include PHP code on differing operating systems and setups.
|
|
|
|
In templates, make sure to use this as well (``<?php echo $varname ?>``), as
|
|
the shortcut version (``<?= $var ?>``) does not work with `short_open_tag`_
|
|
turned off.
|
|
|
|
Files should never end with a closing '?>'. This is redundant and can allow
|
|
whitespace added to the end of a file to cause PHP to send headers early.
|
|
|
|
.. _short_open_tag: http://www.php.net/manual/en/configuration.directives.php#ini.short-open-tag
|
|
|
|
|
|
Header Comment Blocks
|
|
=====================
|
|
|
|
All source code files in the Horde distribution should contain the following
|
|
comment block as the header:
|
|
|
|
Example for `LGPL`_'ed Horde code::
|
|
|
|
/**
|
|
* The Horde_Foo class provides an API for various foo
|
|
* techniques that can be used by Horde applications.
|
|
*
|
|
* Copyright 2009-2017 Horde LLC (http://www.horde.org/)
|
|
*
|
|
* See the enclosed file COPYING for license information (LGPL-2). If you
|
|
* did not receive this file, see http://www.horde.org/licenses/lgpl.
|
|
*
|
|
* @author Original Author <author@example.com>
|
|
* @author Your Name <you@example.com>
|
|
* @category Horde
|
|
* @package Package
|
|
* @license http://www.horde.org/licenses/lgpl LGPL-2
|
|
* @since Horde 4.0.1 [only if needed]
|
|
*/
|
|
|
|
.. _LGPL: http://www.opensource.org/licenses/lgpl-license.php
|
|
|
|
Example for `GPL`_'ed application code::
|
|
|
|
/**
|
|
* The App_Bar class contains all functions related to handling
|
|
* bars in App.
|
|
*
|
|
* Copyright 2009-2017 Horde LLC (http://www.horde.org/)
|
|
*
|
|
* See the enclosed file COPYING for license information (GPL). If you
|
|
* did not receive this file, see http://www.horde.org/licenses/gpl.
|
|
*
|
|
* @author Original Author <author@example.com>
|
|
* @author Your Name <you@example.com>
|
|
* @category Horde
|
|
* @package App
|
|
* @license http://www.horde.org/licenses/gpl GPL
|
|
* @since App 1.0.1 [only if needed]
|
|
*/
|
|
|
|
|
|
.. _GPL: http://www.opensource.org/licenses/gpl-license.php
|
|
|
|
There's no hard rule to determine when a new code contributer should be added
|
|
to the list of authors for a given source file. In general, their changes
|
|
should fall into the "substantial" category (meaning somewhere around 10% to
|
|
20% of code changes). Exceptions could be made for rewriting functions or
|
|
contributing new logic.
|
|
|
|
Simple code reorganization or bug fixes would not justify the addition of a
|
|
new individual to the list of authors.
|
|
|
|
|
|
Example URLs
|
|
============
|
|
|
|
Use ``example.com`` for all example URLs, per `RFC 2606`_.
|
|
|
|
.. _RFC 2606: http://www.faqs.org/rfcs/rfc2606.html
|
|
|
|
|
|
php.ini settings
|
|
================
|
|
|
|
Horde code MUST NOT use global variables set by EGPCS (Environment, GET, POST,
|
|
Cookie, Server) data. Instead, the magic variables ``$_ENV``, ``$_GET``,
|
|
``$_POST``, $_COOKIE``, and ``$_SERVER`` must be used instead.
|
|
|
|
To retrieve posted data (in the global ``$_GET`` and ``$_POST`` variables),
|
|
you should normally use `Horde_Util::getFormData()`_ which will automatically
|
|
run `Horde_Util::dispelMagicQuotes()`_. This will ensure that all Horde code
|
|
will work regardless of the setting of `magic_quotes_gpc`_. The only time you
|
|
should not use `Horde_Util::getFormData()`_ is if you want to directly access
|
|
a GET or POST variable instead; in this case, you should use
|
|
`Horde_Util::getGet()`_ or `Horde_Util::getPost()`_ respectively.
|
|
|
|
All Horde code should work with `error_reporting`_ = E_ALL. Failure to do so
|
|
would result in ugly output, error logs getting filled with lots of warning
|
|
messages, or even downright broken scripts.
|
|
|
|
No Horde code should assume that '.' is in the include path. Always specify
|
|
'./' in front of a filename when you are including a file in the same
|
|
directory.
|
|
|
|
.. _magic_quotes_gpc: http://www.php.net/manual/en/ref.info.php#ini.magic-quotes-gpc
|
|
.. _error_reporting: http://www.php.net/manual/en/ref.errorfunc.php#ini.error-reporting
|
|
.. _Horde_Util::getFormData(): http://dev.horde.org/api/framework/Horde_Util/Horde_Util.html#methodgetFormData
|
|
.. _Horde_Util::dispelMagicQuotes(): http://dev.horde.org/api/framework/Horde_Util/Horde_Util.html#methoddispelMagicQuotes
|
|
.. _Horde_Util::getGet(): http://dev.horde.org/api/framework/Horde_Util/Horde_Util.html#methodgetGet
|
|
.. _Horde_Util::getPost(): http://dev.horde.org/api/framework/Horde_Util/Horde_Util.html#methodgetPost
|
|
|
|
|
|
HTML 4 Compliance
|
|
=================
|
|
|
|
All Horde libraries and applications that generate content to be sent to a web
|
|
browser are expected to generate HTML 4 compliant syntax. This is a change
|
|
from earlier mandates to use XHTML 1.0.
|
|
|
|
All tag names and parameters must be lower case including javascript event
|
|
handlers::
|
|
|
|
<font color="#ffffff">...</font>
|
|
<a href="http://example.com" onmouseover="status=''" onmouseout="status=''">...</a>
|
|
|
|
All tag parameters must be of a valid parameter="value" form (numeric values
|
|
must also be surrounded by quotes). For parameters that had no value in HTML,
|
|
the parameter name is the value. For example::
|
|
|
|
<input type="checkbox" checked="checked">
|
|
<select name="example">
|
|
<option selected="selected" value="1">Example</option>
|
|
</select>
|
|
<td nowrap="nowrap">Example</td>
|
|
|
|
All form definitions must be on their own line and either fully defined within
|
|
a ``<td></td>`` pair or be outside table tags. Forms must also always have an
|
|
action parameter::
|
|
|
|
<form method="post" action="http://example.com/example.cgi">
|
|
<table>
|
|
<tr><td>example</td></tr>
|
|
</table>
|
|
</form>
|
|
|
|
<table>
|
|
<tr><td>
|
|
<form action="javascript:void(0)" onsubmit="return false;">
|
|
</form>
|
|
</td></tr>
|
|
</table>
|
|
|
|
All JavaScript tags must have a valid type parameter::
|
|
|
|
<script type="text/javascript">
|
|
...
|
|
</script>
|
|
|
|
Nothing may appear after ``</html>``, therefore include any common footers
|
|
after all other output.
|
|
|
|
All images must have an ``alt`` attribute::
|
|
|
|
<img src="example.gif" alt="<?php echo _("Example") ?>" />
|
|
<?php
|
|
echo Horde_Themes_Image::tag('example.gif', array(
|
|
'alt' => _("Example")
|
|
));
|
|
?>
|
|
|
|
Input fields of type "image" do not allow the border attribute and may render
|
|
with a border on some browsers. Use the following instead::
|
|
|
|
<a href="" onclick="document.formname.submit(); return false;"><?php
|
|
echo Horde_Themes_Image::tag("example.gif", array(
|
|
'alt' => _("Example")
|
|
));
|
|
?></a>
|
|
|
|
|
|
Database Conventions
|
|
====================
|
|
|
|
All database tables used by Horde resources and Horde applications need to
|
|
make sure that their table and field names work in all databases. Many
|
|
databases reserve words like 'uid', 'user', etc. for internal use, and forbid
|
|
words that are SQL keywords (select, where, etc.). Also, all names should be
|
|
lowercase, with underscores ('_') to separate words, to avoid case sensitivity
|
|
issues.
|
|
|
|
A good way to do this for field names is to make the field name
|
|
tablename_fieldname.
|
|
|
|
Other general guidelines: Table names should be plural (users); field names
|
|
should be singular (user_name).
|
|
|
|
Use Horde_Db for any SQL schema management and for queries, especially for
|
|
result set pagination ('LIMIT' is not portable). Also, do not use the 'AS'
|
|
keyword for table aliases.
|
|
|
|
In SQL queries, keywords should be capitalized.
|
|
|
|
Identifiers should not be longer than 30 characters.
|
|
|
|
|
|
Regular Expression Use
|
|
======================
|
|
|
|
Always use the `preg_`_ functions instead of `ereg_`_ (and
|
|
`preg_split()`_ instead of `split()`_); they are included in PHP by default
|
|
and are faster than `ereg_`_; `ereg_`_ is also deprecated in PHP 5.3+.
|
|
|
|
**NEVER** use a regular expression to match or replace a static string.
|
|
`explode()`_ (in place of `preg_split()`_), `str_replace()`_, `strpos()`_, or
|
|
`strtr()`_ do the job much more efficiently.
|
|
|
|
In addition, when doing replacement or regex matching on large
|
|
strings, if you don't know if the target string contains the text to
|
|
be matched or replaced, it is often a performance win to use
|
|
`strpos()`_ to check first. Then, only if the text to be matched or
|
|
replaced is present, go ahead and do the more memory intensive string
|
|
manipulation.
|
|
|
|
.. _preg_: http://www.php.net/manual/en/ref.pcre.php
|
|
.. _ereg_: http://www.php.net/manual/en/ref.regex.php
|
|
.. _preg_split(): http://www.php.net/manual/en/function.preg-split.php
|
|
.. _split(): http://www.php.net/manual/en/function.split.php
|
|
.. _explode(): http://www.php.net/manual/en/function.explode.php
|
|
.. _str_replace(): http://www.php.net/manual/en/function.str-replace.php
|
|
.. _strpos(): http://www.php.net/manual/en/function.strpos.php
|
|
.. _strtr(): http://www.php.net/manual/en/function.strtr.php
|
|
|
|
|
|
Parameter Passing
|
|
=================
|
|
|
|
Parameters should be passed by value wherever semantically possible.
|
|
This practice takes full advantage of reference counting.
|
|
|
|
.. Note:: The `ternary operator`_ automatically returns a copy of its
|
|
operands, so don't use it with objects unless you are sure you want
|
|
to return an object copy.
|
|
|
|
If at all possible, the number of optional arguments to a function should be
|
|
limited. If optional arguments are needed, it is usually best to have the
|
|
last parameter be an array which contains the list of optional arguments.
|
|
This also eases future expansion since new options can be added without
|
|
changing the API.
|
|
|
|
.. _`ternary operator`: http://www.php.net/manual/en/language.operators.comparison.php#language.operators.comparison.ternary
|
|
|
|
|
|
Long Lines
|
|
==========
|
|
|
|
The optimal line length is 80 characters, including comments. The maximum line
|
|
length is 100 characters unless this severely impacts the clarity of the
|
|
code. When deciding where to wrap, aim for readability, understanding that
|
|
different developers will have different views on this. Always wrap comments
|
|
and leave at least 2 spaces at the right margin (in other words, wrap at 78
|
|
characters).
|
|
|
|
|
|
Line Breaks
|
|
===========
|
|
|
|
Only use UNIX style of linebreak (``\n``), not Windows/DOS/Mac style
|
|
(``\r\n``).
|
|
|
|
Using vim, to convert from DOS style type::
|
|
|
|
:set ff=unix
|
|
|
|
Using vi, to convert from DOS style type::
|
|
|
|
:g/^M/s///g
|
|
|
|
(Note that the ``^M`` is a control character, and to reproduce it when you
|
|
type in the vi command you have to pad it first using the special ``^V``
|
|
character.)
|
|
|
|
|
|
Private Variables
|
|
=================
|
|
|
|
Variables used exclusively within a class should begin with a underscore ('_')
|
|
character. An example class variable definition: ``protected
|
|
$_variablename;``
|
|
|
|
|
|
Array Definitions
|
|
=================
|
|
|
|
When defining arrays, or nested arrays, use the following format, where
|
|
indentation is noted via the closing parenthesis characters::
|
|
|
|
$arrayname['index'] = array(
|
|
'name1' => 'value1',
|
|
'name2' => array(
|
|
'subname1' => 'subvalue1',
|
|
'subname2' => 'subvalue2'
|
|
),
|
|
);
|
|
|
|
The only exception should be for empty or short arrays that fit on one line,
|
|
which may be written as::
|
|
|
|
$arrayname['index'] = array();
|
|
$arrayvar = array('foo1' => 'bar1');
|
|
|
|
|
|
Internationalization (I18n)
|
|
===========================
|
|
|
|
Mark all strings presented to the user as gettext strings by calling the
|
|
gettext shortcut function (``_()``)::
|
|
|
|
echo _("Hello world");
|
|
|
|
Don't use the gettext functions for strings that will be written to a log file
|
|
or otherwise presented to the administrator.
|
|
|
|
The Horde_String:: class contains several string manipulation methods that
|
|
are, as opposed to their PHP equivalents, locale and charset safe.
|
|
|
|
Use Horde_String::convertCharset() if you need to convert between different
|
|
character set encodings (for example, between user input and a storage backend
|
|
or data from an external source and the user interface). You don't need to
|
|
care if the character sets are really different.
|
|
|
|
Use the Horde_String::lower() and Horde_String::upper() methods without a
|
|
second parameter if you need to perform a locale-independent string
|
|
conversion. That's the case for all strings that are further processed or
|
|
interpreted by code. Use these methods with the second parameter set to true
|
|
for strings that need to be converted correctly according to the current (or
|
|
specified) character set.
|
|
|
|
Use the other Horde_String:: equivalents of PHP string functions to manipulate
|
|
strings correctly according to the current (or specified) character set but
|
|
use the PHP functions for code/machine processed strings.
|
|
|
|
|
|
Error checking
|
|
==============
|
|
|
|
Horde code should throw ``Horde_Exception`` objects (or equivalent Exception
|
|
derivatives) to report errors. The calling code should either explicitly
|
|
catch the exception and handle it or else the Horde-wide exception handler
|
|
will handle the exception, display an error message, and exit the script::
|
|
|
|
try {
|
|
$result = $something->call('may error');
|
|
// Succeeded.
|
|
} catch (Horde_Exception $e) {
|
|
// Handle error condition.
|
|
}
|
|
|
|
Do NOT return PEAR_Error objects. Any PEAR_Error objects should be converted
|
|
to a Horde_Exception before throwing.
|
|
|
|
When throwing a Horde_Exception the error message provided should not be
|
|
localized.
|
|
|
|
Throwing exceptions is expensive, so use them carefully for logic flow. You
|
|
should cache results of Horde_Core_Hooks#callHook() calls for example, if you
|
|
plan on calling a hook hundreds of time an access.
|
|
|
|
|
|
Exceptions in Framework Packages
|
|
================================
|
|
|
|
Exceptions thrown in packages can be divided into two categories: code/logic
|
|
exceptions and execution exceptions.
|
|
|
|
Code/Logic Exceptions
|
|
---------------------
|
|
|
|
These are exceptions that should be thrown when there is an issue with the
|
|
programming logic - e.g. the environment was not properly set up by an
|
|
application, or a required parameter was not provided.
|
|
|
|
These type of exceptions should throw one of the SPL Exceptions (e.g.
|
|
BadFunctionCallException, LogicException, RuntimeException), NOT a package
|
|
exception. This ensures that errors will normally cause a fatal error, rather
|
|
than potentially being caught and ignored by the calling code. As such, there
|
|
is no need to document the exception in phpdoc. Further, the exception
|
|
messages should not be translated, as these messages are intended for
|
|
developers not users.
|
|
|
|
Execution Exceptions
|
|
--------------------
|
|
|
|
These are exceptions that are thrown due to an error encountered while
|
|
performing the action requested by the calling code.
|
|
|
|
Any package that throws at least one execution Exception should define a
|
|
Horde_PackageName_Exception class that extends Horde_Exception. Execution
|
|
exceptions should use this class when throwing an error. Any method that
|
|
throws this kind of exception should document it in the phpdoc using the
|
|
@throws keyword. These messages should be translated, as they potentially
|
|
could be displayed to the user at the application level.
|
|
|
|
|
|
Existence checking
|
|
==================
|
|
|
|
Often you'll need to check whether or not a variable or property exists.
|
|
There are several cases here:
|
|
|
|
a. If you need to know if a variable exists at all and is not ``null``, use
|
|
`isset()`_::
|
|
|
|
// Check to see if $param is defined.
|
|
if (isset($param)) {
|
|
// $param may be false, but it's there.
|
|
}
|
|
|
|
b. If you need to know if a variable exists AND has a non-empty value (not
|
|
``null``, 0, ``false``, empty string or undefined), use !empty()::
|
|
|
|
// Make sure that $answer exists, is not an empty string, and is
|
|
// not 0:
|
|
if (!empty($answer)) {
|
|
// $answer has some non-false content.
|
|
} else {
|
|
// (bool)$answer would be false.
|
|
}
|
|
|
|
As pointed out in the comment of the else clause, `empty()`_ essentially does
|
|
the same check as `isset()`_ -- is this variable defined in the current scope?
|
|
-- and then, if it is, returns what the variable would evaluate to as a
|
|
boolean. This means that 0, while potentially valid input, is "empty" - so if
|
|
0 is valid data for your case, don't use !empty().
|
|
|
|
c. If you want to know if a variable is a non-empty string, including 0, you
|
|
have to use `strlen()`_. This only works with strings or scalars that
|
|
automatically cast to strings without problems::
|
|
|
|
// Make sure that $str is a non-empty string. $str must exist, so add an
|
|
// additional check with isset() if necessary.
|
|
if (strlen($str)) {
|
|
// $str has a non-zero length.
|
|
} else {
|
|
// $str is an empty string '', or null.
|
|
}
|
|
|
|
d. If you know you are working with a mixed variable then using just
|
|
`isset()`_ and `empty()`_ could cause unexpected results, for example if
|
|
testing for a key and the variable is actually a string::
|
|
|
|
$foo = 'bar';
|
|
if (isset($foo['somekey'])) {
|
|
// This will evaluate to TRUE!
|
|
}
|
|
|
|
If you know that there is a possibility of a mixed type variable the solution
|
|
in this case would be to add an `is_array()`_ check in the ``if()`` statement.
|
|
|
|
e. Use `array_key_exists()`_ when you want to check if an array key is defined
|
|
even if it has a value of ``null``::
|
|
|
|
// Make sure we have a charset parameter. Value could also be null.
|
|
if (!array_key_exists('charset', $params)) {
|
|
throw new Horde_Exception('Incomplete configuration');
|
|
}
|
|
|
|
Please note that `array_key_exists()`_ is a performance hit (25%-100%) and
|
|
should only be used when necessary. Instead try to use !empty() or
|
|
`isset()`_ instead.
|
|
|
|
.. _isset(): http://www.php.net/manual/en/function.isset.php
|
|
.. _empty(): http://www.php.net/manual/en/function.empty.php
|
|
.. _strlen(): http://www.php.net/manual/en/function.strlen.php
|
|
.. _is_array(): http://www.php.net/manual/en/function.is-array.php
|
|
.. _array_key_exists(): http://www.php.net/manual/en/function.array-key-exists.php
|
|
|
|
|
|
Quotes
|
|
======
|
|
|
|
You should always use single quote (') characters around strings, except where
|
|
double quote (") characters are required. All literal strings should be in
|
|
single quotes. A comparison of single and double quote usage follows:
|
|
|
|
Single Quotes:
|
|
* Variables in the string are not parsed or expanded.
|
|
* New line symbols can be included as literal line ends (not recommended).
|
|
* To include a single quote character, escape it with a ``\`` (backslash)
|
|
character, as in: ``echo 'Here\'s an example';``
|
|
* To specify a ``\`` (backslash) character, double it: ``echo 'c:\\temp';``
|
|
|
|
Double Quotes:
|
|
* Parses and expands variables in the string.
|
|
* Uses advanced (`sprintf`_-style) escape sequences like ``\n``, ``\$``,
|
|
``\t``, etc.
|
|
* Should be used in the gettext shortcut ``_("")`` format.
|
|
* Use with care, as many correct looking strings are really invalid.
|
|
|
|
The following are incorrect::
|
|
|
|
echo "Today is the $date['day'] of $date['month']"
|
|
$foo[index] = $foo["old_index"];
|
|
|
|
.. _sprintf: http://www.php.net/sprintf
|
|
|
|
|
|
define()
|
|
========
|
|
|
|
`define()`_ is a somewhat slow function in PHP (as of PHP 4.3.x) so excessive
|
|
use is discouraged. Every constant created by ``define()`` should be prefixed
|
|
with ``HORDE_``, its package name, or the application name.
|
|
|
|
Class constants should be defined inside the class using the ``const``
|
|
keyword.
|
|
|
|
|
|
.. _define(): http://www.php.net/manual/en/function.define.php
|
|
|
|
|
|
Security Considerations
|
|
=======================
|
|
|
|
The following are a non-exhaustive list of features/functions to take
|
|
special care with:
|
|
|
|
PHP Code Execution:
|
|
-------------------
|
|
|
|
require, include, require_once, include_once - Carefully audit any
|
|
variables used in these functions, and check the source of any
|
|
constants as well.
|
|
|
|
eval and create_function - Obvious danger if user input is supplied to
|
|
it in uncontrolled conditions.
|
|
|
|
preg_replace - The /e modifier is deprecated and must no longer be used.
|
|
|
|
Command Execution:
|
|
------------------
|
|
|
|
exec - Executes a specified command and returns the last line of output.
|
|
|
|
passthru - Executes a specified command and writes the output to STDOUT.
|
|
|
|
`` (backticks) - Executes the specified command and returns all the
|
|
output in an array.
|
|
|
|
system - Like passthru() but doesn't handle binary data.
|
|
|
|
popen - Executes a specified command and connects its output or input
|
|
stream to a PHP file descriptor.
|
|
|
|
File Disclosure:
|
|
----------------
|
|
|
|
File functions which can be potentially used to open remote or
|
|
unintended files: fopen, readfile, file, file_get_contents.
|
|
|
|
|
|
Optimizations
|
|
=============
|
|
|
|
The following optimizations should be used, if possible:
|
|
|
|
extension_loaded()
|
|
------------------
|
|
This appears to be an expensive PHP call. Use Horde_Util::extensionExists()
|
|
instead, which will cache the results of the call.
|
|
|
|
Concatenate strings
|
|
-------------------
|
|
Building a string with concatenation (the "." operator) using
|
|
single-quoted strings and variables is faster than using an
|
|
interpolated string (a string inside double quotes with variables
|
|
inside the string itself). In addition, concatenation is easier to
|
|
read and audit for logic and security problems.
|
|
|
|
Loops
|
|
-----
|
|
Make sure that you do not continue to define the same variable within a
|
|
loop. Instead, declare the variable a single time before the loop is run.
|
|
|
|
Additionally, for large amounts of data, do not use foreach() loops, as PHP
|
|
will make an additional copy in memory of every element of the array when
|
|
traversing. Instead, use either array_shift, a for() loop, or the
|
|
next()/each() functions. (NOTE: As of PHP 5, it is possible to indicate that
|
|
the values should be provided to the interior of the loop by reference,
|
|
thereby eliminating the need to create a copy of the value.)
|
|
|
|
Array
|
|
-----
|
|
Avoid frequent array accesses. If you use an array or an array member in a
|
|
loop, assign it to a variable first::
|
|
|
|
$a = array('x' => 'y');
|
|
$entries = array(...);
|
|
|
|
$length = count($entries);
|
|
$x = $a['x'];
|
|
for ($i = 0; $i < $length; ++$i) {
|
|
echo $x;
|
|
}
|
|
|
|
Don't use array_unique(). The following is reported to be 20 times faster::
|
|
|
|
$unique = array_keys(array_flip($foo));
|
|
|
|
Even better, if possible rework the algorithm to store the unique data in the
|
|
key and then call array_keys($foo) to return the unique list.
|
|
|
|
Avoid array_push(), the [] operator is much faster::
|
|
|
|
$array[] = 'element';
|
|
|
|
User defined functions
|
|
----------------------
|
|
User defined functions are more expensive than "regular" functions. Use them
|
|
only if they improve the code readability more than regular functions.
|
|
|
|
Dynamically created functions (eval and create_function)
|
|
--------------------------------------------------------
|
|
Code executed with eval, and functions created with create_function,
|
|
are essentially PHP code that the engine has to parse every time it is
|
|
run. This code is impossible to cache with an opcode cache, and even
|
|
without one is slower than regular PHP code. Because they also have
|
|
security risks, they should be avoided if at all possible.
|
|
|
|
Disk I/O
|
|
--------
|
|
Disk read and write operations are slow. If possible read and write files in
|
|
large chunks. According to the PHP documentation, file_get_contents() is
|
|
potentially much faster than using fopen()/fread()/fclose(). BUT the
|
|
trade-off is the need to read the entire file into memory, so use of
|
|
file_get_contents() depends on the data size.
|
|
|
|
|
|
STDIN/STDOUT/STDERR
|
|
===================
|
|
|
|
To access either STDIN, STDOUT, or STDERR, the following code should be used::
|
|
|
|
while (!feof([STDIN|STDOUT|STDERR])) {
|
|
$line = fgets([STDIN|STDOUT|STDERR]);
|
|
// process $line here
|
|
}
|
|
|
|
|
|
PHP Function Issues
|
|
===================
|
|
|
|
fpassthru()
|
|
-----------
|
|
This function can fail depending on the size of the stream. Instead, use
|
|
fread() to get the stream data::
|
|
|
|
// NO: fpassthru($stream);
|
|
while (!feof($stream)) {
|
|
echo fread($stream, 8192);
|
|
}
|
|
|
|
|
|
Hashing Algorithm
|
|
=================
|
|
|
|
Recommended Algorithm
|
|
---------------------
|
|
FNV1-32 is the recommended algorithm to use for non-cryptographic hashing
|
|
actions. Unfortunately, it is not available until PHP 5.4. For PHP 5.3, use
|
|
SHA-1 instead::
|
|
|
|
$hash = hash(PHP_MINOR_VERSION >= 4) ? 'fnv132' : 'sha1', $string);
|
|
|
|
MD5 should NOT be used, as it has known collision issues. (NOTE: SHA-1 hashes
|
|
are 160-bit vs. MD5 hashes which are 128-bit; care needs to be taken if
|
|
switching between methods regarding storage side of the hash string. FNV1-32
|
|
hashes are 32-bit.)
|
|
|
|
|
|
CSRF Token protection
|
|
=====================
|
|
|
|
Any "destructive" action must be protected by a token to prevent CSRF attacks.
|
|
The session token can be retrieved via Horde_Session#getToken() and checked
|
|
via Horde_Session@checkToken().
|