Skip to content

Commit

Permalink
SqlQuery: Write NULL when intended #4548
Browse files Browse the repository at this point in the history 
In SQLite bindings are not cleared by sqlite3_reset() calls, so
skipping a sqlite3_bind call to create a NULL value doesn't work,
instead the previous value will be written.

To fix this, I clear all bindings in SqlQuery::reset and make sure
to explicitly bind NULL when desired in SqlQuery::bind.

To make sure there's no confusion about SqlQuery::reset and
sqlite3_reset, I rename our method to reset_and_clear_bindings().

(cherry picked from commit 7bd4f95)
  • Loading branch information
@ckamm
ckamm committed Mar 15, 2016
1 parent ecd44f7 commit 4900703
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/gui/socketapi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ SyncJournalFileRecord SocketApi::dbFileRecord_capi( Folder *folder, QString file
rec._fileId = query->baValue(4);
rec._remotePerm = query->baValue(5);
}
query->reset();
query->reset_and_clear_bindings();
}
return rec;
}
Expand Down
6 changes: 3 additions & 3 deletions src/libsync/ownsql.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ void SqlQuery::bindValue(int pos, const QVariant& value)
res = sqlite3_bind_text16(_stmt, pos, str->utf16(),
(str->size()) * sizeof(QChar), SQLITE_TRANSIENT);
} else {
// unbound value create a null entry.
res = SQLITE_OK;
res = sqlite3_bind_null(_stmt, pos);
}
break; }
default: {
Expand Down Expand Up @@ -365,9 +364,10 @@ void SqlQuery::finish()
_stmt = 0;
}

void SqlQuery::reset()
void SqlQuery::reset_and_clear_bindings()
{
SQLITE_DO(sqlite3_reset(_stmt));
SQLITE_DO(sqlite3_clear_bindings(_stmt));
}

} // namespace OCC
2 changes: 1 addition & 1 deletion src/libsync/ownsql.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class OWNCLOUDSYNC_EXPORT SqlQuery
void bindValue(int pos, const QVariant& value);
QString lastQuery() const;
int numRowsAffected();
void reset();
void reset_and_clear_bindings();
void finish();

private:
Expand Down
64 changes: 32 additions & 32 deletions src/libsync/syncjournaldb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ bool SyncJournalDb::setFileRecord( const SyncJournalFileRecord& _record )
QString remotePerm (record._remotePerm);
if (remotePerm.isEmpty()) remotePerm = QString(); // have NULL in DB (vs empty)
int contentChecksumTypeId = mapChecksumType(record._contentChecksumType);
_setFileRecordQuery->reset();
_setFileRecordQuery->reset_and_clear_bindings();
_setFileRecordQuery->bindValue(1, QString::number(phash));
_setFileRecordQuery->bindValue(2, plen);
_setFileRecordQuery->bindValue(3, record._path );
Expand Down Expand Up @@ -722,7 +722,7 @@ bool SyncJournalDb::setFileRecord( const SyncJournalFileRecord& _record )
<< record._etag << record._fileId << record._remotePerm << record._fileSize << (record._serverHasIgnoredFiles ? 1:0)
<< record._contentChecksum << record._contentChecksumType << contentChecksumTypeId;

_setFileRecordQuery->reset();
_setFileRecordQuery->reset_and_clear_bindings();
return true;
} else {
qDebug() << "Failed to connect database.";
Expand All @@ -739,7 +739,7 @@ bool SyncJournalDb::deleteFileRecord(const QString& filename, bool recursively)
// always delete the actual file.

qlonglong phash = getPHash(filename);
_deleteFileRecordPhash->reset();
_deleteFileRecordPhash->reset_and_clear_bindings();
_deleteFileRecordPhash->bindValue( 1, QString::number(phash) );

if( !_deleteFileRecordPhash->exec() ) {
Expand All @@ -749,9 +749,9 @@ bool SyncJournalDb::deleteFileRecord(const QString& filename, bool recursively)
return false;
}
qDebug() << _deleteFileRecordPhash->lastQuery() << phash << filename;
_deleteFileRecordPhash->reset();
_deleteFileRecordPhash->reset_and_clear_bindings();
if( recursively) {
_deleteFileRecordRecursively->reset();
_deleteFileRecordRecursively->reset_and_clear_bindings();
_deleteFileRecordRecursively->bindValue(1, filename);
if( !_deleteFileRecordRecursively->exec() ) {
qWarning() << "Exec error of SQL statement: "
Expand All @@ -760,7 +760,7 @@ bool SyncJournalDb::deleteFileRecord(const QString& filename, bool recursively)
return false;
}
qDebug() << _deleteFileRecordRecursively->lastQuery() << filename;
_deleteFileRecordRecursively->reset();
_deleteFileRecordRecursively->reset_and_clear_bindings();
}
return true;
} else {
Expand All @@ -778,7 +778,7 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename )
SyncJournalFileRecord rec;

if( checkConnect() ) {
_getFileRecordQuery->reset();
_getFileRecordQuery->reset_and_clear_bindings();
_getFileRecordQuery->bindValue(1, QString::number(phash));

if (!_getFileRecordQuery->exec()) {
Expand Down Expand Up @@ -808,7 +808,7 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename )
QString err = _getFileRecordQuery->error();
qDebug() << "No journal entry found for " << filename;
}
_getFileRecordQuery->reset();
_getFileRecordQuery->reset_and_clear_bindings();
}
return rec;
}
Expand Down Expand Up @@ -907,7 +907,7 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString& filename,
int checksumTypeId = mapChecksumType(contentChecksumType);
auto & query = _setFileRecordChecksumQuery;

query->reset();
query->reset_and_clear_bindings();
query->bindValue(1, QString::number(phash));
query->bindValue(2, contentChecksum);
query->bindValue(3, checksumTypeId);
Expand All @@ -922,7 +922,7 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString& filename,
qDebug() << query->lastQuery() << phash << contentChecksum
<< contentChecksumType << checksumTypeId;

query->reset();
query->reset_and_clear_bindings();
return true;
}

Expand Down Expand Up @@ -964,7 +964,7 @@ static bool deleteBatch(SqlQuery & query, const QStringList & entries, const QSt
qDebug() << "Removing stale " << qPrintable(name) << " entries: " << entries.join(", ");
// FIXME: Was ported from execBatch, check if correct!
foreach( const QString& entry, entries ) {
query.reset();
query.reset_and_clear_bindings();
query.bindValue(1, entry);
if (!query.exec()) {
QString err = query.error();
Expand All @@ -973,7 +973,7 @@ static bool deleteBatch(SqlQuery & query, const QStringList & entries, const QSt
return false;
}
}
query.reset(); // viel hilft viel ;-)
query.reset_and_clear_bindings(); // viel hilft viel ;-)

return true;
}
Expand All @@ -985,7 +985,7 @@ SyncJournalDb::DownloadInfo SyncJournalDb::getDownloadInfo(const QString& file)
DownloadInfo res;

if( checkConnect() ) {
_getDownloadInfoQuery->reset();
_getDownloadInfoQuery->reset_and_clear_bindings();
_getDownloadInfoQuery->bindValue(1, file);

if (!_getDownloadInfoQuery->exec()) {
Expand All @@ -999,7 +999,7 @@ SyncJournalDb::DownloadInfo SyncJournalDb::getDownloadInfo(const QString& file)
} else {
res._valid = false;
}
_getDownloadInfoQuery->reset();
_getDownloadInfoQuery->reset_and_clear_bindings();
}
return res;
}
Expand All @@ -1013,7 +1013,7 @@ void SyncJournalDb::setDownloadInfo(const QString& file, const SyncJournalDb::Do
}

if (i._valid) {
_setDownloadInfoQuery->reset();
_setDownloadInfoQuery->reset_and_clear_bindings();
_setDownloadInfoQuery->bindValue(1, file);
_setDownloadInfoQuery->bindValue(2, i._tmpfile);
_setDownloadInfoQuery->bindValue(3, i._etag );
Expand All @@ -1025,18 +1025,18 @@ void SyncJournalDb::setDownloadInfo(const QString& file, const SyncJournalDb::Do
}

qDebug() << _setDownloadInfoQuery->lastQuery() << file << i._tmpfile << i._etag << i._errorCount;
_setDownloadInfoQuery->reset();
_setDownloadInfoQuery->reset_and_clear_bindings();

} else {
_deleteDownloadInfoQuery->reset();
_deleteDownloadInfoQuery->reset_and_clear_bindings();
_deleteDownloadInfoQuery->bindValue( 1, file );

if( !_deleteDownloadInfoQuery->exec() ) {
qWarning() << "Exec error of SQL statement: " << _deleteDownloadInfoQuery->lastQuery() << " : " << _deleteDownloadInfoQuery->error();
return;
}
qDebug() << _deleteDownloadInfoQuery->lastQuery() << file;
_deleteDownloadInfoQuery->reset();
_deleteDownloadInfoQuery->reset_and_clear_bindings();
}
}

Expand Down Expand Up @@ -1104,7 +1104,7 @@ SyncJournalDb::UploadInfo SyncJournalDb::getUploadInfo(const QString& file)

if( checkConnect() ) {

_getUploadInfoQuery->reset();
_getUploadInfoQuery->reset_and_clear_bindings();
_getUploadInfoQuery->bindValue(1, file);

if (!_getUploadInfoQuery->exec()) {
Expand All @@ -1122,7 +1122,7 @@ SyncJournalDb::UploadInfo SyncJournalDb::getUploadInfo(const QString& file)
res._modtime = Utility::qDateTimeFromTime_t(_getUploadInfoQuery->int64Value(4));
res._valid = ok;
}
_getUploadInfoQuery->reset();
_getUploadInfoQuery->reset_and_clear_bindings();
}
return res;
}
Expand All @@ -1136,7 +1136,7 @@ void SyncJournalDb::setUploadInfo(const QString& file, const SyncJournalDb::Uplo
}

if (i._valid) {
_setUploadInfoQuery->reset();
_setUploadInfoQuery->reset_and_clear_bindings();
_setUploadInfoQuery->bindValue(1, file);
_setUploadInfoQuery->bindValue(2, i._chunk);
_setUploadInfoQuery->bindValue(3, i._transferid );
Expand All @@ -1150,17 +1150,17 @@ void SyncJournalDb::setUploadInfo(const QString& file, const SyncJournalDb::Uplo
}

qDebug() << _setUploadInfoQuery->lastQuery() << file << i._chunk << i._transferid << i._errorCount;
_setUploadInfoQuery->reset();
_setUploadInfoQuery->reset_and_clear_bindings();
} else {
_deleteUploadInfoQuery->reset();
_deleteUploadInfoQuery->reset_and_clear_bindings();
_deleteUploadInfoQuery->bindValue(1, file);

if( !_deleteUploadInfoQuery->exec() ) {
qWarning() << "Exec error of SQL statement: " << _deleteUploadInfoQuery->lastQuery() << " : " << _deleteUploadInfoQuery->error();
return;
}
qDebug() << _deleteUploadInfoQuery->lastQuery() << file;
_deleteUploadInfoQuery->reset();
_deleteUploadInfoQuery->reset_and_clear_bindings();
}
}

Expand Down Expand Up @@ -1203,7 +1203,7 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry( const QStrin
// SELECT lastTryEtag, lastTryModtime, retrycount, errorstring

if( checkConnect() ) {
_getErrorBlacklistQuery->reset();
_getErrorBlacklistQuery->reset_and_clear_bindings();
_getErrorBlacklistQuery->bindValue( 1, file );
if( _getErrorBlacklistQuery->exec() ){
if( _getErrorBlacklistQuery->next() ) {
Expand All @@ -1215,7 +1215,7 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry( const QStrin
entry._ignoreDuration = _getErrorBlacklistQuery->int64Value(5);
entry._file = file;
}
_getErrorBlacklistQuery->reset();
_getErrorBlacklistQuery->reset_and_clear_bindings();
} else {
qWarning() << "Exec error blacklist: " << _getErrorBlacklistQuery->lastQuery() << " : "
<< _getErrorBlacklistQuery->error();
Expand Down Expand Up @@ -1331,7 +1331,7 @@ void SyncJournalDb::updateErrorBlacklistEntry( const SyncJournalErrorBlacklistRe
qDebug() << "set blacklist entry for " << item._file << item._retryCount
<< item._errorString << item._lastTryTime << item._ignoreDuration
<< item._lastTryModtime << item._lastTryEtag;
_setErrorBlacklistQuery->reset();
_setErrorBlacklistQuery->reset_and_clear_bindings();

}

Expand Down Expand Up @@ -1402,7 +1402,7 @@ QStringList SyncJournalDb::getSelectiveSyncList(SyncJournalDb::SelectiveSyncList
return result;
}

_getSelectiveSyncListQuery->reset();
_getSelectiveSyncListQuery->reset_and_clear_bindings();
_getSelectiveSyncListQuery->bindValue(1, int(type));
if (!_getSelectiveSyncListQuery->exec()) {
qWarning() << "SQL query failed: "<< _getSelectiveSyncListQuery->error();
Expand Down Expand Up @@ -1434,7 +1434,7 @@ void SyncJournalDb::setSelectiveSyncList(SyncJournalDb::SelectiveSyncListType ty

SqlQuery insQuery("INSERT INTO selectivesync VALUES (?1, ?2)" , _db);
foreach(const auto &path, list) {
insQuery.reset();
insQuery.reset_and_clear_bindings();
insQuery.bindValue(1, path);
insQuery.bindValue(2, int(type));
if (!insQuery.exec()) {
Expand Down Expand Up @@ -1526,7 +1526,7 @@ QByteArray SyncJournalDb::getChecksumType(int checksumTypeId)

// Retrieve the id
auto & query = *_getChecksumTypeQuery;
query.reset();
query.reset_and_clear_bindings();
query.bindValue(1, checksumTypeId);
if( !query.exec() ) {
qWarning() << "Error SQL statement getChecksumType: "
Expand All @@ -1549,7 +1549,7 @@ int SyncJournalDb::mapChecksumType(const QByteArray& checksumType)
}

// Ensure the checksum type is in the db
_insertChecksumTypeQuery->reset();
_insertChecksumTypeQuery->reset_and_clear_bindings();
_insertChecksumTypeQuery->bindValue(1, checksumType);
if( !_insertChecksumTypeQuery->exec() ) {
qWarning() << "Error SQL statement insertChecksumType: "
Expand All @@ -1559,7 +1559,7 @@ int SyncJournalDb::mapChecksumType(const QByteArray& checksumType)
}

// Retrieve the id
_getChecksumTypeIdQuery->reset();
_getChecksumTypeIdQuery->reset_and_clear_bindings();
_getChecksumTypeIdQuery->bindValue(1, checksumType);
if( !_getChecksumTypeIdQuery->exec() ) {
qWarning() << "Error SQL statement getChecksumTypeId: "
Expand Down

0 comments on commit 4900703

Please sign in to comment.