Skip to content

Stop harvesting S.IO.Pipes.AccessControl #52294

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

Merged
merged 2 commits into from
May 11, 2021
Merged

Stop harvesting S.IO.Pipes.AccessControl #52294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c11134d
Stop harvesting S.IO.Pipes.AccessControl
ViktorHofer May 5, 2021
7c22dc7
Use non shipping refs for Pipes typeforwards
ViktorHofer May 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/libraries/System.IO.Pipes.AccessControl/pkg/System.IO.Pipes.AccessControl.pkgproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@
<Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.props))" />
<ItemGroup>
<ProjectReference Include="..\ref\System.IO.Pipes.AccessControl.csproj">
<SupportedFramework>$(NetCoreAppCurrent)</SupportedFramework>
<SupportedFramework>net461;netcoreapp2.0;uap10.0.16299;$(AllXamarinFrameworks)</SupportedFramework>
Copy link
Contributor

Choose a reason for hiding this comment

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

For my own education, why was uap added again? I remember it was removed some time ago.

Copy link
Member Author

Choose a reason for hiding this comment

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

uap10.0.16299 supports netstandard2.0. As we are producing such an asset, we mark uap10.0.16299 as a supported framework.

</ProjectReference>
<ProjectReference Include="..\src\System.IO.Pipes.AccessControl.csproj" />
<HarvestIncludePaths Include="ref/net461;lib/net461;runtimes/win/lib/net461" />
<HarvestIncludePaths Include="ref/netstandard2.0;lib/netstandard2.0" />
<HarvestIncludePaths Include="runtimes/win/lib/netcoreapp2.1" />
<HarvestIncludePaths Include="ref/net5.0;lib/net5.0;runtimes/win/lib/net5.0" />
<!-- Exclude TFMs that aren't supported by the package anymore from validation. -->
<ExcludeHarvestedSupportedFramework Include="netcoreapp1.0;netcoreapp1.1;netcore50;uap10.0;net46" />
</ItemGroup>
Expand Down
10 changes: 0 additions & 10 deletions src/libraries/System.IO.Pipes.AccessControl/ref/System.IO.Pipes.AccessControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,4 @@ public void ResetAccessRule(System.IO.Pipes.PipeAccessRule rule) { }
public void SetAccessRule(System.IO.Pipes.PipeAccessRule rule) { }
public void SetAuditRule(System.IO.Pipes.PipeAuditRule rule) { }
}

public static class AnonymousPipeServerStreamAcl
Copy link
Contributor

Choose a reason for hiding this comment

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

I now remember these APIs were added to .NET Core with the intention of bringing ACL support to pipes, which already existed in .NET Framework.

There were other ACL cases in which I worked were the new extensions class needed to be consumable both from .NET Core and .NET Framework. It seems it's not the case with these two classes, so this change makes sense (within my limited understanding of the infra changes).

{
public static System.IO.Pipes.AnonymousPipeServerStream Create(System.IO.Pipes.PipeDirection direction, System.IO.HandleInheritability inheritability, int bufferSize, System.IO.Pipes.PipeSecurity? pipeSecurity) { throw null; }
}

public static class NamedPipeServerStreamAcl
{
public static System.IO.Pipes.NamedPipeServerStream Create(string pipeName, System.IO.Pipes.PipeDirection direction, int maxNumberOfServerInstances, System.IO.Pipes.PipeTransmissionMode transmissionMode, System.IO.Pipes.PipeOptions options, int inBufferSize, int outBufferSize, System.IO.Pipes.PipeSecurity? pipeSecurity, System.IO.HandleInheritability inheritability = System.IO.HandleInheritability.None, System.IO.Pipes.PipeAccessRights additionalAccessRights = default) { throw null; }
}
}
19 changes: 14 additions & 5 deletions src/libraries/System.IO.Pipes.AccessControl/ref/System.IO.Pipes.AccessControl.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
<TargetFrameworks>net5.0;netstandard2.0;net461</TargetFrameworks>
<Nullable>enable</Nullable>
</PropertyGroup>
<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
<PropertyGroup>
<IsPartialFacadeAssembly Condition="'$(TargetFramework)' == 'net461'">true</IsPartialFacadeAssembly>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was it necessary to add this net461 check (along with others)? It was my understanding that the *PipeServerStreamAcl APIs were only available in .NET Core.

Copy link
Member Author

Choose a reason for hiding this comment

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

We are generating a partial facade for System.IO.Pipes.AccessControl so that the package can be used on .NET Framework as well so that customers don't need to condition their PackageReferences for different TFMs. I can share more details (perhaps offline) in case you are interested.

</PropertyGroup>
<ItemGroup>
<Compile Include="System.IO.Pipes.AccessControl.cs" />
<Compile Include="System.IO.Pipes.AccessControl.netcoreapp.cs" Condition="'$(TargetFramework)' == 'net5.0'" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\System.Security.AccessControl\ref\System.Security.AccessControl.csproj" />
<ProjectReference Include="..\..\System.Security.Principal.Windows\ref\System.Security.Principal.Windows.csproj" />
<ProjectReference Include="..\..\System.IO.Pipes\ref\System.IO.Pipes.csproj" />
<ItemGroup Condition="'$(TargetFramework)' != 'net461'">
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.AccessControl\ref\System.Security.AccessControl.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Principal.Windows\ref\System.Security.Principal.Windows.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net5.0'">
<Reference Include="netstandard" />
<Reference Include="System.IO.Pipes" />
<Reference Include="System.Runtime" />
</ItemGroup>
</Project>
17 changes: 17 additions & 0 deletions src/libraries/System.IO.Pipes.AccessControl/ref/System.IO.Pipes.AccessControl.netcoreapp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// ------------------------------------------------------------------------------
// Changes to this file must follow the https://aka.ms/api-review process.
// ------------------------------------------------------------------------------

namespace System.IO.Pipes
{
public static class AnonymousPipeServerStreamAcl
{
public static System.IO.Pipes.AnonymousPipeServerStream Create(System.IO.Pipes.PipeDirection direction, System.IO.HandleInheritability inheritability, int bufferSize, System.IO.Pipes.PipeSecurity? pipeSecurity) { throw null; }
}
public static class NamedPipeServerStreamAcl
{
public static System.IO.Pipes.NamedPipeServerStream Create(string pipeName, System.IO.Pipes.PipeDirection direction, int maxNumberOfServerInstances, System.IO.Pipes.PipeTransmissionMode transmissionMode, System.IO.Pipes.PipeOptions options, int inBufferSize, int outBufferSize, System.IO.Pipes.PipeSecurity? pipeSecurity, System.IO.HandleInheritability inheritability = System.IO.HandleInheritability.None, System.IO.Pipes.PipeAccessRights additionalAccessRights = default) { throw null; }
}
}
9 changes: 9 additions & 0 deletions src/libraries/System.IO.Pipes.AccessControl/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="ArgumentOutOfRange_NeedValidPipeAccessRights" xml:space="preserve">
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reason for the addition of these resource strings? I don't see src changes in this PR, which is why I ask.

Copy link
Member Author

Choose a reason for hiding this comment

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

These strings are used by the added netstandard2.0 build.

<value>Invalid PipeAccessRights value.</value>
</data>
<data name="Argument_NonContainerInvalidAnyFlag" xml:space="preserve">
<value>This flag may not be set on a pipe.</value>
</data>
<data name="IO_IO_PipeBroken" xml:space="preserve">
<value>Pipe is broken.</value>
</data>
<data name="PlatformNotSupported_AccessControl" xml:space="preserve">
<value>Access Control List (ACL) APIs are part of resource management on Windows and are not supported on this platform.</value>
</data>
Expand Down
48 changes: 34 additions & 14 deletions src/libraries/System.IO.Pipes.AccessControl/src/System.IO.Pipes.AccessControl.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,46 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Nullable>enable</Nullable>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent);net5.0-windows;net5.0;netcoreapp3.1-windows;netcoreapp3.1;netstandard2.0-windows;netstandard2.0;net461-windows</TargetFrameworks>
<ExcludeCurrentNetCoreAppFromPackage>true</ExcludeCurrentNetCoreAppFromPackage>
</PropertyGroup>

<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
<PropertyGroup>
<GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetsWindows)' != 'true'">SR.PlatformNotSupported_AccessControl</GeneratePlatformNotSupportedAssemblyMessage>
<OmitResources Condition="'$(TargetsWindows)' == 'true'">true</OmitResources>
<IsPartialFacadeAssembly Condition="'$(TargetsWindows)' == 'true'">true</IsPartialFacadeAssembly>
<IsPartialFacadeAssembly Condition="'$(TargetsWindows)' == 'true' and '$(TargetFramework)' != 'netstandard2.0'">true</IsPartialFacadeAssembly>
<OmitResources Condition="'$(IsPartialFacadeAssembly)' == 'true'">true</OmitResources>
<!-- API Compat will fail for these since we use a fake System.IO.Pipes to forward to. -->
<RunApiCompat Condition="'$(IsPartialFacadeAssembly)' == 'true' and '$(TargetFramework)' != '$(NetCoreAppCurrent)-windows'">false</RunApiCompat>
</PropertyGroup>
<ItemGroup>
<Reference Include="System.Runtime" />

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
<Compile Include="System\IO\PipesAclExtensions.net461.cs" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0' and '$(TargetsWindows)' == 'true'">
<Compile Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System\IO\Pipes\PipeSecurity.cs" />
<Compile Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System\IO\Pipes\PipeAccessRights.cs" />
<Compile Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System\IO\Pipes\PipeAccessRule.cs" />
<Compile Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System\IO\Pipes\PipeAuditRule.cs" />
<Compile Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System\IO\Pipes\PipesAclExtensions.cs" />
</ItemGroup>
<!-- Include src projects during restore as TargetsWindows isn't set. -->
<ItemGroup Condition="'$(TargetsWindows)' == 'true' or '$(MSBuildRestoreSessionId)' != ''">
<ProjectReference Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System.IO.Pipes.csproj" SkipUseReferenceAssembly="true" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.AccessControl\src\System.Security.AccessControl.csproj" SkipUseReferenceAssembly="true" />

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETStandard'">
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.AccessControl\src\System.Security.AccessControl.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Principal.Windows\src\System.Security.Principal.Windows.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsWindows)' != 'true'">
<!-- Referencing the ref project directly as the src project doesn't have a RID less configuration. -->
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.AccessControl\ref\System.Security.AccessControl.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Principal.Windows\ref\System.Security.Principal.Windows.csproj" />
<Reference Include="System.IO.Pipes" />

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<Reference Include="System.Resources.ResourceManager" Condition="'$(TargetFramework)' == 'netcoreapp3.1'" />
<Reference Include="System.Runtime" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.AccessControl\src\System.Security.AccessControl.csproj" />

<!-- Compile against the shipping reference assembly when not targeting Windows. -->
<Reference Include="System.IO.Pipes" Condition="'$(TargetsWindows)' != 'true'" />
<!-- Compile against the shipping implementation assembly when targeting NetCoreAppCurrent-Windows. -->
<ProjectReference Include="$(LibrariesProjectRoot)System.IO.Pipes\src\System.IO.Pipes.csproj" Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)-windows'" SkipUseReferenceAssembly="true" />
<!-- Compile against the non-shipping reference assembly when targeting Windows for older .NETCoreApp tfms. -->
<ProjectReference Include="$(LibrariesProjectRoot)System.IO.Pipes\ref\System.IO.Pipes.csproj" Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)-windows' and '$(TargetsWindows)' == 'true'" />
</ItemGroup>
</Project>
21 changes: 21 additions & 0 deletions src/libraries/System.IO.Pipes.AccessControl/src/System/IO/PipesAclExtensions.net461.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using System.Security.AccessControl;

namespace System.IO.Pipes
{
public static class PipesAclExtensions
{
public static PipeSecurity GetAccessControl(PipeStream stream)
{
return stream.GetAccessControl();
}

public static void SetAccessControl(PipeStream stream, PipeSecurity pipeSecurity)
{
stream.SetAccessControl(pipeSecurity);
}
}
}
20 changes: 20 additions & 0 deletions src/libraries/System.IO.Pipes/ref/System.IO.Pipes.Internal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// ------------------------------------------------------------------------------
// Changes to this file must follow the https://aka.ms/api-review process.
// ------------------------------------------------------------------------------

namespace System.IO.Pipes
{
// The following types are incomplete, meant only to be the target of type forwards
public enum PipeAccessRights { }
public sealed class PipeAccessRule { }
public sealed class PipeAuditRule { }
public static class PipesAclExtensions { }
public class PipeSecurity { }

#if NET5_0_OR_GREATER
public static class AnonymousPipeServerStreamAcl { }
public static class NamedPipeServerStreamAcl { }
#endif
}
20 changes: 16 additions & 4 deletions src/libraries/System.IO.Pipes/ref/System.IO.Pipes.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
<!-- The non NetCoreAppCurrent tfms never ship, they are just used to produce reference assemblies,
to expose types for Pipes.AccessControl to forward to. -->
<TargetFrameworks>$(NetCoreAppCurrent);net5.0;netcoreapp3.1</TargetFrameworks>
<Nullable>enable</Nullable>
</PropertyGroup>
<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
<PropertyGroup>
<AssemblyVersion Condition="'$(TargetFramework)' == 'netcoreapp3.1'">4.1.2.0</AssemblyVersion>
<AssemblyVersion Condition="'$(TargetFramework)' == 'net5.0'">5.0.0.0</AssemblyVersion>
</PropertyGroup>
<ItemGroup>
<Compile Include="System.IO.Pipes.cs" />
<Compile Include="System.IO.Pipes.Internal.cs" Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)'" />
Copy link
Member

Choose a reason for hiding this comment

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

I guess this name is OK. I couldn't think of anything better.

Copy link
Member Author

Choose a reason for hiding this comment

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

I scanned existing patterns of that and found couple of ".Internal" files so I renamed it from ".Internals" to ".Internal".

Copy link
Contributor

Choose a reason for hiding this comment

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

Second attempt (not sure why GitHub deleted my original message):

"*TypeForwards.cs"? I don't mind the currently selected name, since it seems to align with other similar patterns.

Copy link
Member Author

Choose a reason for hiding this comment

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

Even thought that file is the input to generate type forwards, it doesn't contain any. It only lists non exposed APIs which is why we chose the "Internal" wording.

</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\System.Runtime\ref\System.Runtime.csproj" />
<ProjectReference Include="..\..\System.Security.Principal\ref\System.Security.Principal.csproj" />
<ItemGroup Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'">
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime\ref\System.Runtime.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Security.Principal\ref\System.Security.Principal.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)'">
<Reference Include="System.Runtime" />
<Reference Include="System.Security.Principal" />
</ItemGroup>
</Project>