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.

Sign up for GitHub

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

Jump to bottom

Prototype: Don't Be A CA #2

Closed
wants to merge 43 commits into from
Closed

Prototype: Don't Be A CA #2

wants to merge 43 commits into from

Conversation

bartonjs
Copy link
Owner

@bartonjs bartonjs commented Jun 3, 2022 •
edited
Loading

No description provided.

vcsjones
vcsjones reviewed Jun 4, 2022
View reviewed changes
...s/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CrlBuilderTests.cs Outdated Show resolved Hide resolved
...s/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/CrlBuilderTests.cs Outdated Show resolved Hide resolved
...braries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/DontBeACA.cs Show resolved Hide resolved
...braries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/DontBeACA.cs
public static class DontBeACA
{
[Fact]
public static void EndToEnd()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this test is impressive... I am uncertain exactly what is under test.

Copy link
Owner Author

@bartonjs bartonjs Jun 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Largely it's just to be a place to try the scenario and leave the comments, like "don't trust the SAN"...

....Security.Cryptography.X509Certificates/tests/ExtensionsTests/SubjectAlternativeNameTests.cs Outdated
}
}

AssertMatches(ext, "foo", true);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a Theory to me.

...graphy/src/System/Security/Cryptography/X509Certificates/CertificateRevocationListBuilder.cs Outdated Show resolved Hide resolved
...graphy/src/System/Security/Cryptography/X509Certificates/CertificateRevocationListBuilder.cs Outdated
key = rsa;
generator = X509SignatureGenerator.CreateForRSA(rsa!, RSASignaturePadding);
break;
case Oids.EcPublicKey:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could maybe see throwing a NotSupportedException for Oids.Dsa.

.../System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/PublicKey.cs Outdated Show resolved Hide resolved
...ptography/src/System/Security/Cryptography/X509Certificates/SubjectAlternativeNameBuilder.cs Show resolved Hide resolved
...phy/src/System/Security/Cryptography/X509Certificates/X509SubjectAlternativeNameExtension.cs Show resolved Hide resolved
bartonjs pushed a commit that referenced this pull request Jul 1, 2022
@krwq @eiriktsarpalis
JSON contract customization (dotnet#70435)
897b2a3 
* Initial implementation for contract customization

fix build errors

Move converter rooting to DefaultJsonTypeInfoResolver so that it can be used standalone

Fix ConfigurationList.IsReadOnly

Minor refactorings (#1)

* Makes the following changes:

* Move singleton initialization for DefaultTypeInfoResolver behind a static property.
* Consolidate JsonSerializerContext & IJsonTypeInfoResolver values to a single field.
* Move reflection fallback logic away from JsonSerializerContext and into JsonSerializerOptions

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

* remove testing of removed field

Simplify the JsonTypeInfo.CreateObject implemenetation (#2)

* Simplify the JsonTypeInfo.CreateObject implemenetation

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs

Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>

Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>

Tests and fixes for JsonTypeInfoKind.None

TypeInfo type mismatch tests

Allow setting NumberHandling on JsonTypeInfoKind.None

test resolver returning wrong type of options

JsonTypeInfo/JsonPropertyInfo mutability tests

rename test file

Move default converter rooting responsibility behind DefaultJsonTypeInfoResolver (#3)

* Move default converter rooting responsibility behind DefaultJsonTypeInfoResolver

* address feedback

Add simple test for using JsonTypeInfo<T> with APIs directly taking it

fix and tests for untyped/typed CreateObject

uncomment test cases, remove todo

More tests and tiny fixes

Add a JsonTypeInfoResolver.Combine test for JsonSerializerContext (#4)

* Fix JsonTypeInfoResolver.Combine for JsonSerializerContext

* Break up failing test

Fix simple scenarios for combining contexts (#6)

* Fix simple scenarios for combining contexts

* feedback

JsonSerializerContext combine test with different camel casing

Remove unneeded virtual calls & branching when accessing Get & Set delegates (#7)

JsonPropertyInfo tests everything minus ShouldSerialize & NumberHandling

Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

throw InvalidOperationException rather than ArgumentNullException for source gen when PropertyInfo.Name is assigned through JsonPropertyInfoValues

tests for duplicated property names and JsonPropertyInfo.NumberHandling

Add tests for NumberHandling and failing tests for ShouldSerialize

disable the failing test and add extra checks

disable remainder of the failing ShouldSerialize tests, fix working one

Fix ShouldSerialize and IgnoreCondition interop

Add failing tests for CreateObject + parametrized constructors

Fix CreateObject support for JsonConstructor types (dotnet#10)

* Fix CreateObject support for JsonConstructor types

* address feedback

Make contexts more combinator friendly (#9)

* Make contexts more combinator friendly

* remove converter cache

* redesign test to account for JsonConstructorAttribute

* Combine unit tests

* address feedback

* Add acceptance tests for DataContract attributes & Specified pattern (dotnet#11)

* Add private field serialization acceptance test (dotnet#13)

* tests, PR feedback (dotnet#14)

* PR feedback and extra tests

* Shorten class name, remove incorrect check (not true for polimorphic cases)

* Make parameter matching for custom properties map property Name with parameter (dotnet#16)

* Test static initialization with JsonTypeInfo (dotnet#17)

* Fix test failures and proper fix this time (dotnet#18)

* Fix test failures and proper fix this time

* reinstate ActiveIssueAttribute

* PR feedback and adjust couple of tests which don't set TypeInfoResolver

* fix IAsyncEnumerable tests

* Lock JsonSerializerOptions in JsonTypeInfo.EnsureConfigured()

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
@bartonjs bartonjs closed this Aug 16, 2022
bartonjs pushed a commit that referenced this pull request Jan 4, 2024
@MichalStrehovsky
Clear IconHandle when we fold compares (dotnet#95383)
b95bccb 
Fixes dotnet#95367.

Relevant part of the JitDump:

```
Using `if true` assertions from pred BB02
Assertions in: #1
fgMorphTree BB04, STMT00021 (before)
               [000070] DA---------                         *  STORE_LCL_VAR ubyte  V10 tmp9
               [000057] -----------                         \--*  CAST      int <- ubyte <- int
               [000006] -----------                            \--*  EQ        int
               [000004] -----------                               +--*  LCL_VAR   ref    V02 tmp1          (last use)
               [000055] H----------                               \--*  CNS_INT(h) ref     'Frozen EmptyPartition`1<Int32> object'

Assertion prop for index #1 in BB04:
               [000006] -----------                         *  EQ        int
GenTreeNode creates assertion:
               [000070] DA---+-----                         *  STORE_LCL_VAR ubyte  V10 tmp9
In BB04 New Local Constant Assertion: V10 == [0000000000000001], index = #2

fgMorphTree BB04, STMT00021 (after)
               [000070] DA---+-----                         *  STORE_LCL_VAR ubyte  V10 tmp9
               [000055] H----+-----                         \--*  CNS_INT(h) int
```

The JitDump is unfinished because the compiler crashes when trying to dump the last line. Clearly, the `CNS_INT` is no longer a handle at that point because we just bashed it to a constant 1.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ChadNedzlek ChadNedzlek ChadNedzlek left review comments

@GrabYourPitchforks GrabYourPitchforks GrabYourPitchforks left review comments

@vcsjones vcsjones vcsjones left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bartonjs @vcsjones @GrabYourPitchforks @ChadNedzlek