- Notifications
You must be signed in to change notification settings - Fork5.2k
Fix X509 test failures on Android#50301
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…ve, PSS padding in cert signatures)
…public key. Return early with null instead of crashing with an assert.
ghost commentedMar 26, 2021
Tagging subscribers to this area:@bartonjs,@vcsjones,@krwq,@GrabYourPitchforks Issue DetailsDisable tests on Android that use unsupported features (Brainpool curve, PSS padding in cert signatures). Disable outerloop test that checks against a validation status that is unsupported on Android. Handle JNI thread shutdown (Android API Level 21 is more strict about this than API Level 29). Fix some memory leaks. Disable tests that depend on OCSP when on older Android versions. Older versions of Android can include the same cert in the cert collection twice if it is also the trusted root. Handle this case and don't return the cert twice.
|
src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| <ItemGroupCondition="'$(UseAndroidCrypto)' == 'true'"> | ||
| <CompileInclude="X509StoreMutableTests.Android.cs" /> | ||
| </ItemGroup> | ||
| <ItemGroupCondition="'$(UseAndroidCrypto)' != 'true'"> | ||
| <CompileInclude="RevocationTests\DynamicRevocationTests.Default.cs" /> | ||
| </ItemGroup> | ||
| <ItemGroupCondition="'$(UseAndroidCrypto)' == 'true'"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Let's reuse existing groups when possible.
This comment has been minimized.
This comment has been minimized.
elinor-fung commentedApr 1, 2021
CI wasm failure is happening because of theadded Line 18 in6617892
This entire test assembly is actually disabled / marked as failing on browser: runtime/src/libraries/System.Security.Cryptography.X509Certificates/tests/AssemblyInfo.cs Line 7 in6617892
But the test discovery still goes through every test method and tries to determine the traits for each method. We could rework this so that it continues avoiding using any types that will hit PNSE on browser during test discovery, but it seems weird/wasteful that we're making our test runs go through bundling, sending off to helix, test discovery, and result reporting when we know the entire suite (and many of its dependencies) are not currently supported. @steveisok /@lewing is there a reason we want to have this suite disabled on browser through the |
steveisok commentedApr 1, 2021
@elinor-fung There was probably a point in time where we thought labeling |
src/libraries/System.Security.Cryptography.X509Certificates/tests/PublicKeyTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
bartonjs left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
PublicKeyTests.SupportsBrainpool shouldn't be needed.
bartonjs left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for humoring me.
ghost commentedApr 5, 2021
Hello@jkoritzinsky! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me ( |
…shim_mono# By Aaron Robinson (10) and others# Via GitHub* upstream/main: (108 commits) [mbr] Add Apple sample (dotnet#50740) make EstablishProxyTunnelAsync throw on failure status code from proxy (dotnet#50763) Improve RGB Min Max evaluation performance by using 2 or 3 comparison… (dotnet#50622) [mono] More domain cleanups (dotnet#50479) Fix Crossgen2 of PlatformDefaultMemberFunction methods and calls. (dotnet#50754) Disable EventSource generator in design-time builds (dotnet#50741) Fix X509 test failures on Android (dotnet#50301) Do not confuse fgDispBasicBlocks in fgMorphBlocks (dotnet#50703) Enforce 64KB event payload size limit on EventPipe (dotnet#50600) Reorganize CoreCLR native build to reduce CMake reconfigures when the build system is untouched (dotnet#49906) [mbr] Turn on hot reload for iOS, tvOS and MacCatalyst (dotnet#50458) improve connection scavenge logic by doing zero-byte read (dotnet#50545) Resolve call mdtokens when making tier 1 inline observations (dotnet#50675) Annotate APIs in System.Private.Xml (dotnet#49682) Support compiling against OpenSSL 3 headers Change Configuration.Json to use a regular Dictionary. (dotnet#50611) Remove unused BigNumFromBinary P/Invoke (dotnet#50670) Make Ninja the default CMake generator on Windows for the repo (dotnet#49715) [AppleAppBuilder] Entitlements to run tests on catalyst using the JIT (dotnet#50637) [mono] Fix delegate invokes to dynamic methods in mixed mode. (dotnet#50547) ...# Conflicts:#src/mono/dlls/mscordbi/CMakeLists.txt
Uh oh!
There was an error while loading.Please reload this page.
Disable tests on Android that use unsupported features (Brainpool curve, PSS padding in cert signatures).
Disable outerloop test that checks against a validation status that is unsupported on Android.
Handle JNI thread shutdown (Android API Level 21 is more strict about this than API Level 29).
Fix some memory leaks.
Disable tests that depend on OCSP when on older Android versions.
Older versions of Android can include the same cert in the cert collection twice if it is also the trusted root. Handle this case and don't return the cert twice.
Fixes#50565