Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unsafe mode #367

Closed
wants to merge 4 commits into from
Closed
Changes from 1 commit
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
Next Next commit
unsafe: optionally allow potentially unsafe operations
missinglink committed Mar 23, 2020
commit ec039bf5d03a7af55ec07c175c5ba9c63471183e
4 changes: 3 additions & 1 deletion lib/database.js
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ function Database(filenameGiven, options) {
const readonly = util.getBooleanOption(options, 'readonly');
const fileMustExist = util.getBooleanOption(options, 'fileMustExist');
const timeout = 'timeout' in options ? options.timeout : 5000;
const unsafe = util.getBooleanOption(options, 'unsafe');
const verbose = 'verbose' in options ? options.verbose : null;

if (readonly && (memory || anonymous)) throw new TypeError('In-memory databases cannot be readonly');
@@ -43,7 +44,8 @@ function Database(filenameGiven, options) {
.replace(/\/\/+/g, '/')
+ '?mode=memory&cache=shared';
}
return new CPPDatabase(filename, filenameGiven, memory || anonymous, readonly, fileMustExist, timeout, verbose || null);

return new CPPDatabase(filename, filenameGiven, memory || anonymous, readonly, fileMustExist, timeout, unsafe, verbose || null);
}

util.wrap(CPPDatabase, 'pragma', require('./pragma'));
11 changes: 8 additions & 3 deletions src/objects/database.lzz
Original file line number Diff line number Diff line change
@@ -55,6 +55,7 @@ public:
const bool safe_ints;
bool was_js_error;
unsigned short iterators;
bool unsafe_mode;
};

inline State* GetState() {
@@ -89,7 +90,7 @@ private:
}
};

explicit Database(sqlite3* _db_handle, v8::Isolate* isolate, v8::Local<v8::Value> _logger) : node::ObjectWrap(),
explicit Database(sqlite3* _db_handle, v8::Isolate* isolate, bool _unsafe, v8::Local<v8::Value> _logger) : node::ObjectWrap(),
db_handle(_db_handle),
open(true),
busy(false),
@@ -98,6 +99,7 @@ private:
was_js_error(false),
has_logger(_logger->IsFunction()),
iterators(0),
unsafe_mode(_unsafe),
logger(isolate, _logger),
stmts(),
backups() {
@@ -137,7 +139,8 @@ private:
REQUIRE_ARGUMENT_BOOLEAN(fourth, bool readonly);
REQUIRE_ARGUMENT_BOOLEAN(fifth, bool must_exist);
REQUIRE_ARGUMENT_INT32(sixth, int timeout);
REQUIRE_ARGUMENT_ANY(seventh, v8::Local<v8::Value> logger);
REQUIRE_ARGUMENT_BOOLEAN(seventh, bool unsafe);
REQUIRE_ARGUMENT_ANY(eighth, v8::Local<v8::Value> logger);

UseIsolate;
sqlite3* db_handle;
@@ -164,11 +167,12 @@ private:
assert(status == SQLITE_OK);

UseContext;
Database* db = new Database(db_handle, isolate, logger);
Database* db = new Database(db_handle, isolate, unsafe, logger);
db->Wrap(info.This());
SetFrozen(isolate, ctx, info.This(), CS::memory, in_memory ? v8::True(isolate) : v8::False(isolate));
SetFrozen(isolate, ctx, info.This(), CS::readonly, readonly ? v8::True(isolate) : v8::False(isolate));
SetFrozen(isolate, ctx, info.This(), CS::name, filenameGiven);
SetFrozen(isolate, ctx, info.This(), CS::unsafe, unsafe ? v8::True(isolate) : v8::False(isolate));

info.GetReturnValue().Set(info.This());
}
@@ -375,6 +379,7 @@ private:
bool was_js_error;
const bool has_logger;
unsigned short iterators;
bool unsafe_mode;
const CopyablePersistent<v8::Value> logger;
std::set<Statement*, Database::CompareStatement> stmts;
std::set<Backup*, Database::CompareBackup> backups;
2 changes: 2 additions & 0 deletions src/util/constants.lzz
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@ public:
static ConstantString memory;
static ConstantString readonly;
static ConstantString name;
static ConstantString unsafe;
static ConstantString next;
static ConstantString length;
static ConstantString done;
@@ -37,6 +38,7 @@ private:
AddString(isolate, CS::memory, "memory");
AddString(isolate, CS::readonly, "readonly");
AddString(isolate, CS::name, "name");
AddString(isolate, CS::unsafe, "unsafe");
AddString(isolate, CS::next, "next");
AddString(isolate, CS::length, "length");
AddString(isolate, CS::done, "done");
2 changes: 1 addition & 1 deletion src/util/macros.lzz
Original file line number Diff line number Diff line change
@@ -64,7 +64,7 @@ void ThrowRangeError(const char* message) { EasyIsolate; isolate->ThrowException
if (db->busy) \
return ThrowTypeError("This database connection is busy executing a query")
#define REQUIRE_DATABASE_NO_ITERATORS(db) \
if (db->iterators) \
if (db->iterators && !db->unsafe_mode) \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense that unsafe also affects the case above, not just the iterators case. E.g. to allow things such as #338

And what about locked? Which use-case does that prevent and could this be disabled in unsafe mode as well?

Copy link
Author

Choose a reason for hiding this comment

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

Please provide a test case which demonstrates the functionality and I'll include it.

return ThrowTypeError("This database connection is busy executing a query")
#define REQUIRE_STATEMENT_NOT_LOCKED(stmt) \
if (stmt->locked) \
15 changes: 15 additions & 0 deletions test/10.database.open.js
Original file line number Diff line number Diff line change
@@ -51,6 +51,7 @@ describe('new Database()', function () {
expect(db.readonly).to.be.false;
expect(db.open).to.be.true;
expect(db.inTransaction).to.be.false;
expect(db.unsafe).to.be.false;
expect(existsSync(':memory:')).to.be.false;
});
it('should allow named in-memory databases to be created', function () {
@@ -61,6 +62,7 @@ describe('new Database()', function () {
expect(db.readonly).to.be.false;
expect(db.open).to.be.true;
expect(db.inTransaction).to.be.false;
expect(db.unsafe).to.be.false;
expect(existsSync(util.current())).to.be.false;
});
it('should allow disk-bound databases to be created', function () {
@@ -71,6 +73,7 @@ describe('new Database()', function () {
expect(db.readonly).to.be.false;
expect(db.open).to.be.true;
expect(db.inTransaction).to.be.false;
expect(db.unsafe).to.be.false;
expect(existsSync(util.current())).to.be.true;
});
it('should not allow conflicting in-memory options', function () {
@@ -90,6 +93,7 @@ describe('new Database()', function () {
expect(db.readonly).to.be.true;
expect(db.open).to.be.true;
expect(db.inTransaction).to.be.false;
expect(db.unsafe).to.be.false;
expect(existsSync(util.current())).to.be.true;
});
it('should not allow the "readonly" option for in-memory databases', function () {
@@ -110,6 +114,7 @@ describe('new Database()', function () {
expect(db.readonly).to.be.false;
expect(db.open).to.be.true;
expect(db.inTransaction).to.be.false;
expect(db.unsafe).to.be.false;
expect(existsSync(util.current())).to.be.true;
});
util.itUnix('should accept the "timeout" option', function () {
@@ -145,6 +150,16 @@ describe('new Database()', function () {
expect(existsSync(filepath)).to.be.false;
expect(existsSync(util.current())).to.be.false;
});
it('should accept the "unsafe" option', function () {
const db = this.db = new Database(util.current(), { unsafe: true });
expect(db.name).to.equal(util.current());
expect(db.memory).to.be.false;
expect(db.readonly).to.be.false;
expect(db.open).to.be.true;
expect(db.inTransaction).to.be.false;
expect(db.unsafe).to.be.true;
expect(existsSync(util.current())).to.be.true;
});
it('should have a proper prototype chain', function () {
const db = this.db = new Database(util.next());
expect(db).to.be.an.instanceof(Database);