2

I have problem inserting sql results into TStringGrid.I have following code:

 var i:Integer;
 begin
   SqlQuery1.SQL.Text:= 'SELECT * FROM `users`'; 
   SqlQuery1.Open;
   MySql55Connection1.Open;
   i:= 0;
   while not SQLQUERY1.EOF do
   begin
     i:= i+1;
     StringGrid1.Cells[0,i]:= SqlQuery1.FieldByName('Username').AsString;
     StringGrid1.Cells[1,i]:= SqlQuery1.FieldByName('Password').AsString;
     StringGrid1.Cells[2,i]:= SqlQuery1.FieldByName('id').AsString;
   end;
 end;

So in my database only one line. But program adding a lot of copies of this line in StringGrid and it causes error(Index out of bounds).

Improve this question
2
  • 3
    Try SQLQUERY1.next inside the loop top get the next record. If there are no next record, EOF is reached. Commented Apr 12, 2016 at 15:21
  • 3
    Why not just using TDBGrid? Commented Apr 12, 2016 at 16:11

1 Answer 1

Reset to default
4

Danger
It appears you are storing passwords in plain text form in a database.
This is an extremely bad idea.
Never store passwords in a database.
Use salted hashes instead.
See: How do I hash a string with Delphi?

There are a couple of other problems in your code:

  • You don't ensure that the stringgrid has enough rows to hold your data.
  • You're not moving to the next line in the query.
  • You're opening the query before the connection is open.
  • You're using FieldByName inside a loop, this is going to be very slow.

Simple solution
Use a DBGrid.

If you insist on using a StringGrid
I suggest refactoring the code like so: 

 var 
   i,a:Integer;
   FUsername, FPasswordHash, Fid, FSalt: TField;
 begin
   if not(MySQl55Connection.Active) then MySql55Connection1.Open;
   SqlQuery1.SQL.Text:= 'SELECT * FROM users';  //only use backticks on reserved words.
   SqlQuery1.Open;
   FUsername:= SqlQuery1.FieldByName('Username');
   //do not use plain text passwords!!
   FPasswordHash:= SQLQuery1.FieldByName('SaltedPasswordHashUsingSHA256');
   FId:= SqlQuery1.FieldByName('id');
   FSalt:= SQLQuery1.FieldByName('SaltUsingCryptoRandomFunction');
   a:= StringGrid1.FixedRowCount;
   if SQLQuery1.RecordCount = -1 then StringGrid1.RowCount = 100 //set it to something reasonable.  
   else StringGrid1.RowCount:= a + SQLQuery1.RecordCount;
   //SQLQuery1.DisableControls 
   try
     i:= StringGrid1.FixedRowCount;
     while not(SQLQuery1.EOF) do begin
       if i >= StringGrid1.RowCount then StringGrid1.RowCount:= i;
       StringGrid1.Cells[0,i]:= FUserName.AsString;
       StringGrid1.Cells[1,i]:= FPasswordHash.AsString;
       StringGrid1,Cells[3,i]:= FSaltInHex.AsString;
       StringGrid1.Cells[2,i]:= FId.AsString;
       SQLQuery1.Next;  //get next row.
       Inc(i);
     end; {while}
   finally
     //just in case you want to do endupdate or close the SQLQuery or do SQLQuery1.EnableControls 
   end;
 end;

Basic security example
Here's how to hash a password:

Download Lockbox3.
Put a THash on your form and set the hash property to SHA-512.
Use the following code to produce a hash result. 

function StringToHex(const input: string): AnsiString;
var
  NumBytes, i: Integer;
  B: Byte;
  W: word;
  Wa: array[0..1] of byte absolute W;
begin
  NumBytes := input.length * SizeOf(Char);
  SetLength(Result, NumBytes * 2);
  for i := 1 to NumBytes do begin
    if SizeOf(Char) = 1 then begin
      B:= Byte(input[i]);
      BinToHex(@B, @Result[(I*2)+1], 1);
    end else begin
      W:= Word(input[i]);
      BinToHex(@Wa[0], @Result[(i*4+0)],1);
      BinToHex(@Wa[1], @Result[(i*4+1)],1);
    end; {else}  
  end;
end;

function TForm1.HashPassword(var Password: string; const Salt: string): string;
var
  KillPassword: pbyte;
begin
  Hash1.HashString(StringToHex(Password)+StringToHex(Salt));
  KillPassword:= PByte(@Password[1]);
  FillChar(KillPassword^, Length(Password)*SizeOf(Char), #0); //remove password from memory.  
  Password:= ''; //Now free password.
end;

function GenerateSalt( ByteCount: integer = 32): string;
var
  Buffer: TMemoryStream;
begin
  Buffer := TMemoryStream.Create;
  try
    Buffer.Size := ByteCount;
    RandomFillStream( Buffer);
    result := Stream_to_Base64( Buffer);
  finally
    Buffer.Free
  end;
end;

This is the minimum amount of work you can get away with whilst still having things secure.
Do not think that your passwords are unimportant because you just have a toy database, because people reuse passwords and thus your toy passwords end up being the same passwords used for online banking and such.
People are lazy....

Improve this answer
Sign up to request clarification or add additional context in comments.

19 Comments

you were faster! Well then two things to add: 1: RecordCount may just return -1for SQL queries. Then you would have to calculate it by an extra loop. 2: you should reduce screen flicker slow-down by calling StringGrid.Rows.BeginUpdate and freepascal.org/docs-html/fcl/db/tdataset.disablecontrols.html in try-finally block
@Arioch'The, the for loop will not execute when recordcount = -1.
@Johan but it SHOULD execute, because it is NORMAL to return -1 for SQL queries no matter how many rows there really would be. RecordCount is ONLY required when dealing with local ISAM databases like DBF and Paradox, never required when dealing with remote SQL. PS didn't we already discussed it few months ago ?
you should use SELECT COUNT(*) because you can not trust SQLQuery1.RecordCount
@moskito-x no, that would be wrong too. After he would call COUNT and before he would fill the grid in another SELECT someone else can add more rows into the table. You should call dataset.last then WITHOUT CLOSING IT iterate back to dataset.BOF counting rows, then WITHOUT CLOSING IT set the grid row count and fill it iterating forward until EOF. That all to be done with suppressing screen flickering
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.