oliwer / mango

Pure-Perl non-blocking I/O MongoDB driver
https://metacpan.org/release/Mango
Artistic License 2.0
27 stars 12 forks source link

Test t/bson.t failing (with Mojolicious 8.50?) #36

Open andk opened 4 years ago

andk commented 4 years ago

The test t/bson.t started failing recently. It seems all fails happened with Mojolicious 8.50 so far.

Sample fail reports can be found via the matrix: http://fast-matrix.cpantesters.org/?dist=Mango%201.30

Picking the first one that arrived at cpantesters: http://www.cpantesters.org/cpan/report/482d3238-a223-11ea-bf34-d6ec90b5e442

jkeenan commented 4 years ago

I encountered this failure while testing Mango on FreeBSD-12 against a perl very close to 5.32-RC0.

$ dumpjson ODC.Mango-1.30.log.json 
{
  author => "ODC",
  dist => "Mango",
  distname => "Mango-1.30",
  distversion => "1.30",
  grade => "FAIL",
  prereqs => undef,
  test_output => [
    "Building and testing Mango-1.30",
    "cp lib/Mango/Cursor.pm blib/lib/Mango/Cursor.pm",
    "cp lib/Mango/BSON/Document.pm blib/lib/Mango/BSON/Document.pm",
    "cp lib/Mango/Auth.pm blib/lib/Mango/Auth.pm",
    "cp lib/Mango/BSON/Timestamp.pm blib/lib/Mango/BSON/Timestamp.pm",
    "cp lib/Mango/BSON/Time.pm blib/lib/Mango/BSON/Time.pm",
    "cp lib/Mango/Auth/SCRAM.pm blib/lib/Mango/Auth/SCRAM.pm",
    "cp lib/Mango/Collection.pm blib/lib/Mango/Collection.pm",
    "cp lib/Mango/GridFS/Writer.pm blib/lib/Mango/GridFS/Writer.pm",
    "cp lib/Mango/Bulk.pm blib/lib/Mango/Bulk.pm",
    "cp lib/Mango/BSON/Number.pm blib/lib/Mango/BSON/Number.pm",
    "cp lib/Mango/Protocol.pm blib/lib/Mango/Protocol.pm",
    "cp lib/Mango/BSON/Binary.pm blib/lib/Mango/BSON/Binary.pm",
    "cp lib/Mango/GridFS/Reader.pm blib/lib/Mango/GridFS/Reader.pm",
    "cp lib/Mango.pm blib/lib/Mango.pm",
    "cp lib/Mango/Database.pm blib/lib/Mango/Database.pm",
    "cp lib/Mango/Cursor/Query.pm blib/lib/Mango/Cursor/Query.pm",
    "cp lib/Mango/BSON/ObjectID.pm blib/lib/Mango/BSON/ObjectID.pm",
    "cp lib/Mango/GridFS.pm blib/lib/Mango/GridFS.pm",
    "cp lib/Mango/BSON/Code.pm blib/lib/Mango/BSON/Code.pm",
    "cp lib/Mango/BSON.pm blib/lib/Mango/BSON.pm",
    "PERL_DL_NONLAZY=1 \"/usr/home/jkeenan/testing/smoke-me/khw-intrpvar/bin/perl\" \"-MExtUtils::Command::MM\" \"-MTest::Harness\" \"-e\" \"undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')\" t/*.t t/*/*.t",
    "t/auth/auth.t .......... skipped: set TEST_ONLINE to enable this test",
    "t/auth/authenticate.t .. skipped: set TEST_ONLINE to enable this test",
    "",
    "#   Failed test 'successful roundtrip'",
    "#   at t/bson.t line 274.",
    "#          got: '\23\0\0\0\13regex\0a*b\0ui\0\0'",
    "#     expected: '\22\0\0\0\13regex\0a*b\0i\0\0'",
    "# Looks like you failed 1 test of 182.",
    "t/bson.t ............... ",
    "Dubious, test returned 1 (wstat 256, 0x100)",
    "Failed 1/182 subtests ",
    "t/bulk.t ............... skipped: set TEST_ONLINE to enable this test",
    "t/collection.t ......... skipped: set TEST_ONLINE to enable this test",
    "t/connection.t ......... skipped: set TEST_ONLINE to enable this test",
    "t/cursor.t ............. skipped: set TEST_ONLINE to enable this test",
    "t/database.t ........... skipped: set TEST_ONLINE to enable this test",
    "t/gridfs.t ............. skipped: set TEST_ONLINE to enable this test",
    "t/leaks/auth.t ......... skipped: set TEST_ONLINE to enable this test",
    "t/pod.t ................ skipped: set TEST_POD to enable this test (developer only!)",
    "t/pod_coverage.t ....... skipped: set TEST_POD to enable this test (developer only!)",
    "t/protocol.t ........... ok",
    "",
    "Test Summary Report",
    "-------------------",
    "t/bson.t             (Wstat: 256 Tests: 182 Failed: 1)",
    "  Failed test:  106",
    "  Non-zero exit status: 1",
    "Files=13, Tests=205,  5 wallclock secs ( 0.04 usr  0.05 sys +  3.32 cusr  1.20 csys =  4.62 CPU)",
    "Result: FAIL",
  ],
  via => "App::cpanminus::reporter 0.17 (1.7044)",
}

Can you investigate?

Thank you very much. Jim Keenan

ppisar commented 4 years ago

This is triggered by https://github.com/mojolicious/mojo/commit/52a237a813e778a998caa99e71dd8c8d5fd0cd2a.

ppisar commented 4 years ago

The Mojolicious change adds use 5.016; that enables unicode_eval feature. The Mojolicious scope somehow leaks into Mango::BSON::_decode_value() that compiles a regular expression:

  # Regex
  if ($type eq REGEX) {
    my ($p, $m) = (_decode_cstring($bsonref), _decode_cstring($bsonref));
    croak "invalid regex modifier(s) in 'qr/$p/$m'"
      if length($m) and $m !~ /^[msixpadlun]+\z/;
    # escape $pat to avoid code injection
    return eval "qr/\$p/$m";
  }

As a result (?^i:a*b) regular expression stored in the test data of t/base.t becomes compiled as (?^ui:a*b) and thus the roundtrip test fails.

Coincidentely, another t/base.t test compares the parsed value against qr/a*b/i that because of the same leaks gets compiled also into qr/a*b/ui and thus that another test passes. When I disable unicode_eval feature in Mango::BSON::_decode_value() code, then the reported failure disappears, but this another test emerges.

So the question is why the feature bit leaks. And how to fix it.

ppisar commented 4 years ago

Actually it's simple. Mojo::Base that is used from both /lib/Mango/BSON.pm a t/base.t does use 5.016; in the top-level scope. Thus it's in the same scope as the test code. This behavior is even documented in Mojolicious::Guides:

      Mojolicious uses a modern subset of Perl exclusively, and therefore
      all documentation assumes that strict, warnings, utf8 and Perl 5.16
      features are enabled, even if examples don't specifically mention it.

        use strict;
        use warnings;
        use utf8;
        use feature ':5.16';

      Some modules, like Mojo::Base and Mojolicious::Lite, will enable them
      for you automatically, whenever they are used.
ppisar commented 4 years ago

A plain top-level use feature ':5.16'; is not enough. The explanation is a bit longer:

package Mojo::Base;
sub import {
   require feature;
   feature->import(':5.16');
}
1;