User:Cpeel/Remove magic quotes

From DPWiki
Jump to navigation Jump to search

This was completed, merged, and deployed to PROD in Nov 2016.

To get us off PHP 5.3, we have to remove our dependence on magic quotes. To do this, we need to audit files that use $_POST/$_GET/$_REQUEST/$_COOKIE and also have mysql_query() calls to ensure that the variables passed into the SQL used in mysql_query() are being correctly escaped.

General approach

  1. We will incrementally address each script in question, and ensure (either by inspection or conversion) that it is "magic-quotes-agnostic". That is, it should exhibit the same (correct) behavior regardless of the setting of magic_quotes.
  2. Over the course of this effort, we will occasionally install the latest changes to the test server and get people to test them.
  3. Once we believe we have made the code-base entirely magic-quotes-agnostic, we can disable magic quotes on the test server for further testing.
  4. If that goes okay, we can install the code to production.
  5. And if that goes okay, we can disable magic_quotes.

What needs to be done

For the .php files below:

  1. Add undo_all_magic_quotes() at the top of the file right after the includes.
  2. Audit all mysql_query() or dpsql_query() calls and ensure that arguments included in the SQL are escaped with mysql_real_escape_string().
    • Not all variables must be escaped if they are otherwise validated from user input (like projectid) or can't contain single quotes.
  3. Audit the use of addslashes(). If present, determine if it should be using mysql_real_escape_string() and change it.
  4. Audit the use of stripslashes(). undo_all_magic_quotes() will strip the slashes automatically, so the stripslashes() call against $_POST/$_GET/$_REQUEST/$_COOKIE variables is not necessary.

For the .inc files below:

  1. Audit all mysql_query() or dpsql_query() calls and ensure that arguments included in the SQL are escaped with mysql_real_escape_string().
    • Not all variables must be escaped if they are otherwise validated from user input (like projectid) or can't contain single quotes.

What we're not doing

We're not moving the DP codebase to a new DB abstraction layer. That work is good and necessary, but I think getting the current model working with correctly-escaped strings is an easier approach. I'd like for us to move to a more object-centric model where we're simply doing much less SQL all over the code to begin with, so I'd rather focus our efforts first on that rather than moving to a DB abstraction layer. Then when we move to a DB abstraction layer we have much less code to move to begin with.

How to help

All known code changes have been done. Assisting with testing is the best way to help.

Testing

(Please note: you can test in any area you'd like even if someone else has signed up for it -- the more eyes the better!)

An up-to-date copy of the code with all magic quotes disabled can be found on the TEST server at: http://www.pgdp.org/c.remove_magic_quotes

We need to validate that the site continues to function normally. This includes all aspects of proofing (including WordCheck), project managing (including creating and editing projects and managing word lists), smoothreading, etc.

The primary thing to watch out for are form fields that can take quotes (single and double) and backslash characters. Try adding these characters to forms and submit them. The behavior of the c.remove_magic_quotes code should match that of the main code on the TEST server.

Exception: The only exception is tasks.php. The current tasks.php file stores data in the database double-escaped. The new one does not. There is a transition script to fix the database files but it should only be run on TEST after the remove_magic_quotes code is checked in. This means that using tasks.php inside c.remove_magic_quotes will likely show already-escaped characters for existing data, but creating new data or editing existing data should function normally.

If you find problems or have questions, please use the #magic-quotes Slack channel to let cpeel know about it or send him a PM.

Test plan

The following are areas we want to test:

  • Accounts
    • Signing in - jjz
    • Registering & Activating new account - jjz
  • Overall site navigation
    • Project search - jjz
    • Round pages
      • Using filters with single-quotes in the values - srjfoo: selecting a special day that contains an apostrophe works on all round pages, plus SR/PP/PPV.
  • Statistics
    • Searching for members - jjz
    • Searching for teams
      • Adding a team - wfarrell
      • Editing a team - wfarrell
  • Preferences
    • General - srjfoo
    • Proofreading - srjfoo
  • Proofreading
    • Standard Interface - jjz
    • Enhanced Interface - WebRover
    • Wordcheck - WebRover
  • Project Management
    • Creating project - srjfoo
    • Editing project information - srjfoo
    • Editing word lists - fvandrog
    • Handling bad page
    • Remote File Manager - srjfoo: renaming, moving files, loading files into a project (which is technically a function of the project page) all work. Deleting files and creating subfolders also work.
  • Smooothreading
    • Download a file - WebRover
    • Add comments to file, with and without curly quotes and upload - WebRover
    • Make project available for SR - srjfoo
  • PP and PPV
    • Check out for PP and upload for PPV - fvandrog
    • Check out for PPV, generate PPV report - fvandrog
    • Change project state - fvandrog
  • Quizes - ThePiedTyper
  • Site admin
  • Misc
  • Please add other things to this list if you think of them or run across them!

Testing results

If you've done some testing on http://www.pgdp.org/c.remove_magic_quotes please append your name to the #Test plan above and optionally note below your experience. This will help us get coverage of what has and hasn't been tested.

  • Project creation, success: Created two projects, one in the rmq sandbox and a control on the general test server. Included some combination of double and single quotes, and backslashes in Title, Author, Extra Credits and Project Comments. No apparent difference, no problems noticed. Sharon (talk) 17:53, 14 March 2016 (EDT)
  • Edit good and bad word lists. No problems.Frank
  • Check out project for PP and upload for verification. No problems.Frank
  • Check out project for PPV, generate PPV report and send project back to PP. No problems.Frank
  • Send project back to PP queue from PP. No problem. Frank
  • Add special Day -- I've added Tardis Tuesday -- and it worked OK. (lhamilton)
  • Changed Prefs (WebRover)
  • Completed all Proofreading Quizzes -- no problems (ThePiedTyper)
 General--Changed pref for Credit to username--successful
 Proofreading Preferences
   Changed to Enhanced Interface
   Changed Interface Layout to horizontal  
   Changed Font Face to DPCustomMono2; Image Zoom to 110%; Text frame size to 50%; Length of Text Lines to 90--Saved Preferences using 'Save Preferences'. 
   Created a new Profile called Wide90, saved the preferences to Wide90 using 'Save Preferences and Quit'
  • Proofing using Enhanced Interface (WebRover)
 Selected project from P1
 Linked to Forum Discussion; added 2 comments, second with curly quotes.
 Clicked 'Start Proofreading from Project Page
   Made Edits
   Used Wordcheck, with no changes (Submit Corrections)
   Saved as Done
   Used Wordcheck, with no changes (Save as Done and Proof Next Page using Wordcheck)
   Added Curly Quotes to a page and successfully saved
   Returned page to round without saving
   Opened previously saved page and returned to round.
   Within proofing interface changed from horizontal layout to vertical and back again
   Refreshed the image
   Reverted to original document
   Made a change, Reverted to original document, Reverted to last save; change reappeared.        
     (Did this again in production and confirmed this is the same behavior.)
   Used Big Eye (Show all Text)
   Clicked on Project Comments
   Saved as In Progress, Refreshed and looked at Page Details--Page marked P1.Page_Temp
   Clicked Save as Done and Proofread next Page (with Icon, not wordcheck)
   Proofed page and saved as done
   From Page details clicked on Temp page, saved as done
   Clicked [** ][]{}
   Selected a word and Clicked ABC
   Selected a word and Clicked Abc
   Selected a word with capitalization and clicked abc
   Opened the Greek Transliterator and clicked on everything. Copied it; 
     clicked [Greek: ] and pasted the text 
   Clicked Guidelines
   Clicked Proofing Diagram
   Saved
   Clicked Blank Page, Reverted to original text
   (All behaved as expected. More to come)
  • Added and edited authors and biographies with strange characters; linked Project Comments to them. No more surprises. (ArleneJoyce)
  • Modified project properties and good word/bad word lists with strange characters (especially slash, backslash, single and double quotes) in all text fields. No surprises. (ArleneJoyce)
Retrieved from "https://www.pgdp.net/w/index.php?title=User:Cpeel/Remove_magic_quotes&oldid=99235"