avoid Reflection in Quic #68189
avoid Reflection in Quic #68189
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThis is alternative to #67552 to avoid reflection in MsQuic configuration. Also this should be OK since neither SslStream nor Quic is shipped as OOB package. (note that current reflection use would make it also problematic) contributes to #67552
|
There was a problem hiding this comment.
LGTM, thanks!
BTW, could you also add SslClientHelloInfo ctor while you at it? We'll need it for the server options callback.
And I presume that ref source generator takes into account the settings and will not generate the API in ref with the next regeneration. In other words, I hope it doesn't need to be removed manually after each regeneration.
| public partial class SslStreamCertificateContext | ||
| { | ||
| internal readonly X509Certificate2 Certificate; | ||
| internal readonly X509Certificate2[] IntermediateCertificates; | ||
| public readonly X509Certificate2 Certificate; | ||
| public readonly X509Certificate2[] IntermediateCertificates; |
There was a problem hiding this comment.
Would it make sense to add another comment here mentioning that these fields are not part of the ref assembly and are not, in fact, reachable in user code?
I do not think it will work like this with the setup in the current PR. MatchingRefApiCompatBaseline.txt is for API compat verification only. It does not control ref source generator. Setting GenAPIExcludeApiList may help with the ref source generator. |
|
It is not clear to me how the GenAPIExcludeApiList should be used. I did not find any use of it for other examples where we have public API not present in ref assembly. cc: @ericstj @ViktorHofer |
In dotnet/runtime, the GenAPIExcludeAttributesList input to the task is this file: https://github.com/dotnet/runtime/blob/main/eng/DefaultGenApiDocIds.txt. We only specify attributes in it which should not appear in the contract but in the implementation. When API, apart from these attributes intentionally differs between the contract and the implementation (for which we have many cases, the lower you get in the graph) you would need to manually remove these after GenAPI generated/updated the reference project's source file. That said, if it makes sense, we could feed the |
|
@ViktorHofer GenAPI actually has both runtime/eng/resolveContract.targets Line 11 in 7667e41 GenAPIExcludeApiList is not in use, and may be used by source projects to specify which docIDs to skip when the developer runs /t:GenerateReferenceSource to generate the reference assembly source file. Here's an example of one used by WPF: https://github.com/dotnet/wpf/blob/5bf4a29ab3d4d6b69595304df2d68e5de37142b4/eng/WpfArcadeSdk/tools/GenApi/GlobalApiExclusions.txt
The format of API suppressions is different from GenAPI exclusions since API Compat does more checks for a given API. Adding exclusion of API by DocID to API compat makes sense though, and seems like a reasonable feature to add. It should be added to the new API compat here: https://github.com/dotnet/sdk/tree/main/src/Compatibility. I could imagine dotnet/runtime defining a common convention for such a file and passing it to both tools to minimize the things the developer needs to do when they explicitly want API excluded from the reference assembly. |
|
So if you wanted to make a static rule to exclude the API from the reference assembly you could do the following in System.Net.Security. Create a file in the project folder and give it a name like In the project set the property |
ghost
locked as resolved and limited conversation to collaborators
This is alternative to #67552 to avoid reflection in MsQuic configuration.
Unlike truly public API with documentation and maintenance forever this allows us to consume SslStream internals in Quic.
Since the API is not in ref assembly it is invisible in practice to whole world and does not need API review.
The original design of
SslStreamCertificateContexttried to prevent users from fiddling with its internals.Since the only motivation for #67552 is use in MsQuic this seems like better approach.
We can still add public API independently if there is good use case for it.
Also this should be OK since neither SslStream nor Quic is shipped as OOB package. (note that current reflection use would make it also problematic)
contributes to #67552
fixes #53507