Bug #112474 for DBD-SQLite: FTS3 custom tokenizers disabled by default in SQLite 3.11.0

Sat Feb 27 06:00:23 2016 Stromeko [...] NexGo.DE - Ticket created

Subject:	FTS3 custom tokenizers disabled by default in SQLite 3.11.0
Date:	Sat, 27 Feb 2016 11:59:59 +0100
To:	bug-DBD-SQLite [...] rt.cpan.org
From:	Achim Gratz <Stromeko [...] nexgo.de>

Starting with SQLite version 3.11.0 the FTS3 custom tokenizer is disabled due to security concerns unless SQLITE_ENABLE_FTS3_TOKENIZER is defined during compilation. The following patch skips the FTS3 tests in this case since they don't work if the custom tokenizer can't be registered: --8<---------------cut here---------------start------------->8--- --- origsrc/DBD-SQLite-1.50/t/43_fts3.t 2015-02-17 07:57:49.000000000 +0100 +++ src/DBD-SQLite-1.50/t/43_fts3.t 2016-02-27 11:55:34.715375700 +0100 @@ -30,9 +30,12 @@ BEGIN { if ($] < 5.008005) { plan skip_all => 'Unicode is not supported before 5.8.5'; } - if (!grep /ENABLE_FTS3/, DBD::SQLite::compile_options()) { + unless (grep /ENABLE_FTS3/, DBD::SQLite::compile_options()) { plan skip_all => 'FTS3 is disabled for this DBD::SQLite'; } + unless (has_sqlite('3.11.0') and grep /ENABLE_FTS3_TOKENIZER/, DBD::SQLite::compile_options()) { + plan skip_all => 'FTS3 custom tokenizers are disabled for this DBD::SQLite'; + } } use Test::NoWarnings; --8<---------------cut here---------------end--------------->8--- Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Wavetables for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Sat Feb 27 07:00:21 2016 ishigaki [...] cpan.org - Correspondence added

On Sat Feb 27 20:00:23 2016, Stromeko@NexGo.DE wrote: Show quoted text

> > Starting with SQLite version 3.11.0 the FTS3 custom tokenizer is > disabled due to security concerns unless SQLITE_ENABLE_FTS3_TOKENIZER > is > defined during compilation. The following patch skips the FTS3 tests > in > this case since they don't work if the custom tokenizer can't be > registered: > > --8<---------------cut here---------------start------------->8--- > --- origsrc/DBD-SQLite-1.50/t/43_fts3.t 2015-02-17 07:57:49.000000000 > +0100 > +++ src/DBD-SQLite-1.50/t/43_fts3.t 2016-02-27 11:55:34.715375700 > +0100 > @@ -30,9 +30,12 @@ BEGIN { > if ($] < 5.008005) { > plan skip_all => 'Unicode is not supported before > 5.8.5'; > } > - if (!grep /ENABLE_FTS3/, DBD::SQLite::compile_options()) { > + unless (grep /ENABLE_FTS3/, DBD::SQLite::compile_options()) { > plan skip_all => 'FTS3 is disabled for this > DBD::SQLite'; > } > + unless (has_sqlite('3.11.0') and grep /ENABLE_FTS3_TOKENIZER/, > DBD::SQLite::compile_options()) { > + plan skip_all => 'FTS3 custom tokenizers are disabled > for this DBD::SQLite'; > + } > } > use Test::NoWarnings; > > --8<---------------cut here---------------end--------------->8--- > > > Regards, > Achim.

Thanks, but that's already been there since 1.51_01 :)

Sat Feb 27 07:00:28 2016 The RT System itself - Status changed from 'new' to 'open'

Sat Feb 27 07:00:32 2016 ishigaki [...] cpan.org - Status changed from 'open' to 'patched'

Sat Jul 23 14:26:07 2016 ppisar [...] redhat.com - Correspondence added

From:

ppisar [...] redhat.com

Dne So 27.úno.2016 07:00:21, ISHIGAKI napsal(a): Show quoted text

> On Sat Feb 27 20:00:23 2016, Stromeko@NexGo.DE wrote:

> > > > Starting with SQLite version 3.11.0 the FTS3 custom tokenizer is > > disabled due to security concerns unless SQLITE_ENABLE_FTS3_TOKENIZER > > is > > defined during compilation.

[...] Show quoted text

> Thanks, but that's already been there since 1.51_01 :)

I will hijack this bug report, because with the current code, the t/43_fts3.t fails if compiled against sqlite 3.7.17: PERL_DL_NONLAZY=1 "/opt/rh/rh-perl524/root/usr/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/**/*.t # $DBI::VERSION=1.636 # Compile Options: # DISABLE_DIRSYNC # ENABLE_COLUMN_METADATA # ENABLE_FTS3 # ENABLE_RTREE # ENABLE_UNLOCK_NOTIFY # SECURE_DELETE # TEMP_STORE=1 # THREADSAFE=1 t/01_compile.t ........................................ ok # sqlite_version=3.7.17 t/02_logon.t .......................................... ok [...] t/42_primary_key_info.t ............................... ok DBD::SQLite::db do failed: unknown tokenizer: perl at t/43_fts3.t line 90. # Failed test 'no warnings' # at /opt/rh/rh-perl524/root/usr/share/perl5/vendor_perl/Test/Builder.pm line 135. # There were 1 warning(s) # Previous test 1 'An object of class 'DBI::db' isa 'DBI::db'' # DBD::SQLite::db do failed: unknown tokenizer: perl at t/43_fts3.t line 90. # at t/43_fts3.t line 90. # # Looks like your test exited with 255 just after 2. t/43_fts3.t ........................................... Dubious, test returned 255 (wstat 65280, 0xff00) Failed 34/35 subtests The reason is the FT3 tokenizer tests are skipped like this: if ($DBD::SQLite::sqlite_version_number >= 3011000 and !grep /ENABLE_FTS3_TOKENIZER/, DBD::SQLite::compile_options()) { plan skip_all => 'FTS3 tokenizer is disabled for this DBD::SQLite'; } But DBD-SQLite does not register the perl tokenizer by default: static int register_fts3_perl_tokenizer(dbh) SV *dbh ALIAS: DBD::SQLite::db::sqlite_register_fts3_perl_tokenizer = 1 CODE: #if SQLITE_ENABLE_FTS3_TOKENIZER RETVAL = sqlite_db_register_fts3_perl_tokenizer(aTHX_ dbh); #else RETVAL = 0; #endif OUTPUT: RETVAL So the DBD-SQLite code does not support the tokenizer and the test does not find sqlite >= 3.11.0, so it proceeds with the tests and then the tests fail on unknown perl tokenizer because the code did not register it. I don't know how to fix the test properly. We cannot check for the keyword in the compile options unconditionally because older sqlite does have the keyword even if it supports the tokenizer. Either the Makefile.PL could propagate the SQLITE_ENABLE_FTS3_TOKENIZER environment variable to the tests, or maybe better one as dbdimp.c condition the struct perl_tokenizer by: #if SQLITE_VERSION_NUMBER < 3011000 || SQLITE_ENABLE_FTS3_TOKENIZER the SQLite.xs should do the same instead of current: #if SQLITE_ENABLE_FTS3_TOKENIZER Any ideas?

Sun Jul 24 07:27:02 2016 ppisar [...] redhat.com - Correspondence added

From:

ppisar [...] redhat.com

Dne So 23.čec.2016 14:26:07, ppisar napsal(a): Show quoted text

> I will hijack this bug report, because with the current code, the > t/43_fts3.t fails if compiled against sqlite 3.7.17:

[...] Show quoted text

> or maybe better one as dbdimp.c condition the struct perl_tokenizer by: > > #if SQLITE_VERSION_NUMBER < 3011000 || SQLITE_ENABLE_FTS3_TOKENIZER > > the SQLite.xs should do the same instead of current: > > #if SQLITE_ENABLE_FTS3_TOKENIZER >

I went this way. See attached patch. It works with sqlite 3.13.0 as well as with 3.7.17.

Subject:

DBD-SQLite-1.51_05-Always-register-perl-FTS3-tokenizer-with-sqlite-befo.patch

From 5687040e291c06a7e49cae9cfae0f2be96b05ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com> Date: Sun, 24 Jul 2016 11:15:52 +0200 Subject: [PATCH] Always register perl FTS3 tokenizer with sqlite before 3.11.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disabling perl FTS3 tokenizer by default caused t/43_fts3.t failures if building against sqlite < 3.11.0. That was caused by assymetry between enabling the support (purely on SQLITE_ENABLE_FTS3_TOKENIZER environemnt variable) and skipping the tests (on version and ENABLE_FTS3_TOKENIZER compile option). CPAN RT#112474 Signed-off-by: Petr PÃsaÅ <ppisar@redhat.com> --- SQLite.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SQLite.xs b/SQLite.xs index f20e511..06fc425 100644 --- a/SQLite.xs +++ b/SQLite.xs @@ -288,7 +288,7 @@ register_fts3_perl_tokenizer(dbh) ALIAS: DBD::SQLite::db::sqlite_register_fts3_perl_tokenizer = 1 CODE: -#if SQLITE_ENABLE_FTS3_TOKENIZER +#if SQLITE_VERSION_NUMBER < 3011000 || SQLITE_ENABLE_FTS3_TOKENIZER RETVAL = sqlite_db_register_fts3_perl_tokenizer(aTHX_ dbh); #else RETVAL = 0; -- 2.5.5

Tue Jul 26 15:48:26 2016 ishigaki [...] cpan.org - Correspondence added

On Sun Jul 24 20:27:02 2016, ppisar wrote: Show quoted text

> Dne So 23.čec.2016 14:26:07, ppisar napsal(a):

> > I will hijack this bug report, because with the current code, the > > t/43_fts3.t fails if compiled against sqlite 3.7.17:

> [...]

> > or maybe better one as dbdimp.c condition the struct perl_tokenizer > > by: > > > > #if SQLITE_VERSION_NUMBER < 3011000 || SQLITE_ENABLE_FTS3_TOKENIZER > > > > the SQLite.xs should do the same instead of current: > > > > #if SQLITE_ENABLE_FTS3_TOKENIZER > >

> I went this way. See attached patch. It works with sqlite 3.13.0 as > well as with 3.7.17.

Hi, sorry for the delayed reply. Apparently you must have patched your Makefile.PL (because DBD::SQLite doesn't support older versions of sqlite library by default, right?), it's up to you to patch whatever you think is necessary for your users :) Joking apart, we actually need to consider more to fix the real problem here. It's not only the version of SQLite, but the result of sqlite_db_config() (which is not yet supported in DBD::SQLite, but should be) may matter, either (or not, depending on the way to write tests). So, what do you think is the best possible scenario? I'm happy to support/apply anything, as long as that doesn't break other important modules.

Fri Dec 23 21:45:14 2016 ishigaki [...] cpan.org - Correspondence added

On Wed Jul 27 04:48:26 2016, ISHIGAKI wrote: Show quoted text

> On Sun Jul 24 20:27:02 2016, ppisar wrote:

> > Dne So 23.čec.2016 14:26:07, ppisar napsal(a):

> > > I will hijack this bug report, because with the current code, the > > > t/43_fts3.t fails if compiled against sqlite 3.7.17:

> > [...]

> > > or maybe better one as dbdimp.c condition the struct perl_tokenizer > > > by: > > > > > > #if SQLITE_VERSION_NUMBER < 3011000 || SQLITE_ENABLE_FTS3_TOKENIZER > > > > > > the SQLite.xs should do the same instead of current: > > > > > > #if SQLITE_ENABLE_FTS3_TOKENIZER > > >

> > I went this way. See attached patch. It works with sqlite 3.13.0 as > > well as with 3.7.17.

> > Hi, sorry for the delayed reply. > > Apparently you must have patched your Makefile.PL (because DBD::SQLite > doesn't support older versions of sqlite library by default, right?), > it's up to you to patch whatever you think is necessary for your users > :) > > Joking apart, we actually need to consider more to fix the real > problem here. It's not only the version of SQLite, but the result of > sqlite_db_config() (which is not yet supported in DBD::SQLite, but > should be) may matter, either (or not, depending on the way to write > tests). > > So, what do you think is the best possible scenario? I'm happy to > support/apply anything, as long as that doesn't break other important > modules.

Fixed this issue by calling sqlite_db_config. Thanks.

Fri Dec 23 21:45:15 2016 ishigaki [...] cpan.org - Status changed from 'patched' to 'resolved'