Filtered by category Coding Standards, 1 - 10 of 19 Postings (
all,
summary)
Created by Gustaf Neumann, last modified by Gustaf Neumann 24 Jul 2023, at 05:19 PM
The maintained OpenACS packages are regularly tested with state-of-the-art web vulnerability scanners. The packages are developed using the following the guidelines.
Protecting against SQL injection (SQI):
- Use colon-prefixed bind variables in SQL statements when possible (for local variables)
# use bindvars with local variables
set x 0
db_0or1row . {select * from acs_objects where object_id = :x}
or pass bind variables explicitly, using the -bind option
# use bind variables by passing a dictionary (attribute value list)
set bindvars {object_id 0 security_inherit_p t}
db_0or1row . {
select * from acs_objects
where object_id = :object_id
and security_inherit_p = :security_inherit_p
} -bind $bindvars
- In case, bind-variables are not possible, use ns_dbquotevalue or ns_dbquotelist (see NaviServer documentation)
set x 0
db_0or1row . [subst {select * from acs_objects where object_id = [ns_dbquotevalue $x]}]
Manual page
- In general, avoid using double-quoted SQL statements whenever possible to avoid unescaped substitutions.
Protecting against cross side scripting (XSS):
- Validate all input values using page contracts with value checkers, and validate all form variables in the form specification.
- Make sure that Content Security Policies (CSP) are activated in the package parameters. When you have to relax the CSP roles, use "security::csp::require" as local as possible (e.g. on the page level) and not on the site level.
Created by Rocael Hernández Rizzardini, last modified by Gustaf Neumann 24 Jul 2023, at 10:45 AM
A coding style is always important to maintain quality of the code and in this case, the OpenACS project. Here you'll find a set of links that will guide through our most common standards.
The definitive guide on coding standards can be found at OpenACS Style Guide.
Many stuff has been gathered from many post or guides other openacs community members have done, such as:
Created by Rocael Hernández Rizzardini, last modified by Gustaf Neumann 28 Sep 2022, at 11:32 AM
- Use namespace
Define your procs with a namespace like mypackage::foo_proc
. Here is a discussion about [this]. Check many examples in the code, example:
namespace eval auth {}
ad_proc -public auth::require_login {
{-level ok}
{-account_status ok}
} {
doc...
@return something
@see ad_script_abort
} {
... proc body
}
- Use procs safely and their safer variations to help keep code robust and avoid security issues.
Particularly in cases, where user_input is processed, be sure to avoid executing unwanted code. Use the Tcl expand operator {*}
instead of eval
. Use
{*}$cmd
instead of
eval $cmd
For legacy code, you might use util::safe_eval instead of eval in such cases; subst_safe precedes meta characters with backslashes.
- Use named parameters whenever possible
Define named parameters on your procs such that parameters will not be mixed up if somebody makes a mistake on passing the order of parameters. Also, this makes the proc easier to add additional parameters in the future if needed.
Use:
ad_proc proc_name { {-parent_id pid} {-child_id cid} } ...
and not
ad_proc proc_name {pid cid} ...
This way, developers will need to call proc stating explicitly which parameter are passed. This is especially useful when some parameters are optional.
Also, when calling a proc in your Tcl script, it is recommended to write one parameter per line like this:
set my_var [proc_name \
-parent_id $pid \
-child_id $cid]
Rather than:
set my_var [proc_name -parent_id $pid -child_id cid]
Again, this helps to make the code more clean and readable.
- Use ad_proc to define your Tcl procs
Make use of ad_proc. And make use of the self documentation facility of ad_proc.
ad_proc foo {}
Use this area to document
}
# .... your implementation of proc foo
}
This way, the API browser will pick up your documentation automatically. Is encouraged to use automatic api-doc documentation that ad_provides, such as: @author
, @return
, @see
-
Use "ad_proc -private ..." always when a proc is used only in one package
This reduces the size of the public API and improves the flexibility of the package maintainers.
-
Use "ad_proc -deprecate ..." when removing definitions from the public API
When deprecated code is called, the error.log of the site will show its usage. This way, a site maintainer can update with code with the new replacement call.
Don't move deprecated calls immediately to the long-range backward compatibility procs (tcl/deprecated-procs.tc). When OpenACS is configured to omit loading of long deprecated code (WithDeprecatedCode set to 0) these files are not loaded to reduce the every growing blueprint bloat. Therefore, these files should only contain code, which was deprecated at LEAST ONE RELEASE EARLIER, such that site admins have one release time to fix calls to deprecated code.
- Avoid using "upvar"
Try to avoid using "upvar". If needed, pass in a parameter that specifies the "upvar" name. This way, the one using your proc has the option to name his/her variable. Example:
ad_proc upvaring {-upvar_name:required} {
upvar $upvar_name local_var
}
-
Use modern Tcl idioms
Do not use "==
" in comparing strings. Using "if {$string == "foo"}
" tries to make a numeric comparison first. Instead, make use of "if {"foo" eq $string}
" or if you need the negation "if {"foo" ne $string}
".
Do not use "if {[lsearch -exact $list $element] > -1}
", but use "if {$element in $list}
" instead, or "if {$element ni $list}
" in case a "not in" test is required.
-
Always "return" at the end of your proc
And if you have to return more than one variable, use associative arrays, which can be extended by additional fields without breaking code
So instead of this:
ad_proc ... {
.....
return [list $creation_status $creation_message ...]
}
use key/value pairs or Tcl arrays to group related information:
ad_proc ... {
array set creation_info {
creation_status {}
creation_message {}
element_messages {}
account_status {}
account_message {}
}
.....
return [array get creation_info]
}
- ... or even better: use Tcl dicts
ad_proc proc ... {} {
set creation_info [dict create \
creation_status {} \
creation_message {} \
element_messages {} \
account_status {} \
account_message {} ]
....
return $creation_info
}
- Read the Tcl Style guide
This is the Tcl style guide (PDF), try to apply relevant guidelines. In particular chapter 4,5 and 7
Created by Rocael Hernández Rizzardini, last modified by Héctor Romojaro 27 Jan 2021, at 03:47 PM
- Use always lowercase: file.tcl
- Separate names with dash “-“: my-file.tcl
- Use English for naming (at least if you plan to contribute to the community)
- Use generic names, such as:
- my-page-add-edit.tcl
- my-page-delete.tcl
- index.tcl
- Follow the object-action naming convention
- my-page-delete.tcl
- NOT delete-my-page.tcl
Procedures (Tcl functions)
- Usually in lowercase (only in proc callbacks or service contracts you will use TheFirstLetterOfEachWord as uppercase)
- Separate words with “_”: my_proc
- Use names that indicate what the proc does, usually not too long
- my_package::user::add_to_class
- Always use namespaces
- The namespace must be the same as the package-key, just that with “_” if applies
-
- If package-key is: my-package
-
- Then: my_package::add_user
- Follow the package_key::object::action convention, use
- my_package::user::add_to_class
- NOT my_package::add_to_class::user
- If you have only one action on users you might also use:
- my_package::add_user_to_class
SQL Statements
- Lowercase
- Separate words with "_": select_users_in_class
- Use names that indicate what the SQL statement does, or what it returns
- db_list list_of_unregistered_user_ids {}
- db_1row user_info {}
- db_dml update_user_biography {}
- Preferably, in db_multirow calls, keep your statement name the same as the multirow name
- For PL/SQL look here
- Read the document behind the link
- All constraints have to be named
- The naming convention for constraints is as follows:
- Primary Key: tablename_pk (acs_mail_lite_queue_pk)
- Unique: tablename_columnname1_columnname2_un
- Foreign Key: tablename_columnname_fk ( acs_mail_lite_queue_pck_fk)
- Check: tablename_column_name_ck (acs_mail_lite_co_qu_use_sender_p_ck)
- Column names can be abbreviated (e.g. pck instead of package_id)
- Lowercase
- Separate words with "_": user_id
- Use names that indicate what the variable contains. If this matches with a database column, use the same name as used for the column in the database.
- Use the following conventions for variables containing
- IDs: Append "_id" at the end, like "user_id"
- Booleans: Append "_p" at the end, like "employee_p" to define if the user is an employee
- Arrays: Append "_arr" at the end, like "employee_arr". This way you can immediately detect arrays in the code.
Package Parameters
- Use camel case, start with uppercase character
- Example "IndexRedirectUrl"
Created by Malte Sussdorff, last modified by Poor Yorick 27 Oct 2020, at 01:35 PM
By convention, OpenACS stores all its SQL code in .xql files, which facilitates support for OpenACS on multiple databases. Though this adds an extra burden to the development, please adhere to this convention.
- Keep SQL scripts out of .tcl or .adp files, unless they conform to SQL99. See details below.
- Assign a name to each db_statement. see Naming Conventions.
- Database-specific SQL statements, i.e. statements not conforming to SQL99, belongs in a -database.xql file:
- select content_item__new() => content-item-postgresql.xql
- select item_id from content_items => content-item.xql
- Do not spread pieces of an SQL statement into multiple places, e.g. inline in Tcl, generic .xql, database-specific .xql.
- If in doubt, be conservative and add SQL scripts into the database-specific file of the database you are working with.
- Format each SQL script properly. E.g. break each statement into multiple lines as appropriate to make it more readable. See code-formatting
To make changes to a .xql file take effect, reload the file.
As an exception to the rule about keeping SQL out of Tcl scripts, pieces of an SQL script that is dynamically generated may be placed in a Tcl script if they conform to the SQL 99 standard. This is often the case for "where_clause" of listbuilder.
Creation of tables in PostgreSQL:
- Do not use the "SERIAL" datatype. SERIAL is an ugly hack meant to make it easier to port Sybase scripts to PostgreSQL. We have a standard way of defining integer primary keys and giving values to them: Integer with a sequence.
- When creating a sequence use the CREATE VIEW hack so sequence queries can be shared between PG and Oracle without any extra work. Otherwise, do the work to make the sequence work both in -oracle.xql as well as -postgresql.xql
Created by Rocael Hernández Rizzardini, last modified by Gustaf Neumann 18 Jan 2020, at 10:54 AM
- Avoid putting in Tcl code on ADP pages if possible
Although AOLserver/NaviServer ADP supports placing Tcl codes directing into ADP pages, one should used the ADP system wherever possible (see e.g. Using Templates in OpenACS or OpenACS Templating System).
- Quote in the master, pass "properties" literally from slave adp files
when variables are used in templates without modifiers (marked with a ";") then the values of the variables are internationalized and html-quoted. The substitutions should be done at the place, where the variables are actually used, which is for "properties" in the master templates. That the places, where the variable values are just passed on, the modifier ";literal" should be used to prevent quoting and internationalization.
Master:
<head>
<title>@doc.title@</title>
</head>
<body bgcolor="#ffffff">
<h1>@heading@</h1>
<slave>
</body>
Slave:
<master>
<property name="doc(title)">@title;literal@</property>
<property name="heading">@title;literal@</property>
...
Passing arguments to ADP includes:
<include src="name-of-included-adp" ... var="@value;literal@" ...>
or one can pass variables via reference to the include
<include src="name-of-included-adp" ... &="varName" ...>
- Pass always the "context" and "doc(title)" properties to the site master template
Example:
<property name="doc(title)">@title;literal@</property>
<property name="context">@context;literal@</property>
- Quote HTML attributes
Quoting HTML attribute values improves the safety against XSS attacks, especially when the attribute values are variables. Double quotes are preferred over single quotes, both are fine.
-
Common doc properties
The following doc properties are used in the blank-master template:
- doc(title): Title of the document
- doc(lang): Language of the document
- doc(type): HTML doc type declaration
- doc(base_href): The base URL to be used throughout the document for relative URLs (see base element)
- doc(base_target): A keyword or author-defined name of the default browsing context to display the result when links or forms cause navigation, for <a> or <form> elements without an explicit target attribute. (see base element)
Created by Rocael Hernández Rizzardini, last modified by Gustaf Neumann 22 Mar 2019, at 11:21 PM
Use ad_form to create HTML forms. If you work on a page that does not, convert it to use ad_form.
You can grep the toolkit under /packages and see many examples.
Ad_form can handle many of your possible interactions with the system, such as normal tasks as add and edit data, validate entry form data, etc.
Additional information here:
Rubick on ad_form
Use the following in the tcl part of a page to limit access to page requests via post.. to reduce vulnerability to url hack and insertion attacks from web:
if { [ad_conn method] ne "POST" } {
ad_script_abort
}
Grouping elements into sections using ad_form
The {-section} list allows to group the subsequent elements (until the next section declaration) into a section. {-section} accepts 4 properties:
{-section
section_id
{legendtext $legendstring}
{legend {name value name value ... name value}}
{fieldset {name value name value ... name value}}
}
where:
- section_id: a string to identify the section (mandatory)
- legendtext (optional) a string for the legend of the fieldset (section)
- legend (optional) a list of name value pairs attributes for the LEGEND tag
- fieldset (optional) a list of name value pairs attributes for the FIELDSET tag
Example
ad_form \
-name "your_zen_level" \
-method post -html {enctype multipart/form-data class margin-form} \
-fieldset {{title "T1" class "C1"} "This really works!!"} \ -form { # Start section 1 {-section "sec1" {legendtext "Section Title I"} {legend {class myClass id myId}}}
{zen_comment:text(comment)\
{label "template::widget::comment"}\
{value "Please enter your comments."}\
{html {rows 7 cols 50}}}
{zen_file:text(file),optional\
{label "template::widget::file"}}
# Start section2
{-section "sec2" {legendtext "Section Title II"} {fieldset {class myclass}}}
{zen_multiselect:text(multiselect)\
{label "template::widget::multiselect"}\
{options {"mark" "emma" "avni" "carl" "don"}}}
# Unset the section. subsequent elements will not be in any section.
{-section ""}
{zen_text:text(text),optional,nospell\
{label "template::widget::text"}\
{help_text {"Your identification tag number"}}}
{zen_textarea:text(textarea),optional,nospell\
{label "template::widget::textarea"}\
{help_text {"Please describe your desired state of being"}}\
{html {rows 7 cols 50}}}
}
Created by Rocael Hernández Rizzardini, last modified by Gustaf Neumann 14 Feb 2018, at 09:07 AM
- Use 4 space indentation
- Code lines must not exceed 80 characters in length. Never use a single line that has more than 80 chars.
- Source files should be coded in UTF-8 using Unix-style newlines
These general rules are applicable to Tcl & SQL.
Also, the 80 character limit is important because the OpenACS API browser (at /api-doc/) presents documentation defined in ad_proc's doc_string declaration in html format; whereas ad_proc's body of code is presented wrapped in PRE tags. Longer code lines result in API documentation pages (and their paragraphs) stretching outside of typical browser window width.
If you are using Emacs editor, you usually are on the safe side with regards to Tcl, as the tcl-mode indents just as we want it. ADP pages are formatted correctly when editor is in html-mode.
Formatting SQL Statements
- A proper format for select statements is
select ....
from ....
where clause
and clause
and clause
- A proper format for insert statements (and note the indentation and overflow of attributes)
insert into table_name
(c1, c2, c3 ...
cx, cy, cz)
values
(v1, v2, v3 ...
vx, vy, vz)
- A proper format for update statements:
update table
set c1=v1,
c2=v2,
...
where ...
Editor modes
There is an OpenACS mode for emacs OpenACS mode for Emacs which has features to help meet formatting standards.
One can add the following stanza to the end of a .tcl file, when using Emacs, to avoid, that spaces are changed again into tabs.
#
# Local variables:
# mode: tcl
# tcl-indent-level: 4
# indent-tabs-mode: nil
# End:
Security Considerations
Allowing user input to include images, see patch in this discussion thread: https://openacs.org/forums/message-view?message_id=182057
Created by Gustaf Neumann, last modified by Gustaf Neumann 29 Dec 2017, at 10:11 AM
- Write your commit message in the imperative present tense: Use "Fix bug" and not "Fixed bug" or "Fixes bug." This convention matches up with commit messages generated by commands like git merge and git revert. It should start with a verb like "Fix", "Add" or "Change".
- Commit message should contain a title and an optional body.
- The title of the commit message should be a capitalized, short (50 chars or less) summary, not ending with a period.
- The title can be followed by the body, an explanatory text, if necessary. Title and body are separated by an empty line. The blank line separating the summary from the body is critical (unless you omit the body entirely); tools like rebase can get confused if you run the two together.
- The body should be wrapped to 72 characters. The body can contain multiple paragraphs, containing plain text or bullet points. The bullet points should be typed as a hyphen or asterisk with blank lines in between. Use a hanging indent for longer bullet points
- Make white-space changes separately
See also:
Created by Lee Denison, last modified by Gustaf Neumann 01 Jul 2017, at 01:25 PM
Implementation Details
State of the Code
Template::head is now done and available in OpenACS 5.4. See https://openacs.org/api-doc/procs-file-view?path=packages/acs-templating/tcl/head-procs.tcl
template::head::*
The template::head::* API manipulates the head section of the document that will be returned to the users client. Packages should use this API to add package specific JavaScript code, CSS, link tags and meta tags to the HTML document. The API consists of:
template::head::add_script [-defer] -type <type> [-src <src>] [-charset <charset>] [-script <script>] [-order <order>]
Add a script to the head section of the document to be returned to the users client. A script library in an external file may only be included once; subsequent calls to add_script will replace the existing entry. Anonymous script blocks will be added without checking for duplicates; the caller must ensure that anonymous script blocks are not inadvertently added multiple times. You must supply either src or script.
- @param type the type attribute of the script tag, eg. 'text/javascript'
- @param defer whether execution of the script should be deferred until after the page has been loaded
- @param src the src attribute of the script tag, ie. the source URL of the script
- @param charset the charset attribute of the script tag, ie. the character set of the script if it differs from the main document
- @param script the inline script for the body of the script tag. This parameter will be ignored if a value has been supplied for src
- @param order default 0 (unordered) order the JavaScript should be loaded in
template::head::add_link -rel <rel> -href <href> [-type <type>] [-media <media>] [-title <title>] [-lang <lang>]
Add a link tag to the head section of the document to be returned to the users client. A given target document may only be added once for a specific relation; subsequent calls to add_link will replace the existing entry.
- @param rel the rel attribute of the link tag defining the relationship of the linked document to the current one, eg. 'stylesheet'
- @param href the href attribute of the link tag, eg. the target document of the link
- @param type the type attribute of the link tag, eg. 'text/css'
- @param media the media attribute of the link tag describing which display media this link is relevant to. This may be a comma separated list of values, e.g. 'screen,print,braille'
- @param title the title attribute of the link tag describing the target of this link
- @param lang the lang attribute of the link tag specifying the language of its attributes if they differ from the document language
template::head::add_meta [-http_equiv <http_equiv>] [-name <name>] [-scheme <scheme>] [-content <content>] [-lang <lang>]
Add a meta tag to the head section of the document to be returned to the users client. A meta tag with a given name or http-equiv may only be added once; subsequent calls to add_meta will replace the existing entry. You must supply either name or http_equiv.
- @param http_equiv the http-equiv attribute of the meta tag, ie. the HTTP header which this metadata is equivalent to eg. 'content-type'
- @param name the name attribute of the meta tag, ie. the metadata identifier
- @param scheme the scheme attribute of the meta tag defining which metadata scheme should be used to interpret the metadata, eg. 'DC' for Dublin Core (http://dublincore.org/)
- @param content the content attribute of the meta tag, ie the metadata value
- @param lang the lang attribute of the meta tag specifying the language of its attributes if they differ from the document language
template::head::add_javascript [-defer] [-src <src>] [-script <script>] [-charset <charset>] [-order <order>]
Add a script of type 'text/javascript' to the head section of the document to be returned to the users client. This functions is a wrapper around template::head::add_script. You must supply either src or script.
- @param defer whether execution of the script should be deferred until after the page has been loaded
- @param src the src attribute of the script tag, ie. the source url of the script
- @param script the inline script for the body of the script tag. This parameter will be ignored if a value has been supplied for src
- @param charset the charset attribute of the script tag, ie. the character set of the script if it differs from the main document
- @param order optional defaults to 0 (load in order add_javascript is called) set the order a JavaScript will be loaded
@see template::head::add_script
template::head::add_css [-alternate] -href <href> [-media <media>] [-title <title>] [-lang <lang>]
Add a link tag with relation type 'stylesheet' or 'alternate stylesheet', and type 'text/css' to the head section of the document to be returned to the users client. A given target style-sheet may only be added once; subsequent calls to add_css will replace the existing entry. This function is a wrapper around template::head::add_link.
- @param href the href attribute of the link tag, eg. the target style-sheet
- @param alternate sets the rel attribute of the link tag defining to 'alternate style-sheet' if set, sets it to 'stylesheet' otherwise
- @param media the media attribute of the link tag describing which display media this link is relevant to. This may be a comma separated list of values, eg. 'screen,print,braille'
- @param title the title attribute of the link tag describing the target of this link
- @param lang the lang attribute of the link tag specifying the language of its attributes if they differ from the document language
@see template::head::add_link
template::add_body_handler [-event <event>] [-script <script>] [-identifier <identifier>]
Adds javascript code to an event handler in the body tag. Several
javascript code blocks may be assigned to each handler by subsequent calls
to template::add_body_handler.
If your script may only be added once you may supply an identifier.
Subsequent calls to template::add_body_handler with the same identifier
will replace your script rather than appending to it.
event may be one of:
- onload
- onunload
- onclick
- ondblclick
- onmousedown
- onmouseup
- onmouseover
- onmousemove
- onmouseout
- onkeypress
- onkeydown
- onkeyup
@param event the event during which the supplied script should be
executed
@param script the javascript code to execute
@param identifier a name, if supplied, used to ensure this javascript code
is only added to the handler once
template::add_body_script [-type <type>] [-defer <defer>] [-src <src>] [-charset <charset>] [-script <script>]
Add a script to the start of the body section of the document to be returned
to the users client. You <strong>must</strong> supply either src or script.
@param type the type attribute of the script tag, eg. 'text/javascript'
@param defer whether execution of the script should be defered until after
the page has been loaded
@param src the src attribute of the script tag, ie. the source url of the
script
@param charset the charset attribute of the script tag, ie. the character
set of the script if it differs from the main document
@param script the inline script for the body of the script tag. This
parameter will be ignored if a value has been supplied for
src
template::add_header [-direction "outer"] [-src <src>] [-params ""] [-html ""]
Add a header include to the beginning of the document body. This function
is used by site wide services to add functionality to the beginning of a
page. Examples include the developer support toolbar, acs-lang translation
interface and the acs-templating WYSIWYG editor textarea place holder. If
you are not implementing a site wide service, you should not be using this
function to add content to your page. You must supply either src or html.
@param direction whether the header should be added as the outer most
page content or the inner most
@param src the path to the include
@param params a list of name, value pairs to pass as parameter to the
include
@param html literal html to include in the page. This parameter will
be ignored if a values has been supplied for src.
@see template::add_footer
template::add_footer [-direction "outer"] [-src <src>] [-params <params>] [-html <html>]
Add a footer include to the end of the document body. This function
is used by site wide services to add functionality to the end of a
page. Examples include the developer support toolbar, acs-lang translation
interface and the acs-templating WYSIWYG editor textarea place holder. If
you are not implementing a site wide service, you should not be using this
function to add content to your page. You must supply either src or html.
@param direction whether the footer should be added as the outer most
page content or the inner most
@param src the path to the include
@param params a list of name, value pairs to pass as parameter to the
include
@param html literal html to include in the page. This parameter will
be ignored if a values has been supplied for src.
@see template::add_header
template::reset_request_vars
Reset all templating variables which must be initialized on every request.
template::reset_request_vars would be called at the beginning of the tcl/adp extension handler (adp_parse_ad_conn_file) to reinitialise the global variables used by this API on each request.
legacy information
Proposal: template::head::*
Goals of this proposal, ie. why create template::head::*?
I believe the introduction of these APIs would:
- Better position the toolkit to embrace AJAX technologies.
- Make upgrades easier by reducing the need to manually update customized master templates.
- Enable packages developers introduce package specific JavaScript and CSS without needing to assess the toolkit wide impact.
- consistent handling for adding code to document head from any Tcl script including includes. This means an include that requires a particular CSS or JavaScript can add this to the head of the page easily
- for JavaScript or CSS, if multiple calls are made for the same resource, it can be loaded only once. In this way code can declare the requirements without needing to know if other code loads the same resource
- Support to add header or footer to the document body
Changes
- Create global data structures, and associated APIs, to programmatically determine the contents of the document head section without the need for template property tags and the maintenance cost that introduces.