Skip to content

[Update] Documentation for sniff WordPress.PHP.DiscouragedPHPFunctions #2584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

[Update] Documentation for sniff WordPress.PHP.DiscouragedPHPFunctions #2584

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b91fb4f
adds documentation for sniff
tikifez Sep 17, 2024
d12e715
updated formatting, indentation, add <em> tags
jasonkenison Aug 26, 2025
46f08e3
update indentation, add <em>, linebreak end of document
jasonkenison Aug 26, 2025
a8fffd4
include more details in obfuscate code standard
jasonkenison Aug 26, 2025
b5a5872
requested changes
jasonkenison Sep 14, 2025
013e0d0
fix spelling error
jasonkenison Sep 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions WordPress/Docs/PHP/DiscouragedPHPFunctionsStandard.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?xml version="1.0"?>
<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
title="Discouraged PHP Functions"
>
<standard>
<![CDATA[
Use JSON instead of serialized data, which has known vulnerability problems with object injection.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here and in all other <standard> blocks, the description should be indented four spaces.

Suggested change
Use JSON instead of serialized data, which has known vulnerability problems with object injection.
Use JSON instead of serialized data, which has known vulnerability problems with object injection.

]]>
</standard>
<code_comparison>
<code title="Valid: Using JSON for serialized data.">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have a specific suggestion, but I think this title can be improved. Maybe Using JSON to encode data or JSON encoding data. As a non-English native speaker, I'm not sure, but I don't think the examples below are about Using JSON for serialized data as the data is not necessarily serialized before it is passed to the JSON functions. Makes sense?

<![CDATA[
$serialized = <em>json_encode</em>( $array );
$serialized = <em>wp_json_encode</em>( $array );
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that just a single valid example encoding $array is enough. I don't see a reason for two. Maybe keep this line and remove the one above?

$unserialized = <em>json_decode</em>( $array );
]]>
</code>
<code title="Invalid: Using serialized data strings.">
<![CDATA[
$serialized = <em>serialize</em>( $array );
$unserialized = <em>unserialize</em>( $array );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is better to use a variable name other than $array here as this function takes an string as its first parameter? The same applies to the json_decode() example above.

Please check if other variables could be renamed as well for the same reason in the other examples.

]]>
</code>
</code_comparison>
<standard>
<![CDATA[
URLs should now be encoded using rawurlencode(). Only legacy applications should use urlencode().
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is not necessary to include the second sentence? The sniff will trigger anyway if it finds urlencode(). If there is a reason to include the second sentence, maybe it makes sense to add another sentence explaining what determines the cases where legacy applications should use urlencode(). I don't remember off the top of my head.

]]>
</standard>
<code_comparison>
<code title="Valid: Encoding a url using rawurlencode().">
<![CDATA[
<em>rawurlencode</em>( get_site_url() );
]]>
</code>
<code title="Invalid: Encoding a url using urlencode().">
<![CDATA[
<em>urlencode</em>( get_site_url() );
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
Avoid using functions which change configuration values at runtime.
]]>
</standard>
<code_comparison>
<code title="Valid: Not changing configuration at runtime.">
<![CDATA[
// Configuration not changed at runtime.
]]>
</code>
<code title="Invalid: Changing configuration at runtime">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing period

Suggested change
<code title="Invalid: Changing configuration at runtime">
<code title="Invalid: Changing configuration at runtime.">

<![CDATA[
error_reporting( 0 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This block and most of the blocks below are missing <em> tags.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm unsure (here and in other <code_comparison> blocks) if it is necessary to include an example for every single function that triggers the sniff. Please check other sniff XML docs if you can, but I believe it is more common to just include one example (if all the others are the same except for the function name). Then, if the list is not too long, you can consider mentioning all the functions in the corresponding <standard> description. What do you think?

ini_restore( $option );
apache_setenv( $variable, $value );
putenv( $assignment );
set_include_path( $include_path );
restore_include_path();
magic_quotes_runtime( $new_setting );
set_magic_quotes_runtime( $new_setting );
dl( $extension_filename );
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
Do not use PHP system calls. They are often disabled by server admins.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this sniff triggers a warning and not an error, I suggest rephrasing this description to make it explicit that it is a recommendation. The description above is a code example. Maybe something like "Avoid using PHP system calls as they are often disabled by server administrators."

]]>
</standard>
<code_comparison>
<code title="Valid: Not using PHP system calls.">
<![CDATA[
// Avoiding using PHP system calls.
]]>
</code>
<code title="Invalid: Using PHP system calls.">
<![CDATA[
exec( $command );
passthru( $command );
proc_open( 'php', $desc, $pipes, $cwd, $env );
shell_exec( $command );
system( $command );
popen( $command, $mode );
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
Functions often used for obfuscating code are strongly discouraged. Make sure the function is used for benign reasons.
]]>
</standard>
<code_comparison>
<code title="Valid: Using functions for benign reasons.">
<![CDATA[
base64_encode( $string );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please double-check, but I believe the examples provided here actually trigger the sniff and thus are invalid. If I'm correct, the sniff triggers a warning if one of those functions is used, regardless of how it is used. It is up to a developer to determine if the function is used for benign purposes if they want to keep it. Let me know if you need help testing the sniff.

base64_decode( $encrypted_string );
convert_uuencode( $string );
convert_uudecode( $encrypted_string );
str_rot13( $string );
]]>
</code>
<code title="Invalid: Using functions to obfuscate code.">
<![CDATA[
<em>eval( </em>base64_decode( $code_str )<em> )</em>;
<em>eval( </em>convert_uudecode( $uuencoded )<em> )</em>;
<em>eval( </em>str_rot13( $rot13_encoded )<em> )</em>;
]]>
</code>
</code_comparison>
</documentation>
Loading