Repository navigation
Conversation
Add tests for behaviours that the current suite does not check: - DER rejects a long-form length for a single-octet value - BER rejects a primitive value with indefinite length - NULL rejects non-empty contents - DER BOOLEAN rejects a non-canonical true; zero decodes as false; true serializes as 0xFF - an unsigned integer rejects a negative encoding - BIT STRING rejects eight unused bits - DER rejects an explicitly encoded DEFAULT value - BER decodes an empty constructed OCTET STRING - the integer byte collection's endIndex compares greater than startIndex - UTCTime accepts the lower year boundary 1950 - UTF8String created from a non-ASCII String keeps its UTF-8 bytes and converts back Also add the missing rejection assertion to testSequenceMustConsumeAllNodes: its do/catch accepted the success path without checking that an error is thrown. Each test passes on the current code and fails when the rule it checks is broken on purpose. All 133 tests pass; swift format lint --strict passes.
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
Thanks! This broadly looks really good, just a small structural note in the diff. I'll note that there are a few tests here that aren't actually DER-specific but they correctly reflect intended library behaviour so the tests are still good.
| } | ||
| } | ||
|
|
||
| extension ASN1Tests { |
There was a problem hiding this comment.
Avoid placing these in an extension, please ensure they are defined in the actual test class.
- Build the non-ASCII UTF8String check from a String value: a string literal selects init(stringLiteral:), so the check did not exercise init(_: String). - Assert the DEFAULT rejection on decodeDefault itself: wrapping the whole sequence let its unconsumed-node check satisfy the assertion with the same error code. - Keep the existing comment in testSequenceMustConsumeAllNodes.
as requested, the new tests are now defined directly in ASN1Tests and UTCTimeTests. and right about der scope. really focus was to enumerate requirements and see if all are covered. so failed to follow good structure. |
if youd rather not add any of these test cases, we simply drop this PR.
Disclamer: this pr is prepared by AI and is part of my wider campaignt to promote a format and protocol ive been developing: acceptance format. For now its shown use as a good internal tracking tool of semantic code coverage while also "testing" the test cases by giving only weight to test cases that can be shown to be able to fail. The goal of protocl is wider: a consumer-producer "contract" (legal,monetary not part of it) - you can see in the repo. I take responsibility for this PR and will answer review comments myself.
Who I am: https://github.com/ivmat
Details (AI-generated)
This PR adds tests only. No library code changes.
What it adds. 12 new test methods, 2 non-ASCII checks in
testUTF8StringCanCreateAString, and 1 missing rejection assertion intestSequenceMustConsumeAllNodes. The rules covered:01); zero decodes as false; true serializes as0xFFendIndexcompares greater thanstartIndexASN1UTF8Stringcreated from a non-ASCIIStringkeeps its UTF-8 bytes and converts backtestSequenceMustConsumeAllNodeskeeps its child-decoding check. Thedo/catchis replaced withXCTAssertThrowsError, because thedo/catchpassed when no error was thrown; the error-code check moves into theXCTAssertThrowsErrorhandler.How the gaps were found. Each rule was broken on purpose in a copy of the source, and the existing suite was run against each broken copy. The existing 121 tests pass under every one of those changes. Each test here passes on the current code and fails when its rule is broken. With this patch, all 133 tests pass on
main(917b472), andswift format lint --strictpasses.Example acceptance file for this PR: acceptance.toml for swift-asn1 1.7.3. It covers the whole public API, and each weighted claim links the mutation that shows its test can fail. The package was prepared for the first revision of this PR. The full package holds the mutants, the logs, and the clause-by-clause check against X.690 and RFC 7468.
Relation to open PRs.
Stringinitializer and the conversion back, so they complement Validate UTF8String contents when decoding #129 and don't overlap.[1, 39], so this PR leaves it out.Not in this PR. The study also found a few behaviours that differ from X.690 or RFC 7468, such as CR-only line ends in PEM and a
GeneralizedTimefractional-seconds encoding issue. Those will come as separate issues, each with a minimal reproduction, so this PR stays tests-only.