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

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

Open
andk opened this issue May 30, 2020 · 5 comments
Open

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

andk opened this issue May 30, 2020 · 5 comments

Comments

@andk
Copy link

andk commented May 30, 2020

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
Copy link

jkeenan commented Jun 2, 2020

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
Copy link

ppisar commented Jun 4, 2020

This is triggered by mojolicious/mojo@52a237a.

@ppisar
Copy link

ppisar commented Jun 4, 2020

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
Copy link

ppisar commented Jun 4, 2020

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
Copy link

ppisar commented Jun 4, 2020

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;

ppisar added a commit to ppisar/mango that referenced this issue Jun 4, 2020
Mojo::Base 0.50 enables unicode_strings feature in the scope where it
used from. As a result, deserializing a regular expression injects
qr//u flag to regexp object although the was no such flags in the
deserialized BSON. The same issue repeats in t/bson.t when shuffling
with the regular expressions.

The consequence is that a t/bson.t test fails.

This patch fixes it with disabling unicode_strings feature where
needed.

oliwer#36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants