1

Am using the below NODE js code to run a timeclock. For one specific user, and seemingly this is a problem that's only reared its head today, this particular user (OPID 7) is being told he's worked minus time, whereas other users are shown their correct hours worked in positive time? The user's opid is 7

The tabular data is here, but I'm baffled as I can't see any difference betwene OPID 7 and OPID 3 or 5...

firebird.attach(options, function(err, db) {
    if(err) throw err;
    
    app.get('/', (req, res) => {
        db.query('SELECT opid, name FROM TXOPS WHERE OPID NOT IN (1, -1, 2, 9, 10, 13, 14)', function(err, rows) {
          if(err) throw err;
  
          res.render('index', { operators: rows });
        });
      })
  
      app.get('/clock/:id', (req, res) => {
          if(req.params.id) {
              db.query('SELECT opid, name FROM TXOPS WHERE OPID = ' + req.params.id, function(err, operator_row = []) {
                  if(operator_row.length === 1)
                  {
                      var clockedIn = false;
                      db.query('SELECT FIRST 1 * FROM TXATTENDETXNTS WHERE OPID = ' + req.params.id + ' AND DATETIME >= \'' + moment().startOf('day').format('YYYY-MM-DD HH:mm:ss') + '\' ORDER BY DATETIME DESC', function(err, last_row) {
                          console.log(last_row);
              if(err) throw err;
                          if(last_row.length === 1 && last_row[0].ETXNTTYPE === 0) clockedIn = true;
                      
                          if(clockedIn)
                          {
                              db.query('SELECT FIRST 10 * FROM TXATTENDETXNTS WHERE OPID = ' + req.params.id + ' AND DATETIME >= \'' + moment().startOf('day').format('YYYY-MM-DD HH:mm:ss') + '\'', function(err, today_rows) {
                                  totalMinutes = 0;
                console.log(today_rows);
                                  today_rows.push({ DATETIME: moment().format('YYYY-MM-DD HH:mm:ss')});
                                  splitRows = today_rows.reduce(function(result, value, index, array) {
                                      if (index % 2 === 0)
                                        result.push(array.slice(index, index + 2));
                                      return result;
                                  }, []);
  
                                  splitRows.forEach(pair => {
                                      totalMinutes += Math.round(moment.duration(moment(pair[1].DATETIME).diff(moment(pair[0].DATETIME))).asMinutes());
                                  });
  
                                  console.log(totalMinutes);
  
                                  minuteString = null;
                                  hours = 0;
                                  remainingMinutes = 0;
                                  if(totalMinutes > 60)
                                  {
                                      hours = Math.floor(totalMinutes / 60);
                                      remainingMinutes = totalMinutes - (hours * 60);
                                  }
  
                                  if(hours > 0 && remainingMinutes > 0)
                                  {
                                      minuteString = hours + ' hours and ' + remainingMinutes + ' minutes'
                                  }
                                  else if(hours > 0 && remainingMinutes === 0)
                                  {
                                      minuteString = hours + ' hours';
                                  }
                                  else {
                                      minuteString = totalMinutes + ' minutes';
                                  }
  
                                  res.render('clock', { operator: operator_row[0], working_string: minuteString, clockedIn: clockedIn });
  
                              });
                          }
                          else
                          {
                              res.render('clock', {operator: operator_row[0], clockedIn: clockedIn})
                          }
                      });
  
                  }
              });
          }
      })
  
      app.get('/clock-in/:id', (req, res) => {
          db.query('SELECT opid, name FROM TXOPS WHERE OPID = ' + req.params.id, function(err, rows) {
  
              if(rows.length === 0) 
              {
                  res.render('error');
              }
  
              // Check if he's already clocked in
              db.query('SELECT FIRST 1 OPID FROM TXATTENDETXNTS WHERE OPID = ' + req.params.id + ' ORDER BY ATTENDETXNTID DESC', function(err, rows) {
                  if(err) throw err;
  
                  if(rows.length === 1 && rows[0].ETXNTTYPE == 0)
                  {
                      res.render('error');
                  }
                  else
                  {
                      db.query('SELECT FIRST 1 ATTENDETXNTID FROM TXATTENDETXNTS ORDER BY ATTENDETXNTID DESC', function(err, rows) {
                          lastID = 0;
                          if(rows.length === 1)
                          {
                              lastID = rows[0].ATTENDETXNTID;
                          }
  
                          db.query('INSERT INTO TXATTENDETXNTS (ATTENDETXNTID, OPID, DATETIME, ETXNTTYPE, SCANNEDSTR, BRANCHID, CHANGED_FLAG, DEPTID) VALUES (' + (lastID + 1) + ','+ req.params.id +', \'' + moment().format('YYYY-MM-DD HH:mm:ss') + '\', 0, null, 1, 1, 0)', function(err, rows) {
                              if(err) throw err;
  
                              res.render('success');
                                                            client.publish('timeclock'+ req.params.id, 'clocked in');
                                                            
                          });
                      });
                  }
              });
          });
        })
  
        app.get('/clock-out/:id', (req, res) => {
          db.query('SELECT opid, name FROM TXOPS WHERE OPID = ' + req.params.id, function(err, rows) {
  
              if(rows.length === 0) 
              {
                  res.render('error');
              }
  
              // Check if he's already clocked out
              db.query('SELECT FIRST 1 OPID FROM TXATTENDETXNTS WHERE OPID = ' + req.params.id + ' ORDER BY ATTENDETXNTID DESC', function(err, rows) {
                  if(err) throw err;
  
                  if(rows.length === 1 && rows[0].ETXNTTYPE == 1)
                  {
                      res.render('error');
                  }
                  else
                  {
                      db.query('SELECT FIRST 1 ATTENDETXNTID FROM TXATTENDETXNTS ORDER BY ATTENDETXNTID DESC', function(err, rows) {
                          lastID = 0;
                          if(rows.length === 1)
                          {
                              lastID = rows[0].ATTENDETXNTID;
                          }
  
                          db.query('INSERT INTO TXATTENDETXNTS (ATTENDETXNTID, OPID, DATETIME, ETXNTTYPE, SCANNEDSTR, BRANCHID, CHANGED_FLAG, DEPTID) VALUES (' + (lastID + 1) + ','+ req.params.id +', \''+ moment().format('YYYY-MM-DD HH:mm:ss') +'\', 1, null, 1, 1, 0)', function(err, rows) {
                              if(err) throw err;
  
                              res.render('success');
                                                            client.publish('timeclock'+ req.params.id, 'clocked out');
                          });
                      });
                  }
              });
          });
        })
        app.get('/break-out/:id', (req, res) => {
          db.query('SELECT opid, name FROM TXOPS WHERE OPID = ' + req.params.id, function(err, rows) {
  
              if(rows.length === 0) 
              {
                  res.render('error');
              }
  
              // Check if he's already clocked out
              db.query('SELECT FIRST 1 OPID FROM TXATTENDETXNTS WHERE OPID = ' + req.params.id + ' ORDER BY ATTENDETXNTID DESC', function(err, rows) {
                  if(err) throw err;
  
                  if(rows.length === 1 && rows[0].ETXNTTYPE == 1)
                  {
                      res.render('error');
                  }
                  else
                  {
                      db.query('SELECT FIRST 1 ATTENDETXNTID FROM TXATTENDETXNTS ORDER BY ATTENDETXNTID DESC', function(err, rows) {
                          lastID = 0;
                          if(rows.length === 1)
                          {
                              lastID = rows[0].ATTENDETXNTID;
                          }
  
                          db.query('INSERT INTO TXATTENDETXNTS (ATTENDETXNTID, OPID, DATETIME, ETXNTTYPE, SCANNEDSTR, BRANCHID, CHANGED_FLAG, DEPTID) VALUES (' + (lastID + 1) + ','+ req.params.id +', \''+ moment().format('YYYY-MM-DD HH:mm:ss') +'\', 1, null, 1, 1, 0)', function(err, rows) {
                              if(err) throw err;
  
                              res.render('success');
                                                            client.publish('timeclock'+ req.params.id, 'broke out');
                          });
                      });
                  }
              });
          });
        })              
})
daneee
  • 153
  • 8
  • I'm getting the feeling that somewhere you are mixing up timezones so you end up with a time that's in the future. – CherryDT Sep 09 '21 at 16:19
  • 1
    By the way: SQL injection vulnerability right here: `db.query('SELECT opid, name FROM TXOPS WHERE OPID = ' + req.params.id, ...)` - what if I go to URL `/break-out/0%20OR%201=1`? – CherryDT Sep 09 '21 at 16:21
  • @CherryDT Not just there, the SQL injection vulnerabilities are all over the code shown. – Mark Rotteveel Sep 09 '21 at 16:58
  • Your pastebin link doesn't work for me. Please include all relevant information in the question itself. Make sure your code is a [mre]. Currently it seems to have far more code than necessary. That said, the primary problem seems to be that you don't specify an `order by` when selecting data. Which means data can be returned in arbitrary order, while you seem to expect older rows are always before newer rows. – Mark Rotteveel Sep 09 '21 at 17:02
  • Thanks guys. I’m not worried about sql injection because this is a local LAN thing and not exposed to the internet. There’s trust and a small team. I don’t think it’s a time zone thing because it’s only this one employee affected, more likely to be the sort order someone mentioned maybe? Or something like that? Sorry for providing too much information. In a previous related thread I was taken to task for the opposite problem and that helper was much mollified to see the entire code. Finally re: pastebin, it did the same thing to me but it turned out it was my adblocker! Thanks all! – daneee Sep 09 '21 at 17:52
  • Yes, it is hard to strike the balance between how much code to provide, but in this case I'd recommend to start with 1) example DDL + inserts to create the necessary data and 2) the code that produces the faulty result. That way the code doesn't include all kinds of other potentially distracting things. Have you tried adding an `ORDER BY` as I suggested? – Mark Rotteveel Sep 10 '21 at 06:44
  • Regarding SQL injection not being important in a local network environment: I disagree. This is not only about trust, but also about training yourself to write secure code. A habit like this may easily slip into external accessible applications, and applications written for internal use may get exposed to the internet at a later time (intentionally or otherwise). Not to mention that a bad actor gaining access to your network in a different way, may use such internal vulnerabilities to gain further access or data. – Mark Rotteveel Sep 10 '21 at 06:47
  • Hi Mark, YES, I THINK I've got it sorted by adding + '\' ORDER BY DATETIME DESC' to today_rows. I'm just battling it a t the moment as I am trying to get a situation whereby it can cope with someone remaining clocked in from the day before - at the moment I'm going round and round in circles, but I'm making some slow progress. Thanks – daneee Sep 10 '21 at 08:15
  • Good to hear. If you manage to tackle the issue, consider posting an answer with your solution. – Mark Rotteveel Sep 10 '21 at 08:50
  • `sql injection because this is a local LAN thing` - two things about that: every project is started with a small team on full trust, but given some days or years it tends to deteriorate, another is the faality of data types: what is data in text format? what is string with al the special characters? SQL parameters save you a lot of hassle with ocasional and unexpected SQL syntax errors, or yet worse, silent errors. – Arioch 'The Sep 12 '21 at 12:00

1 Answers1

0

Rows in a database table do not have a specific order, their location depends on order of inserts, free space available on a datapage, which pages are locked at the time of insert, and other factors. In addition, the result of a query can be influenced by things like the query plan (including index use) and I/O considerations.

Your code - specifically with the SELECT FIRST 10 * FROM TXATTENDETXNTS ... query - assumes that older rows are returned before newer rows. However, this is not guaranteed, so when a newer row appears before an older row, you'll get negative values. You need to explicitly specify an ORDER BY (e.g. ORDER BY DATETIME) when selecting rows for your calculation.

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197