[kratoss1812]

Hello, first of all I want to say that I never used MySQL.
I'm trying to create a plugin that insert into a database all the players that join the server.

My Code:

PHP Code:


Database g_hDB;
char g_sSQL[3096];

public void OnPluginStart()
{
    SQL_TConnect(OnConnect, "onlineplayers");
}

public int OnConnect(Handle owner, Handle hndl, char [] error, any data)
{
    if(hndl == INVALID_HANDLE)
    {
        LogError("Database failure: %s", error);
        SetFailState("Unable to connect to Database");
    }
    else
    {
        Format(g_sSQL, sizeof(g_sSQL), \
        "CREATE TABLE IF NOT EXISTS `onlineplayers` ('name' varchar(128) NOT NULL, 'steamid' varchar(32))");
        SQL_TQuery(g_hDB, OnSQLConnectCallback, g_sSQL);
    }
}

public int OnSQLConnectCallback(Handle owner, Handle hndl, char [] error, any data)
{
    if(hndl == INVALID_HANDLE)
    {
        LogError("Query failure: %s", error);
        return;
    }
    
    for(int Client = 1; Client <= MaxClients; Client++)
    {
        if(IsClientInGame(Client))
        {
            OnClientPostAdminCheck(Client);
        }
    }
}

public void OnClientPostAdminCheck(int iClient)
{
    char query[255], sSteamID[32], Name[32];
    GetClientAuthId(iClient, AuthId_Steam2, sSteamID, sizeof(sSteamID));
    GetClientName(iClient, Name, 32);
    
    Format(query, sizeof(query), "INSERT INTO onlineplayers(name, steamid) VALUES('%s', '%s')", sSteamID, Name);
    SQL_TQuery(g_hDB, Check, query, GetClientUserId(iClient));
}

public int Check(Handle owner, Handle hndl, char [] error, any data)
{
    int client = GetClientOfUserId(data);
    if(client == 0)
    {
        return;
    }
    if(hndl == INVALID_HANDLE)
    {
        LogError("Query failure: %s", error);
        return;
    }
} 


0 erros but I want to know what do you think about it

[Drixevel]

Database g_hDB;

public void OnPluginStart()
{
	Database.Connect(OnConnect, "onlineplayers");
}

public void OnConnect(Database db, const char[] error, any data)
{
	if(db == null)
		SetFailState("Unable to connect to database: %s", error);
	
	g_hDB = db;
	
	char sQuery[256];
	g_hDB.Format(sQuery, sizeof(sQuery), "CREATE TABLE IF NOT EXISTS `onlineplayers` ('name' varchar(128) NOT NULL, 'steamid' varchar(32))");
	g_hDB.Query(OnSQLConnectCallback, sQuery);
}

public void OnSQLConnectCallback(Database db, DBResultSet results, const char[] error, any data)
{
	if (results == null)
	{
		LogError("Query failure: %s", error);
		return;
	}
	
	for (int i = 1; i <= MaxClients; i++)
	{
		if (IsClientInGame(i))
			OnClientPostAdminCheck(i);
	}
}

public void OnClientPostAdminCheck(int client)
{ 
	char sSteamID[64];
	if (!GetClientAuthId(client, AuthId_Steam2, sSteamID, sizeof(sSteamID)))
		return;
	
	char sName[MAX_NAME_LENGTH];
	GetClientName(client, sName, sizeof(sName));
	
	char sQuery[256];
	g_hDB.Format(sQuery, sizeof(sQuery), "INSERT INTO onlineplayers(name, steamid) VALUES('%s', '%s')", sSteamID, sName);
	g_hDB.Query(Check, sQuery);
}

public void Check(Database db, DBResultSet results, const char[] error, any data)
{
	if (results == null)
		LogError("Query failure: %s", error);
}

Granted a lot of what I did can be considered picky but you can compare and see what I changed.

Notes:
- You should check the return value for GetClientAuthId whenever you use it because if steam is down, it'll be sending back a non-steamid which you shouldn't save to your database. (you should also call it inside of OnClientAuthorized but I didn't change that)
- You mix syntax a bit so I updated it.
- You already had a security issue while pulling the clients name on connect and saving it to the database, you need to escape the string. (not sure if using the Format methodmap for SQL does that but if not, use g_hDB.Escape to escape the name string)
- I usually don't set fail state on plugins ever since I like to work around error logs but that's an opinion.
- Tabs to spaces because I like tabs and no indentation errors. <3

[ThatKidWhoGames]

Change:

g_hDB.Format(sQuery, sizeof(sQuery), "INSERT INTO onlineplayers(name, steamid) VALUES('%s', '%s')", sSteamID, sName);

To:

g_hDB.Format(sQuery, sizeof(sQuery), "INSERT INTO onlineplayers(name, steamid) VALUES('%s', '%s')", sName, sSteamID);

[asherkin]

Originally Posted by Drixevel - You should check the return value for GetClientAuthId whenever you use it because if steam is down, it'll be sending back a non-steamid which you shouldn't save to your database. (you should also call it inside of OnClientAuthorized but I didn't change that)

It can't fail after OnClientAuthorized has fired for a client (and OnClientPostAdminCheck requires OnClientAuthorized to have fired). Still best practice to check though.

Originally Posted by Drixevel - You already had a security issue while pulling the clients name on connect and saving it to the database, you need to escape the string. (not sure if using the Format methodmap for SQL does that but if not, use g_hDB.Escape to escape the name string)

Database.Format handles escaping for you, that is its sole reason for existing, so your code is correct.

[kratoss1812]

Thank y'all