Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions basicalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ QVector<Fixation> BasicAlgorithm::generateFixations() {
return fixations;
}


QVector <Saccade> BasicAlgorithm::generateSaccades() {
return saccades;
}

Fixation BasicAlgorithm::computeFixationEstimate(QVector<Gaze> fixation_gazes) {
Fixation fixation;
std::vector<double> x_pos, y_pos;
Expand Down
1 change: 1 addition & 0 deletions basicalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class BasicAlgorithm: public FixationAlgorithm {


QVector<Fixation> generateFixations() override;
QVector<Saccade> generateSaccades() override;

QString generateFixationSettings() override;

Expand Down
3 changes: 3 additions & 0 deletions controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,9 @@ void Controller::generateFixationData(QVector<QString> tasks, QString algSetting
for(auto session_id : sessions) {
//std::cout << "?" << std::endl;
QVector<Fixation> session_fixations;
QVector<Saccade> session_saccades;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variable session_saccades is declared but its value is never used. This is considered dead code and should be removed to improve code clarity.



QVector<QString> gaze_targets = idb.getGazeTargetsFromSession(session_id);
QString fixation_filter_settings;
for(auto gaze_target : gaze_targets) {
Expand Down
21 changes: 12 additions & 9 deletions database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,18 @@ void Database::insertGaze(QString event_time, QString session_id, QString calibr
}

//Inserts values/data into the appropriate attribute for the saccade and saccade_gaze tables
// void Database::insertSaccade(QString saccade_id, QString fixation_run_id, QString start_time, QString end_time, QString start_fixation, QString end_fixation, QString start_x, QString start_y, QString end_x, QString end_y, QString amplitude, QString peak_velocity, QString average_velocity, QString direction, QString duration){
// QString query=QString("INSERT INTO saccade(saccade_id,fixation_run_id,start_time,end_time,start_fixation,end_fixation,start_x,start_y,end_x,end_y,amplitude,peak_velocity,average_velocity,direction,duration) VALUES(%1,%2,%3,%4,\"%5\",\"%6\",%7,%8,%9,%10,%11,%12,%13,%14,%15);").arg(saccade_id,fixation_run_id,start_time,end_time,start_fixation,end_fixation,start_x,start_y,end_x,end_y,amplitude,peak_velocity,average_velocity,direction,duration);
// sqlite3_exec(db,query.toStdString().c_str(),NULL,0,NULL);
// }

// void Database::insertSaccadeGaze(QString saccade_id, QString event_time) {
// QString query = QString("INSERT INTO saccade_gaze(saccade_id,event_time) VALUES(%1,%2);").arg(saccade_id,event_time);
// sqlite3_exec(db,query.toStdString().c_str(),NULL,0,NULL);
// }
void Database::insertSaccade(QString saccade_id, QString fixation_run_id, QString start_time, QString end_time,
QString start_fixation, QString end_fixation, QString start_x, QString start_y,
QString end_x, QString end_y, QString amplitude, QString peak_velocity,
QString average_velocity, QString direction, QString duration){
QString query=QString("INSERT INTO saccade(saccade_id,fixation_run_id,start_time,end_time,start_fixation,end_fixation,start_x,start_y,end_x,end_y,amplitude,peak_velocity,average_velocity,direction,duration) VALUES(%1,%2,%3,%4,\"%5\",\"%6\",%7,%8,%9,%10,%11,%12,%13,%14,%15);").arg(saccade_id,fixation_run_id,start_time,end_time,start_fixation,end_fixation,start_x,start_y,end_x,end_y,amplitude,peak_velocity,average_velocity,direction,duration);
sqlite3_exec(db,query.toStdString().c_str(),NULL,0,NULL);
}
Comment on lines +189 to +195

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This method of constructing SQL queries by string formatting is vulnerable to SQL injection. An attacker could provide malicious input for parameters like start_fixation to execute arbitrary SQL commands. Please use prepared statements with parameter binding to prevent this. For example, with sqlite3, you would use sqlite3_prepare_v2, sqlite3_bind_*, and sqlite3_step. This vulnerability also exists in insertSaccadeGaze and other insert methods in this file.


void Database::insertSaccadeGaze(QString saccade_id, QString event_time) {
QString query = QString("INSERT INTO saccade_gaze(saccade_id,event_time) VALUES(%1,%2);").arg(saccade_id,event_time);
sqlite3_exec(db,query.toStdString().c_str(),NULL,0,NULL);
}


// The following parameters are unused here and should always be inserted as ""
Expand Down
9 changes: 5 additions & 4 deletions database.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "gaze.h"

#include <iostream>
#include <QDebug>

class Database {
public:
Expand Down Expand Up @@ -56,9 +57,9 @@ class Database {
void insertSession(QString,QString,QString,QString,QString,QString,QString,QString,QString,QString);
void insertWebContext(QString,QString,QString,QString,QString);

//Declarations for inserting values into the attributes for the saccade and saccade_gaze tables
//void insertSaccade(QString,QString,QString,QString,QString,QString,QString,QString,QString,QString,QString,QString,QString,QString,QString);
//void insertSaccadeGaze(QString,QString);
void insertSaccade(QString, QString, QString, QString, QString, QString, QString, QString, QString, QString, QString, QString,QString, QString, QString);

void insertSaccadeGaze(QString, QString);

QVector<QString> getSessions();
QVector<QString> getAllIDEContextIDs();
Expand All @@ -77,9 +78,9 @@ class Database {

QVector<QVector<QString>> runFilterQuery(QString);
void executeLongUpdateQuery(QString);
sqlite3* db;

private:
sqlite3* db;
QString file_path;
bool open = false;
Comment on lines +82 to 86

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The database handle db has been moved from private to public. This breaks encapsulation and is a poor design choice. It allows any part of the application to directly access and manipulate the database, bypassing the Database class's interface. This can lead to maintenance issues and hard-to-track bugs. The db handle should remain a private member.

Suggested change
sqlite3* db;
private:
sqlite3* db;
QString file_path;
bool open = false;
private:
sqlite3* db;
QString file_path;
bool open = false;

};
Expand Down
62 changes: 62 additions & 0 deletions fixation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,65 @@ void Fixation::calculateDatabaseFields() {
void Fixation::print() {
std::cout << fixation_event_time << "," << x << "," << y << "," << target.toUtf8().constData() << "," << source_file_line << "," << source_file_col << "," << token.toUtf8().constData() << "," << syntactic_category.toUtf8().constData() << "," << xpath.toUtf8().constData() << "," << left_pupil_diameter << "," << right_pupil_diameter << "," << duration << std::endl;
}


///////////////////////////////


Saccade::Saccade() {}

void Saccade::calculateDatabaseFields() {
long long start_time = -1, end_time = -1;
int gaze_count = 0;
std::map<QString,int> candidate_targets;
for(auto gaze : gaze_vec) {
if(!gaze.isValid()) { continue; }
if(fixation_event_time == 0 || fixation_event_time > gaze.event_time) {
fixation_event_time = gaze.event_time;
}

++gaze_count;

if(start_time == -1 || start_time > gaze.system_time) {
start_time = gaze.system_time;
}
if(end_time == -1 || end_time < gaze.system_time) {
end_time = gaze.system_time;
}


left_pupil_diameter += isnan(gaze.left_pupil_diameter) || gaze.left_pupil_diameter == -1.0 ? 0 : gaze.left_pupil_diameter;
right_pupil_diameter += isnan(gaze.right_pupil_diameter) || gaze.left_pupil_diameter == -1.0 ? 0 : gaze.right_pupil_diameter;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There appears to be a copy-paste error here. When calculating right_pupil_diameter, the condition checks gaze.left_pupil_diameter == -1.0 instead of gaze.right_pupil_diameter == -1.0. This will lead to incorrect calculations for the right pupil diameter.

Suggested change
right_pupil_diameter += isnan(gaze.right_pupil_diameter) || gaze.left_pupil_diameter == -1.0 ? 0 : gaze.right_pupil_diameter;
right_pupil_diameter += isnan(gaze.right_pupil_diameter) || gaze.right_pupil_diameter == -1.0 ? 0 : gaze.right_pupil_diameter;


QString candidate_key = gaze.gaze_target + "\t";
candidate_key += (gaze.source_file_line == -1 ? QString("") : QString::number(gaze.source_file_line)) + "\t";
candidate_key += (gaze.source_file_col == -1 ? QString("") : QString::number(gaze.source_file_col)) + "\t";
candidate_key += gaze.source_token + "\t";
candidate_key += gaze.source_token_syntatic_context + "\t";
candidate_key += gaze.source_token_xpath + "\t";

if(candidate_targets.count(candidate_key) == 0) { candidate_targets.emplace(candidate_key,1); }
else { ++(candidate_targets.find(candidate_key)->second); }
}
std::pair<QString,int> most_frequent = std::make_pair(QString(""),0);
for(auto candidate = candidate_targets.begin(); candidate != candidate_targets.end(); ++candidate) {
if(most_frequent.first == "" || most_frequent.second < candidate->second) { most_frequent = *candidate; }
}

QStringList fields = most_frequent.first.split("\t");
target = fields[0] == "" ? "" : fields[0];
source_file_line = fields[1] == "" ? -1 : fields[1].toInt();
source_file_col = fields[2] == "" ? -1 : fields[2].toInt();
token = fields[3] == "" ? "" : fields[3];
syntactic_category = fields[4] == "" ? "" : fields[4];
xpath = fields[5] == "" ? "" : fields[5];

left_pupil_diameter = left_pupil_diameter / double(gaze_count);
right_pupil_diameter = right_pupil_diameter / double(gaze_count);
Comment on lines +127 to +128

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If gaze_vec is empty or contains only invalid gazes, gaze_count will be 0. This will cause a division-by-zero error when calculating left_pupil_diameter and right_pupil_diameter. You should add a check to prevent this.

Suggested change
left_pupil_diameter = left_pupil_diameter / double(gaze_count);
right_pupil_diameter = right_pupil_diameter / double(gaze_count);
if (gaze_count > 0) {
left_pupil_diameter = left_pupil_diameter / double(gaze_count);
right_pupil_diameter = right_pupil_diameter / double(gaze_count);
} else {
left_pupil_diameter = 0;
right_pupil_diameter = 0;
}

duration = end_time - start_time;

}

void Saccade::print() {
std::cout << fixation_event_time << "," << x << "," << y << "," << target.toUtf8().constData() << "," << source_file_line << "," << source_file_col << "," << token.toUtf8().constData() << "," << syntactic_category.toUtf8().constData() << "," << xpath.toUtf8().constData() << "," << left_pupil_diameter << "," << right_pupil_diameter << "," << duration << std::endl;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The Saccade::print() method prints member variables x and y, but these are never initialized for a Saccade object. This will result in printing uninitialized (garbage) values. If they are not needed for Saccade, consider removing them from the print statement. If they are needed, they should be calculated, for example as the midpoint of the saccade.

}
21 changes: 21 additions & 0 deletions fixation.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,25 @@ class Fixation {
QString target = "", syntactic_category = "", token = "", xpath = "";
};

class Saccade {
public:
Saccade();

void calculateDatabaseFields();

void print();

// This could maybe be moved to a set if we want to ignore duplicates
std::vector<Gaze> gaze_vec;
double x, y, left_pupil_diameter = 0, right_pupil_diameter = 0;
int source_file_line, source_file_col, duration = 0;
long long fixation_event_time = 0;
QString target = "", syntactic_category = "", token = "", xpath = "";

//issue 58 - I think we add these here?
int start_x, start_y, end_x, end_y;
double amplitude, peak_velocity, avg_velocity;
double direction;
};

#endif // FIXATION_H
6 changes: 6 additions & 0 deletions fixationalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,9 @@
QVector<Fixation>& FixationAlgorithm::getFixations() {
return fixations;
}


QVector<Saccade>& FixationAlgorithm::getSaccades(){
return saccades;
}

9 changes: 9 additions & 0 deletions fixationalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,20 @@ class FixationAlgorithm

QVector<Fixation>& getFixations();

//issue 58 - Adding virtual function for generating saccades
virtual QVector<Saccade> generateSaccades()=0;
QVector<Saccade>& getSaccades();

protected:
virtual Fixation computeFixationEstimate(QVector<Gaze>)=0;

QVector<Gaze> session_gazes;
QVector<Fixation> fixations;

//issue 58 - a vector to store our saccades
//Also, not sure if we'll need a saccade estimate
QVector<Saccade> saccades;
Saccade computeSaccadeEstimate(QVector<Gaze>);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function computeSaccadeEstimate is declared as a non-virtual member function in the base class FixationAlgorithm, but it is not implemented here. This will cause a linker error if it's ever called on a FixationAlgorithm pointer/reference that points to a derived class that doesn't implement it (like BasicAlgorithm). To enforce that derived classes provide an implementation, you should declare it as a pure virtual function, similar to computeFixationEstimate.

Suggested change
Saccade computeSaccadeEstimate(QVector<Gaze>);
virtual Saccade computeSaccadeEstimate(QVector<Gaze>)=0;

};

#endif // FIXATIONALGORITHM_H
5 changes: 3 additions & 2 deletions gaze.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@ class Gaze {
Gaze(const Gaze&);

bool isValid();

void print();

int db_id = -1;

int left_validation, right_validation,
source_file_line = -1, source_file_col = -1;
long long event_time, system_time;
double x, y, left_pupil_diameter, right_pupil_diameter;
QString gaze_target = "", gaze_target_type = "",
source_token = "", source_token_xpath = "", source_token_syntatic_context = "";
source_token = "", source_token_xpath = "", source_token_syntatic_context = "";

//friend std::ostream& operator<<(std::ostream&,const Gaze&);

Expand Down
4 changes: 4 additions & 0 deletions idtalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ QVector<Fixation> IDTAlgorithm::generateFixations() {
return fixations;
}

QVector<Saccade> IDTAlgorithm::generateSaccades(){
return saccades;
}

Fixation IDTAlgorithm::computeFixationEstimate(QVector<Gaze> fixation_points) {
Fixation fixation;
double x_total = 0, y_total = 0;
Expand Down
2 changes: 2 additions & 0 deletions idtalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class IDTAlgorithm : public FixationAlgorithm{
QVector<Fixation> generateFixations() override;
QString generateFixationSettings() override;

QVector<Saccade> generateSaccades() override;

private:
Fixation computeFixationEstimate(QVector<Gaze>) override;

Expand Down
Loading