Update:Archive/1.0/Developers: Difference between revisions

no edit summary
No edit summary
 
(16 intermediate revisions by 3 users not shown)
Line 1: Line 1:
[[Update:Home_Page|Update Home Page]] » [[Update:Development|Development]] » Audit
{{AmoArchive}}


== Introduction ==
== Introduction ==
Line 237: Line 237:
=== Code Adjustments ===
=== Code Adjustments ===
* <strike>[https://bugzilla.mozilla.org/show_bug.cgi?id=283215 Patch branch to include local modifications in prod.] (under review)</strike>
* <strike>[https://bugzilla.mozilla.org/show_bug.cgi?id=283215 Patch branch to include local modifications in prod.] (under review)</strike>
* Update code based on quantified 'needed changes'(delayed until 03/16)
* Patching
** <strike>Rewrite VersionCheck.php</strike> (morgamic)
** <strike>Rewrite VersionCheck.php</strike> (morgamic)
*** This is currently being reviewed. (mconnor)
*** <strike>This is currently being reviewed. (mconnor)</strike>
** Update UMO configuration and structure to make development easier. (morgamic)
** <strike>Update UMO configuration and structure to make development easier.</strike> (morgamic)
*** Stripping out stuff like hard-coded paths
*** <strike>Stripping out stuff like hard-coded paths</strike>
*** Adding needed directories to CVS
*** <strike>Adding needed directories to CVS</strike>
*** Documenting requirements and installation procedure
*** Documenting requirements and installation procedure in Wiki (need help with this)
** Get PFS into CVS (morgamic, jst, kveton)
** <strike>Get PFS into CVS (morgamic, jst, kveton)</strike>
*** uniqueUrl.php needs to be excluded  
*** <strike>uniqueUrl.php needs to be excluded</strike>
*** it should probably be put into the private CVS
** <strike>Analyze</strike>, optimize PFS scripts. (morgamic, pending import being cleared by jst)
*** other scripts can go into CVS
** <strike>Prioritize needed work and create TODO lists.  (morgamic, alanjstr, pending patches)</strike> -- see hitlist below
** Analyze, optimize PFS scripts. (morgamic)
 
** Prioritize needed work and create TODO lists.  (morgamic, alanjstr)
** Deal out important patching (based on audit log) to community based on wiki information. (kveton, morgamic,alanjstr)
** Deal out important patching to community based on wiki information. (kveton, morgamic,alanjstr)
** <strike>Unfreeze branch and open it up for patching. (kveton)</strike>
** Unfreeze branch and open it up for patching. (kveton)
 
* If possible, update code for readability and efficiency
=== Hit list ===
# Developer Sessions, making sure login works correctly, nav options for admins are hidden (morgamic)
# Comment spam prevention for user ratings, reenabling comments for extensions and themes (chip)
# <strike>Set up -dev,-staging to automatically pull from branch, trunk(kveton)</strike>
# QA for submitted patches (Ctho, alanjstr)
# Run through remaining items in audit log that are important mainly (but not limited to):
## missing input validation
## bad loops
# Work through workflows for developers and admins (everyone)
# Merge branch with trunk (kveton)
# Push trunk into production (justdave,kveton)


=== Assessment ===
=== Assessment ===
Line 284: Line 294:


* [https://bugzilla.mozilla.org/buglist.cgi?product=Update&chfieldfrom=2005-01-01&chfieldto=Now&chfield=%5BBug+creation%5D&chfield=resolution&query_based_on=UMO+1.1&field0-0-0=bug_group&type0-0-0=equals&value0-0-0=update-security Security Bugs]
* [https://bugzilla.mozilla.org/buglist.cgi?product=Update&chfieldfrom=2005-01-01&chfieldto=Now&chfield=%5BBug+creation%5D&chfield=resolution&query_based_on=UMO+1.1&field0-0-0=bug_group&type0-0-0=equals&value0-0-0=update-security Security Bugs]
* usage of uriparams() is done even if it doesn't make any sense


===/.===
===/.===
Line 318: Line 329:
* why is there a function defined in this file? it's a "config" file
* why is there a function defined in this file? it's a "config" file
* This config file should use define() for configuration constants, or at least use a $config[] array so that any configuration variables are clearly marked when used during runtime in a script.
* This config file should use define() for configuration constants, or at least use a $config[] array so that any configuration variables are clearly marked when used during runtime in a script.
* That said, this file should only define constants and config variables.  It should not be performing any tasks.  The act of including things like browserdetect should be done by an init script.  This allows us to include site configuration information without having to run anything (something we could use in the RDF generators for PFS, VersionCheck).


====/core/dbconfig.php====
====/core/dbconfig.php====
Line 324: Line 336:
* This file should not create the connection.  Connections should only be created when they will be used.  As it stands, this forces every page on UMO to create a DB connection whether or not it will be used for anything.
* This file should not create the connection.  Connections should only be created when they will be used.  As it stands, this forces every page on UMO to create a DB connection whether or not it will be used for anything.
* Someone could use a script, point at this file and create however many unused DB connections they wanted by simply opening it.
* Someone could use a script, point at this file and create however many unused DB connections they wanted by simply opening it.
* The proper way to handle db connections would be to explicitly connect to the db in the script that is doing a query.  It reduces confusion and also allows for scripts that don't connect just because they include the config/init scripts.


====/core/inc_browserdetection.php====
====/core/inc_browserdetection.php====
Line 505: Line 518:


===/extensions===
===/extensions===
* These files are duplicated in /themes.  They should really be abstracted and moved up one directory.  Otherwise, any changes happening in /themes will have to be re-implemented in /extensions and vice-versa.
* These files should all be migrated to reduce code:
** /extensions/authorprofiles.php -> ../authorprofiles.php
** /extensions/inc_sidebar.php -> ../core/[something]
** /extensions/index.php -> ../extensions.php
** /extensions/moreinfo.php -> ../moreinfo.php
** /extensions/showlist.php -> ../showlist.php
====/extensions/authorprofiles.php====
====/extensions/authorprofiles.php====
* [98] should htmlentities($userwebsite) for the href property
* [98] should htmlentities($userwebsite) for the href property
Line 535: Line 556:


====/plugins/index.php====
====/plugins/index.php====
* Would be nice to move this information into a database.


====/plugins/download.php====
====/plugins/download.php====
* [61-78] PHP is not for serving files.  Use a redirect that sends them to a URL instead.
* [61-78] PHP is not for serving files.  Is there a better way to get the user to their file?


====/plugins/PluginFinderService.php====
====/plugins/PluginFinderService.php====
Line 547: Line 569:


====/plugins/uniqueUrl.php====
====/plugins/uniqueUrl.php====
* Can't change this.
* Can't touch this.  Dun dun dun dun dun dun dun dun.


----
----
Line 569: Line 591:
===/update===
===/update===
====/update/VersionCheck.php====
====/update/VersionCheck.php====
* Versions should not be mathematically illogical.  i.e. 2.10 should not be a higher version than 2.2! ... :| (alanjstr says: it is in the Gnu and Mozilla World.  Take a look at the bugzilla product)
* [42] DB connection should not happen until after all inputs have been determined and validated
* [42] DB connection should not happen until after all inputs have been determined and validated
* [49] Header sent before contents are determined
* [49] Header sent before contents are determined
canmove, Confirmed users, Bureaucrats and Sysops emeriti
1,043

edits