//
// frmMain
//
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using Owf.Controls; // Custom header panel from openwinforms.com
using System.Data.SQLite;
namespace FlashCards
{
public partial class frmMain : Form
{
// Current directory - Used to find the database.
private string _currentDirectory = Environment.CurrentDirectory;
// Place to store the currentDatabase info once frmMain is initialized.
private string _currentDatabase;
// Place to store the connectionString info.
private string _connectionString;
private SQLiteConnection _sqLiteConnection;
private SQLiteCommand _sqLiteCommand;
private SQLiteDataAdapter _sqLiteDataAdapter;
private DataSet _dataSet = new DataSet();
private DataTable _dataTable = new DataTable();
private Image _answerOff;
private Image _answerOn;
private bool _answerEnabled = false;
private bool _btnPrevEnabled = false;
private bool _btnNextEnabled = false;
private int _answerIndexStart = 0;
private int _answerIndexEnd = 0;
private int _answerIndexCurrent;
public int AnswerIndexEnd
{
get
{
return _answerIndexEnd;
}
}
public frmMain()
{
InitializeComponent();
_answerOff = new Bitmap(typeof(frmMain), "off.png");
_answerOn = new Bitmap(typeof(frmMain), "on.png");
_currentDatabase = _currentDirectory + @"\FlashCards.db";
_connectionString = @"Data Source=" + _currentDatabase + ";New=True;UTF8Encoding=True;Version=3";
_answerIndexCurrent = _answerIndexStart;
GetRecordCount();
LoadData();
SetRtfText();
StatusLabelUpdate();
btnNextGetStatus();
}
private void newToolStripMenuItem_Click(object sender, EventArgs e)
{
frmNewSet _frmNewSet = new frmNewSet(this);
_frmNewSet.Show();
}
public void LoadData()
{
string sqlCommandText = "SELECT * FROM Cards";
SetConnection();
_sqLiteConnection.Open();
_sqLiteDataAdapter = new SQLiteDataAdapter(sqlCommandText, _sqLiteConnection);
_dataSet.Reset();
_sqLiteDataAdapter.Fill(_dataSet);
_dataTable.Reset();
_dataTable = _dataSet.Tables[0];
_sqLiteConnection.Close();
}
private void SetConnection()
{
_sqLiteConnection = new SQLiteConnection(_connectionString);
}
public void ExecuteQuery(string qryText)
{
SetConnection();
_sqLiteConnection.Open();
_sqLiteCommand = _sqLiteConnection.CreateCommand();
_sqLiteCommand.CommandText = qryText;
_sqLiteCommand.ExecuteNonQuery();
_sqLiteConnection.Close();
}
private void AnswerButtonChangeImage()
{
switch (_answerEnabled)
{
case true:
_answerEnabled = false;
this.btnAnswerShow.Image = _answerOff;
this.rtfBoxAnswer.Visible = false;
break;
case false:
_answerEnabled = true;
this.btnAnswerShow.Image = _answerOn;
this.rtfBoxAnswer.Visible = true;
break;
default:
_answerEnabled = false;
this.btnAnswerShow.Image = _answerOff;
this.rtfBoxAnswer.Visible = false;
break;
}
}
private void btnAnswerShow_Click(object sender, EventArgs e)
{
AnswerButtonChangeImage();
}
public void GetRecordCount()
{
string sqlCommandText = "SELECT COUNT(*) AS Total FROM Cards;";
SetConnection();
_sqLiteConnection.Open();
_sqLiteDataAdapter = new SQLiteDataAdapter(sqlCommandText, _sqLiteConnection);
_dataSet.Reset();
_dataTable.Reset();
_sqLiteDataAdapter.Fill(_dataSet);
_dataTable = _dataSet.Tables[0];
_sqLiteConnection.Close();
_answerIndexEnd = Convert.ToInt32(_dataTable.Rows[_answerIndexStart]["Total"]);
}
private void SetRtfText()
{
if ((_answerIndexCurrent < _answerIndexEnd) && (_answerIndexCurrent >= _answerIndexStart))
{
this.rtfBoxQuestion.Text = _dataTable.Rows[_answerIndexCurrent]["card_question"].ToString();
this.rtfBoxAnswer.Text = _dataTable.Rows[_answerIndexCurrent]["card_answer"].ToString();
}
}
private void btnNextGetStatus()
{
switch (_answerIndexCurrent < (_answerIndexEnd - 1))
{
case true:
this.btnNext.Enabled = true;
break;
case false:
this.btnNext.Enabled = false;
break;
default:
this.btnNext.Enabled = false;
break;
}
}
private void btnPrevGetStatus()
{
switch (_answerIndexCurrent > _answerIndexStart)
{
case true:
this.btnPrev.Enabled = true;
break;
case false:
this.btnPrev.Enabled = false;
break;
default:
this.btnPrev.Enabled = false;
break;
}
}
private void btnPrev_Click(object sender, EventArgs e)
{
_answerIndexCurrent = _answerIndexCurrent - 1;
_answerEnabled = true;
AnswerButtonChangeImage();
SetRtfText();
btnPrevGetStatus();
btnNextGetStatus();
StatusLabelUpdate();
}
private void btnNext_Click(object sender, EventArgs e)
{
_answerIndexCurrent = _answerIndexCurrent + 1;
_answerEnabled = true;
AnswerButtonChangeImage();
SetRtfText();
btnNextGetStatus();
btnPrevGetStatus();
StatusLabelUpdate();
}
public void StatusLabelUpdate()
{
this.tStripLabel1.Text = @"Record # " + (_answerIndexCurrent + 1).ToString();
}
}
}
//
// frmAddNew
//
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using System.Data.SQLite;
namespace FlashCards
{
public partial class frmNewSet : Form
{
private frmMain _frmMain;
private int _answerIndexNext;
private RichTextBox _rtfCurrent;
public frmNewSet(frmMain mainFrm)
{
InitializeComponent();
_frmMain = mainFrm;
_rtfCurrent = new RichTextBox();
_answerIndexNext = _frmMain.AnswerIndexEnd + 1;
}
private void btnCancel_Click(object sender, EventArgs e)
{
this.Close();
}
private void btnSave_Click(object sender, EventArgs e)
{
Add();
rtfNewQuestion.Text = string.Empty;
rtfNewAnswer.Text = string.Empty;
_frmMain.LoadData();
this.Close();
}
private void Add()
{
string txtSQLQuery = "INSERT INTO Cards VALUES (" + _answerIndexNext + ",'" + rtfNewQuestion.Text + "','" + rtfNewAnswer.Text + "')";
_frmMain.ExecuteQuery(txtSQLQuery);
}
private void rtfNewQuestion_Enter(object sender, EventArgs e)
{
_rtfCurrent = this.rtfNewQuestion;
}
private void rtfNewAnswer_Enter(object sender, EventArgs e)
{
_rtfCurrent = this.rtfNewAnswer;
}
//
// This method sends special keys like áéÃóúñ¿ to the richtextbox when entering new card data
//
private void btnSpaKey_Click(object sender, EventArgs e)
{
string _btnSpaKeyPressed = (sender as Button).Text;
_rtfCurrent.Focus();
SendKeys.Send(_btnSpaKeyPressed);
}
}
}
Refactorings
No refactoring yet !
Ants
November 1, 2009, November 01, 2009 09:10, permalink
DANGER! SQL Injection vulnerability in frmNewSet.Add() method. Use parameterized queries instead of building SOL queries straight from the user.
Below is a quick first pass refactoring without attempting to fix the vulnerability, nor fix the failure to load the new questions and answers. Changes are:
- Replace switch statements on bools with if-else statements.
- Coalesce shared logic between true and false handling within if-else statements.
- Replace if statements with ? : operators for setting the button image.
- Replaced btnSpaKey_Click() logic that used SendKeys with setting the RichTextBox.SelectedText.
- Replace frmNewSet.Show() call with frmNewSet.ShowDialog().
- Set DialogResult on frmNewSet.btnCancel.
- Removed frmNewSet.btnCancel_Click() handler.
- Moved call to frmMain.LoadData() from frmNewSet.btnSave_Click() to frmMain.newToolStripMenuItem_Click().
- Added call to btnNextGetStatus() to frmMain.newToolStripMenuItem_Click().
- Made frmMain.LoadData() private again.
- Set frmMain._answerIndexEnd in frmMain.LoadData() by getting the row count.
- Got rid of (mis-used) _sqLitConnetion, _sqLiteCommand, _sqLiteDataAdapter, and _dataTable fields.
- Changed SetRtfText() to grab a reference to the data row directly.
- Renamed SetConnection() to GetConnection().
- Return new instance of SQLiteConnection from GetConnection().
- Change usage of SQLiteConnection's to take advantage of IDisposable and using statement.
- Got rid of GetRecordCount() method.
- Get rid of unused _btnNextEnabled and _btnPrevEnabled.
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using Owf.Controls; // Custom header panel from openwinforms.com
using System.Data.SQLite;
namespace FlashCards
{
public partial class frmMain : Form
{
// Current directory - Used to find the database.
private string _currentDirectory = Environment.CurrentDirectory;
// Place to store the currentDatabase info once frmMain is initialized.
private string _currentDatabase;
// Place to store the connectionString info.
private string _connectionString;
private DataSet _dataSet;
private Image _answerOff;
private Image _answerOn;
private bool _answerEnabled = false;
private int _answerIndexStart = 0;
private int _answerIndexEnd = 0;
private int _answerIndexCurrent;
public int AnswerIndexEnd
{
get
{
return _answerIndexEnd;
}
}
public frmMain()
{
InitializeComponent();
_answerOff = new Bitmap(typeof(frmMain), "off.png");
_answerOn = new Bitmap(typeof(frmMain), "on.png");
_currentDatabase = _currentDirectory + @"\FlashCards.db";
_connectionString = @"Data Source=" + _currentDatabase + ";New=True;UTF8Encoding=True;Version=3";
_answerIndexCurrent = _answerIndexStart;
LoadData();
SetRtfText();
StatusLabelUpdate();
btnNextGetStatus();
}
private void newToolStripMenuItem_Click(object sender, EventArgs e)
{
using(frmNewSet frmNewSet = new frmNewSet(this))
{
if(frmNewSet.ShowDialog() == DialogResult.OK)
{
LoadData();
btnNextGetStatus();
}
}
}
private void LoadData()
{
string sqlCommandText = "SELECT * FROM Cards";
using(SQLiteConnection sqLiteConnection = GetConnection())
{
sqLiteConnection.Open();
SQLiteDataAdapter sqLiteDataAdapter = new SQLiteDataAdapter(sqlCommandText, sqLiteConnection);
_dataSet = new DataSet();
sqLiteDataAdapter.Fill(_dataSet);
_answerIndexEnd = _dataSet.Tables[0].Rows.Count;
}
}
private SQLiteConnection GetConnection()
{
return new SQLiteConnection(_connectionString);
}
public void ExecuteQuery(string qryText)
{
using(SQLiteConnection sqLiteConnection = GetConnection())
{
sqLiteConnection.Open();
SQLiteCommand sqLiteCommand = sqLiteConnection.CreateCommand();
sqLiteCommand.CommandText = qryText;
sqLiteCommand.ExecuteNonQuery();
}
}
private void AnswerButtonChangeImage()
{
_answerEnabled = !_answerEnabled;
this.rtfBoxAnswer.Visible = _answerEnabled;
this.btnAnswerShow.Image = _answerEnabled ? _answerOn : _answerOff;
}
private void btnAnswerShow_Click(object sender, EventArgs e)
{
AnswerButtonChangeImage();
}
private void SetRtfText()
{
if ((_answerIndexCurrent < _answerIndexEnd) && (_answerIndexCurrent >= _answerIndexStart))
{
DataRow dataRow = _dataSet.Tables[0].Rows[_answerIndexCurrent];
this.rtfBoxQuestion.Text = dataRow["card_question"].ToString();
this.rtfBoxAnswer.Text = dataRow["card_answer"].ToString();
}
}
private void btnNextGetStatus()
{
this.btnNext.Enabled = _answerIndexCurrent < (_answerIndexEnd - 1);
}
private void btnPrevGetStatus()
{
this.btnPrev.Enabled = _answerIndexCurrent > _answerIndexStart;
}
private void btnPrev_Click(object sender, EventArgs e)
{
_answerIndexCurrent = _answerIndexCurrent - 1;
_answerEnabled = true;
AnswerButtonChangeImage();
SetRtfText();
btnPrevGetStatus();
btnNextGetStatus();
StatusLabelUpdate();
}
private void btnNext_Click(object sender, EventArgs e)
{
_answerIndexCurrent = _answerIndexCurrent + 1;
_answerEnabled = true;
AnswerButtonChangeImage();
SetRtfText();
btnNextGetStatus();
btnPrevGetStatus();
StatusLabelUpdate();
}
public void StatusLabelUpdate()
{
this.tStripLabel1.Text = @"Record # " + (_answerIndexCurrent + 1).ToString();
}
}
}
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using System.Data.SQLite;
namespace FlashCards
{
public partial class frmNewSet : Form
{
private frmMain _frmMain;
private int _answerIndexNext;
private RichTextBox _rtfCurrent;
public frmNewSet(frmMain mainFrm)
{
InitializeComponent();
btnCancel.DialogResult = DialogResult.Cancel;
_frmMain = mainFrm;
_rtfCurrent = new RichTextBox();
_answerIndexNext = _frmMain.AnswerIndexEnd + 1;
}
private void btnSave_Click(object sender, EventArgs e)
{
Add();
btnSave.DialogResult = DialogResult.OK;
}
private void Add()
{
//
// DANGER! Here be dragons!
// SQL injection vulnerability.
//
string txtSQLQuery = "INSERT INTO Cards VALUES (" + _answerIndexNext + ",'" + rtfNewQuestion.Text + "','" + rtfNewAnswer.Text + "')";
_frmMain.ExecuteQuery(txtSQLQuery);
}
private void rtfNewQuestion_Enter(object sender, EventArgs e)
{
_rtfCurrent = this.rtfNewQuestion;
}
private void rtfNewAnswer_Enter(object sender, EventArgs e)
{
_rtfCurrent = this.rtfNewAnswer;
}
//
// This method sends special keys like áéÃóúñ¿ to the richtextbox when entering new card data
//
private void btnSpaKey_Click(object sender, EventArgs e)
{
_rtfCurrent.SelectedText = (sender as Button).Text;
}
}
}
etherealmonkey.myopenid.com
November 2, 2009, November 02, 2009 01:05, permalink
Thank you for the review! I haven't got time to digest the changes fully at this moment, but I will definitely take another look at the SQL parameter suggestion and follow up.
I'm a newb, so please be nice?
Comments regarding what I am doing very wrong would be most appreciated.
This program works (mostly) the way I think it should. One part not working is the insert of a new set requires restarting the app to see the additions.
TIA