PerlDancer/Dancer2-Plugin-Passphrase

Test failure with newer Digest::Bcrypt

gregoa opened this issue · 12 comments

In Debian we are seeing test failures of Dancer2-Plugin-Passphrase, most probably caused by a newer Digest::Bcrypt. The latter switched to using Crypt::Bcrypt in 1.210 (and recommends to use Crypt::Bcrypt directly). Crypt::Bcrypt appears in the test error as well:

#   Failed test 'Bcrypt needs encoded text'
#   at t/008_unicode_matching.t line 44.
#                   'Wide character in subroutine entry at /usr/lib/x86_64-linux-gnu/perl5/5.32/Crypt/Bcrypt.pm line 21.
# '
#     doesn't match '(?^i:input must contain only octets)'
# Looks like you failed 1 test of 8.
t/008_unicode_matching.t ............ 
1..8
ok 1 - UTF8 matches UTF8 for SHA-1
ok 2 - SHA-1 needs encoded text
ok 3 - UTF8 matches UTF8 for SHA-256
ok 4 - SHA-256 needs encoded text
ok 5 - UTF8 matches UTF8 for MD5
ok 6 - MD5 needs encoded text
ok 7 - UTF8 matches UTF8 for Bcrypt
not ok 8 - Bcrypt needs encoded text
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/8 subtests 

Full test log: https://ci.debian.net/data/autopkgtest/testing/amd64/libd/libdancer2-plugin-passphrase-perl/18819128/log.gz

CPAN testers have one example of the same issue: https://www.cpantesters.org/cpan/report/c48d8c4a-7c80-11ec-a7a8-acce433ff426

Cheers,
gregor

Thanks. I noticed this recently as well. We're discussing how we want to fix this. Tagging @SysPete in case he wants to add anything.

This plugin is probably going to be deprecated in favour of Dancer2::Plugin::CryptPassphrase, though I'd also like to see if I can get it fixed in a back-compat way. Hopefully will have chance to look at it this next weekend.

Any news on this issue?

Cheers,
gregor, going through the list of open bugs in Debian and checking the status

I just came across this problem when resurrecting an old project that uses this module. I'll try to replace it with Dancer2::Plugin::CryptPassphrase, but it would be good if the original module could have another release saying that it's been deprecated.

@davorg I'll sort a release with a deprecation notice this weekend, though I'm also still hoping to be able to fix it up somehow. We'll see.

@gregoa I'll try to have a definitive answer for you in the next 7-10 days.

@davorg I'll sort a release with a deprecation notice this weekend, though I'm also still hoping to be able to fix it up somehow. We'll see.

FWIW, I got the replacement working really easily.

PR #6 handles the issue, though I dislike having to monkeypatch. Also marking it as deprecated. New release will happen tomorrow unless any issues are spotted in the meantime.

PR merged and new release pushed to CPAN

@gregoa I just pushed v3.4.1 to CPAN which removes the monkeypatch from the previous release, and instead just adds updated dependencies. If these deps are OK for you then I'd recommend this newer version.