-2

So I've been doing an Employees Attendance Management System using vb.net using a windows form and a mySQL for my database. I successfully inserted an employee into my database but can't seem to check his time in into my database. I have two tables in my database (tbl_attendance and tbl_employee) I've successfully inserted data in my employee table but not in my attendance table. I could really use a fresh new eye since I've been looking for my error for a very long time now and broke down and now I'm here LOL. Here are my codes:

 Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

        Try
            If txtEmployeeID.Text = "" Then
                MessageBox.Show("Please enter Employee ID", "Warning", MessageBoxButtons.OK, MessageBoxIcon.Warning)
            Else
                reloadtext("SELECT * FROM tbl_employees WHERE EMPLOYEEID='" & txtEmployeeID.Text & "'")
                If dt.Rows.Count > 0 Then
                    reloadtext("SELECT * FROM tbl_attendance WHERE EMPLOYEEID='" & txtEmployeeID.Text & "' AND LOGDATE='" & lblDate.Text & "' AND AM_STATUS='Time In' AND PM_STATUS='Time Out'")
                    If dt.Rows.Count > 0 Then
                        MessageBox.Show("You already have an attendance for today", "Reminder", MessageBoxButtons.OK, MessageBoxIcon.Information)
                    Else
                        reloadtext("SELECT * FROM tbl_attendance WHERE EMPLOYEEID ='" & txtEmployeeID.Text & "' AND LOGDATE='" & lblDate.Text & "' AND AM_STATUS ='Time In'")
                        If dt.Rows.Count > 0 Then
                            updatelog("UPDATE tbl_attendance SET TIMEOUT='" & TimeOfDay & "', PM_STATUS='Time Out' WHERE EMPLOYEEID='" & txtEmployeeID.Text & "' AND LOGDATE='" & lblDate.Text & "'")
                            MessageBox.Show("Successfully Timed out", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information)
                        Else
                            createlog("INSERT INTO tbl_attendance(EMPLOYEEID,LOGDATE,TIMEIN,AM_STATUS)VALUES('" & txtEmployeeID.Text & "','" & lblDate.Text & "','" & TimeOfDay & "','Time In')")
                            MessageBox.Show("Successfully Timed in", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information)
                        End If
                    End If
                Else
                    MessageBox.Show("Employee ID not found", "Not found", MessageBoxButtons.OK, MessageBoxIcon.Asterisk)
                End If
            End If
        Catch ex As Exception

        End Try

    End Sub

I have a problem on my line 18

[This are my functions and connections.]

Imports MySql.Data.MySqlClient
Module CRUDConnection
    Public result As String
    Public cmd As New MySqlCommand
    Public da As New MySqlDataAdapter
    Public dt As New DataTable
    Public ds As New DataSet
    Public Sub create(ByVal sql As String)
        Try
            conn.Open()
            With cmd
                .Connection = conn
                .CommandText = sql
                result = cmd.ExecuteNonQuery
                If result = 0 Then
                    MessageBox.Show("Data failed to insert.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Warning)
                Else
                    MessageBox.Show("Data successfully inserted.", "Success", MessageBoxButtons.OK, MessageBoxIcon.Information)
                End If
            End With
        Catch ex As Exception
        Finally
            conn.Close()
        End Try
    End Sub
    Public Sub reload(ByVal sql As String, ByVal DTG As Object)
        Try
            conn.Open()
            dt = New DataTable
            With cmd
                .Connection = conn
                .CommandText = sql
            End With
            da.SelectCommand = cmd
            da.Fill(dt)
            DTG.DataSource = dt
        Catch ex As Exception
        Finally
            conn.Close()
            da.Dispose()
        End Try
    End Sub
    Public Sub reloadtext(ByVal sql As String)
        Try
            conn.Open()
            With cmd
                .Connection = conn
                .CommandText = sql
            End With

            dt = New DataTable
            da = New MySqlDataAdapter(sql, conn)
            da.Fill(dt)
        Catch ex As Exception
        Finally
            conn.Close()
            da.Dispose()
        End Try
    End Sub
    Public Sub createlog(ByVal sql As String)
        Try
            conn.Open()
            With cmd
                .Connection = conn
                .CommandText = sql
                result = cmd.ExecuteNonQuery
            End With
        Catch ex As Exception
        Finally
            conn.Close()
        End Try
    End Sub
    Public Sub updatelog(ByVal sql As String)
        Try
            conn.Open()
            With cmd
                .Connection = conn
                .CommandText = sql
                result = cmd.ExecuteNonQuery
            End With
        Catch ex As Exception
        Finally
            conn.Close()
        End Try
    End Sub
End Module

Here's a screenshot of my database

But this one is working

cant seem to insert the values into my tbl_attendance

I don't know what to do anymore. I can't seem to find that one comma. Thanks in advance!

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
Newbster
  • 1
  • 2
  • Welcome to Stack Overflow! Please see [Something in my web site or project doesn't work. Can I just paste a link to it?](https://meta.stackoverflow.com/q/254428/328193) Relevant code needs to be included in the question to produce a [mcve] which demonstrates the problem, as well as information about the problem itself and what debugging you have done. To learn more about this community and how we can help you, please start with the [tour] and read [ask] and its linked resources. – David Feb 24 '23 at 15:51
  • 4
    Having said that... How specifically is the code failing? Why can't you insert a record? At a glance I notice a few things: (1) Your code is *wide open* to **SQL injection**, so currently neither us nor you even know what SQL code you're trying to execute. You should be using parameterized queries instead. (2) All of your SQL interactions are wrapped in a `Try` block with empty `Catch` blocks, which is your way of telling the code that you *don't care if it fails and don't want to be notified of failures*, but now you're asking why it failed? If you want to correct errors, don't ignore them. – David Feb 24 '23 at 15:54
  • 2
    https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/exceptions/exception-handling – Hogan Feb 24 '23 at 15:57
  • 2
    I second the comment about this being prone to SQL Injection. The way you're composing your SQL queries is just plain wrong. Also, we have no idea which line is line 18. – devlin carnate Feb 24 '23 at 15:58
  • The following may be helpful: https://stackoverflow.com/a/68355687/10024425 and https://stackoverflow.com/a/72794540/10024425 (see function "GetDataSqlServer"). Here's one for MySQL, but it's C#: https://stackoverflow.com/a/71540398/10024425. – Tu deschizi eu inchid Feb 24 '23 at 16:20
  • _I can't seem to find that one comma._ Could you please show us the exact error message? It seems that your inputs contain some character that makes your query text invalid. This happens when there is a single quote somewhere in the inputs and you use that input to build the sql commands. This is the most common scenario that happens when you code the sql commands in that way. And is the most harmless of all. Much worse is when someone exploits this system to destroy or modify your database It is called SQL Injection – Steve Feb 24 '23 at 17:55
  • 1
    There are some structural problems with the way the `CRUDConnection` class is designed. There are **serious flaws**, that will force you to lose type safety in your application and to build you SQL in a way that leaves you wide open to injection... the kind of thing where a year down the rode you find out you were hacked six months ago and all your data is for sale on the dark web. This is a BIG DEAL... too important even to do wrong (and what you have is **wrong**) even for learning and proof of concept work. – Joel Coehoorn Feb 24 '23 at 17:55
  • > `"I can't seem to find that one comma."` It's probably in the **data**, because there's nothing in the code to protect you from the data being interpreted as part of the query control. This is why you need to use **parameterized queries**, which is not possible with the design of the `CRUDConnection` class – Joel Coehoorn Feb 24 '23 at 17:58

1 Answers1

0

I can't seem to find that one comma.

It's probably coming from your data. If you have a query like this:

"INSERT INTO NAMES (LastName, FirstName) VALUES ('" + TextBox1.Text + "', '" + TextBox2.Text "')"

And the text boxes have values like this:

O'Brien   Patty

You end up with SQL like this:

INSERT INTO NAMES (LastName, FirstName) VALUES ('O'Brien', 'Patty')

And suddenly the extra ' character from the last name throws the whole query off. The error message will complain about a missing comma after the 'O' literal.

Worse, attackers can take advantage of this kind of issue to do really bad things to your database... effectively run any arbitrary SQL they want. This is the kind of thing where you find out a year later you were hacked six months ago and attackers are holding your data for ransom before selling it on the dark web.

There are several ways to address this, but some of them are BAD: they only push the issue to the next level. Worse, they might seem to work, but still leave you with subtle openings.

The correct solution involves something called parameterized queries. Unfortunately, using them means you need to go back to the drawing board on CRUDConnection module and re-think how that works.

In a nutshell, this module should not expose a public method to run arbitrary SQL. Instead, every query you want to run will get it's own method, with strongly typed function arguments for the inputs.

We end up with an something more like this, to show you what one method might look like

Module DB

    ' Note this is PRIVATE!
    ' Also note we only keep the string, and not the connection
    ' More info on why: https://softwareengineering.stackexchange.com/a/398790/8057
    Private connectionString As String = " connection string here "

    Public Sub ClockIn(EmployeeID As Integer, ClockTime As DateTime)
        Dim SQL As String = "INSERT INTO tbl_attendance(EMPLOYEEID,LOGDATE,TIMEIN,AM_STATUS)VALUES(@EmployeeID,@LogDate,@TimeOfDay,'Time In')"  

        Using cn As New MySqlConnection(connectionString), 
              cmd As New MySqlCommand(SQL, cn)
 
           cmd.Parameters.AddWithValue("@EmployeeID", EmployeeID)
           cmd.Parameters.AddWithValue("@LogDate", ClockTime.Date)
           cmd.Parameters.AddWithValue("@TimeOfDay", ClockTime.ToString("HH:mm:ss"))
           cn.Open()
           cmd.ExecuteNonQuery()         
           
        End Using
    End Sub


    ' Note this is also PRIVATE!
    ' It exists to make it easier to write the other methods in the module
    Private Function GetRecords(SQL As String, addParams As Func(Of MySqlParameterCollection)) As DataTable
        Dim result As New DataTable
        Using cn As New MySqlConnection(connectionString), _
           da As New MySqlDataAdapter(SQL, cn)

           If addParams IsNot Nothing Then
                addParams(da.SelectCommand.Parameters)
           End If
           da.Fill(result)
        End Using
        Return result
    End Function

End Module

Note I was able to use a basic literal for the Time In string in the ClockIn() method, but everything else was a query parameter. You need EVERY QUERY to have it's own method like this.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794