Code Review Assignment for Class SE604, 2017. This is an opensource project for Online Library Management System, we updated/added some changes to use in our code review assignment.
After compeleting the assignment, student will:
- Gain knowledge about code review process.
- Gain knowlege about secure code review.
- Gain knowlege about software vulnerabilities and how to detect them.
You have to read some tutorials before starting in the assignment.
- Learn how to build PHP applications PHP Crash Course
- Learn coding standards for PHP PHP Coding Standards
- Download and Setup XAMPP v5.6.14
- Download the project or clone it using GIT, you can check this link to know How to clone a repository?
- Put the project in
C:\xampp\htdocs
folder on your machine to run it onlocalhost
according to the Crash Course in the Readings - From XAMPP control panel, click start for Apache
- Open http://localhost:{default-port}/SE604-Library_System/
- Take a journey in the application(Signup, Login, Search for Book, ... etc.)
- Submit some snapshots
- The code is easy to understand
- Follows coding conventions
- Names are simple and if possible short
- There are no usages of magic numbers
- No hard coded constants that could possibly change in the future
- All variables are in the smallest scope possible
- There is no commented out code
- There is no dead code (inaccessible at Runtime)
- No code that can be replaced with library functions
- Variables are not accidentally used with null values
- Variables are immutable where possible
- Code is not repeated or duplicated
- There is an else block for every if clause even if it is empty
- No complex/long boolean expressions
- No negatively named boolean variables
- No empty blocks of code
- Ideal data structures are used
- Constructors do not accept null/none values
- Catch clauses are fine grained and catch specific exceptions
- Exceptions are not eaten if caught, unless explicitly documented otherwise
- Files/Sockets and other resources are properly closed even when an exception occurs in using them
-
null
is not returned from any method - == operator and === (and its inverse !==) are not mixed up
- Floating point numbers are not compared for equality
- Loops have a set length and correct termination conditions
- Blocks of code inside loops are as small as possible
- No methods with boolean parameters
- Methods return early without compromising code readability
- Performance is considered
- Loop iteration and off by one are taken care of
- Law of Demeter is not violated
- A class/file should has only a single responsibility
- Classes, modules, functions, etc. should be open for extension, but closed for modification
- Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program
- Many client-specific interfaces are better than one general-purpose interface.
- Depend upon Abstractions. Do not depend upon concretions.
- Comments should indicate WHY rather that WHAT the code is doing
- All methods are commented in clear language
- Comments exist and describe rationale or reasons for decisions in code
- All public methods/interfaces/contracts are commented describing usage
- All edge cases are described in comments
- All unusual behaviour or edge case handling is commented
- No vulnerable code that can be exploited as SQL Injection
- No vulnerable code that can be exploited as Cross Site Scripting(XSS)
- Sensitive Data are encrypted before store it in the Database
- No use of Hard-coded Credentials
- No missing Authentication for Critical Function
- Cookies data are encrypted
2. Your should review each file according to the review checklist to check if there are any parts of the file that violated the review checklist.
- Complete - No information is missing
- Clear - Every sentence's meaning must be clear
- Consistent – The writing style and notation is consistent throughout the report