-
Notifications
You must be signed in to change notification settings - Fork 12
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
Merge develop branch into master (Zonemaster-LDNS) #215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merge develop into master (Zonemaster/Zonemaster-LDNS)
- Zonemaster::LDNS::RR::CDS and Zonemaster::LDNS::RR::CDNSKEY are now subclasses of Zonemaster::LDNS::RR::DS and Zonemaster::LDNS::RR::DNSKEY, respectively. - Add method Zonemaster::LDNS::RR::DNSKEY->hexkeydata() (similar to Zonemaster::LDNS::RR::DS->hexdigest()) - Add and update documentation - Add and update unit tests
The return value of ldns_rr_descript(), of type const ldns_rr_descriptor*, was assigned to a variable of type ldns_rr_descriptor*, thus dropping the const qualifier. Making said variable a const pointer fixes the warning. And it’s safe to do so too, because no modifications are made through that pointer.
The to_idn() function in Zonemaster::LDNS croaks with a plain string if an error condition is encountered when converting the supplied string to an IDN A-label. The test suite provokes one of those error conditions in order to check that a string is indeed thrown. That string is matched against a regular expression. But when constructing the string, idn2_strerror() is used, which produces a human-readable explanation of the error but in a locale-dependent way. So the test can succeed on one developer’s machine and fail on someone else’s whose system language is different. However, there is little point in replacing these plain strings with exception objects, such as a hypothetical Zonemaster::LDNS::Error class hierarchy. The only other place in the codebase where to_idn() is called is in one file in Zonemaster::Engine, and it doesn’t even care what specific error happened during IDN conversion. Exception objects would work, but they are a pain to build in XS code (even calling out to a constructor function written in Perl is extremely verbose in XS) and would be an extremely overengineered solution to our problem. Fortunately, instead of idn2_strerror(), we can use idn2_strerror_name(), which returns locale-independent strings based on the names of the source code’s macros. This solution is very simple, makes the error messages predictable and does not hamper debuggability of Zonemaster::LDNS::to_idn() too much. Of course, should Zonemaster::Engine want to display what specifically is wrong with the domain name they entered, we might eventually need the alternative solution.
This feature has always been experimental and seems to have been implemented a long time ago to test Zonemaster::LDNS internals. However, there are better ways to ensure that. This commit introduces deprecation warnings in three places: * in the documentation; * when executing Makefile.PL; * and during compilation of the C and XS code. Hopefully, having the same warning in three different places will be enough to alert end users.
Case in point (heh) of how little use the experimental case randomization feature had: one unit tests broke without anyone noticing it. We can reverse this patch later when the feature is actually removed.
Extend CDS/CDNSKEY support
Class Zonemaster::LDNS::RRList - Add XS methods new(), compare(), get(), string() - Add Perl methods do_compare(), to_string() - Add overloading support for some operations - Add and update documentation - Add and update unit tests Class Zonemaster::LDNS::Packet - Add Perl methods {authority,answer,additional,question}_rrlist() - Add and update documentation - Add and update unit tests Class Zonemaster::LDNS::RR - Update documentation
Co-authored-by: Marc van der Wal <[email protected]>
Deprecate case randomization feature
Fix compiler warning in XS code
Extend the functionalities of Zonemaster::LDNS::RRList
to_idn(): croak with locale-independent message
Proposition to set up github action
Check MANIFEST when PR is created
Fix missing module in CI
The change in #203 introduced a new() function in the Zonemaster::LDNS::RRList module. However, calling new() with a ref to an empty array as paramater caused the code to croak with the error message "List is empty". However, there are situations where it may be desirable to construct such an empty RRList. This commit ensures that Zonemaster::LDNS::RRList->new([]) works as intended.
Modify the XS code so that Zonemaster::LDNS::RRList::new() can be called with zero arguments, which is equivalent to passing it an empty arrayref.
Let’s make sure that new() and new([]) have the same semantics, and that the eq operator works correctly in both ways with these empty lists.
Allow construction of empty Zonemaster::LDNS::RRList objects
On platforms or systems where libraries might be installed somewhere else than the usual /usr/local/lib or /usr/lib, running perl Makefile.PL might fail to detect the location of the libraries. Fortunately, pkg-config can help us determine which appropriate CFLAGS and LDFLAGS to append to the compiler and linker command lines respectively, and ExtUtils::PkgConfig provides a simple wrapper around that command line utility. This unfortunately requires one more dependency, but that library is widely packaged across Linux distributions and FreeBSD. It’s packaged as follows: * on Alpine Linux: perl-extutils-pkgconfig; * on Debian-based OSes: libextutils-pkgconfig-perl; * on FreeBSD: p5-ExtUtils-PkgConfig; * on Red Hat-based OSes: perl-ExtUtils-PkgConfig.
Install ExtUtils::PkgConfig in the build container.
As a drive-by change, update the NSEC test in rr.t because the .se has started using ZONEMD.
Use ExtUtils::PkgConfig to discover the CFLAGS and LDFLAGS for external libraries
Compute hashed name from a Zonemaster::LDNS::RR::{NSEC3,NSEC3PARAM} record
Pull request #210 forgot to document the change in README.md. Let’s fix that before quickly before the next release.
Document new dependency on ExtUtils::PkgConfig
Co-authored-by: tgreenx <[email protected]>
Preparation for v2024.2 release (Zonemaster-LDNS)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
https://github.com/zonemaster/zonemaster/blob/develop/docs/internal/maintenance/ReleaseProcess-release.md#15-merge-develop-branch-into-master
How to test this PR
This PR needs no review.