Problem
In GitLabHttpFacade.Setup(), the library sets:
ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls11 | SecurityProtocolType.Tls12;
This has several issues:
1. Mutates process-wide global state
ServicePointManager.SecurityProtocol is a static property that affects all HTTP connections in the host process, not just those made by this library. A NuGet package should not modify global state that impacts other libraries and application code.
2. Prevents TLS 1.3 negotiation
The |= operator adds TLS 1.1 and 1.2 to the default, but never adds TLS 1.3. On .NET Framework where the default is Ssl3 | Tls, the resulting value becomes Ssl3 | Tls | Tls11 | Tls12 — TLS 1.3 is excluded. This is a problem for servers that require TLS 1.3.
On .NET Core/.NET 5+ the default is SystemDefault (value 0), so the |= is effectively a no-op, but it's still bad practice.
3. Adds deprecated TLS 1.1
TLS 1.1 has been deprecated since RFC 8996 (March 2021). Major browsers and services have dropped support for it.
4. Runs on every instantiation
The line executes each time a GitLabClient is constructed, repeatedly mutating global state.
Context
This line was likely added to support older .NET Framework versions (4.5–4.6.2) where SecurityProtocol defaults to Ssl3 | Tls and TLS 1.2 isn't enabled. However:
- .NET Framework 4.7+ defaults to TLS 1.2 via
SystemDefault
- .NET Framework 4.6.1 (the minimum
netstandard2.0-compatible framework) reached end of life in April 2022
- The library also explicitly targets
net48, which doesn't need this
Possible solutions
Option A: Remove the line entirely (recommended)
Remove the ServicePointManager.SecurityProtocol line and the unused System.Net using directive. Per Microsoft's guidance, the application — not individual libraries — should be responsible for configuring SecurityProtocol. Consumers on very old .NET Framework versions would need to configure this themselves, which is already standard practice.
Option B: Use HttpClientHandler.SslProtocols (per-connection)
Replace the global setting with SslProtocols on the HttpClientHandler, which only affects this library's connections. However, this property isn't consistently available across all netstandard2.0 implementations, and would require changes to the constructor logic.
Option C: Conditional compilation
Use #if directives to only apply the setting on .NET Framework targets, and skip it on .NET Core/.NET 5+ where SystemDefault handles it. This reduces the blast radius but still mutates global state on Framework.
Problem
In
GitLabHttpFacade.Setup(), the library sets:This has several issues:
1. Mutates process-wide global state
ServicePointManager.SecurityProtocolis a static property that affects all HTTP connections in the host process, not just those made by this library. A NuGet package should not modify global state that impacts other libraries and application code.2. Prevents TLS 1.3 negotiation
The
|=operator adds TLS 1.1 and 1.2 to the default, but never adds TLS 1.3. On .NET Framework where the default isSsl3 | Tls, the resulting value becomesSsl3 | Tls | Tls11 | Tls12— TLS 1.3 is excluded. This is a problem for servers that require TLS 1.3.On .NET Core/.NET 5+ the default is
SystemDefault(value 0), so the|=is effectively a no-op, but it's still bad practice.3. Adds deprecated TLS 1.1
TLS 1.1 has been deprecated since RFC 8996 (March 2021). Major browsers and services have dropped support for it.
4. Runs on every instantiation
The line executes each time a
GitLabClientis constructed, repeatedly mutating global state.Context
This line was likely added to support older .NET Framework versions (4.5–4.6.2) where
SecurityProtocoldefaults toSsl3 | Tlsand TLS 1.2 isn't enabled. However:SystemDefaultnetstandard2.0-compatible framework) reached end of life in April 2022net48, which doesn't need thisPossible solutions
Option A: Remove the line entirely (recommended)
Remove the
ServicePointManager.SecurityProtocolline and the unusedSystem.Netusing directive. Per Microsoft's guidance, the application — not individual libraries — should be responsible for configuringSecurityProtocol. Consumers on very old .NET Framework versions would need to configure this themselves, which is already standard practice.Option B: Use
HttpClientHandler.SslProtocols(per-connection)Replace the global setting with
SslProtocolson theHttpClientHandler, which only affects this library's connections. However, this property isn't consistently available across allnetstandard2.0implementations, and would require changes to the constructor logic.Option C: Conditional compilation
Use
#ifdirectives to only apply the setting on .NET Framework targets, and skip it on .NET Core/.NET 5+ whereSystemDefaulthandles it. This reduces the blast radius but still mutates global state on Framework.