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.

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

[release/8.0-staging] Do not allow a non-keyed service to be injected to a keyed parameter #106877

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 23, 2024

Backport of #105839 to release/8.0-staging plus a new feature switch for any compat issues.

See issue #102204.

/cc @steveharter

Customer Impact

  • Customer reported
  • Found internally

A customer found unexpected behavior where a keyed service is injected when instead an exception should be thrown.

Regression

  • Yes
  • No

Testing

The tests from version 9 were ported here; also a compat feature switch was added called "Microsoft.Extensions.DependencyInjection.AllowNonKeyedServiceInject" that when set to true will allow the old behavior in case a customer depends on it. That feature switch does not exist in v9.0.

Risk

Low. This is considered a breaking change since we now throw an exception; normally we don't add breaking changes in a service fix. However, consider:

  • Behavior is being corrected which due to the exception, may cause some customer applications to fail which in turn signifies a bug in their code. i.e. customers will now be made aware of any bugs\misuse here.
  • Any dependencies on this should be edge case and super rare. However, there is a feature switch added in this PR to keep the old behavior in v8.0.
  • For some customers, it is difficult to have well-defined, consistent behavior across v8 and v9 without this fix. Without this fix being ported to v8.0, this code for example would need to be reverted so they can work with the old behavior in v8.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

@steveharter steveharter self-assigned this Aug 23, 2024
@steveharter steveharter added this to the 8.0.x milestone Aug 23, 2024
@steveharter steveharter added the Servicing-consider Issue for next servicing release review label Aug 23, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 29, 2024
@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.10 Aug 29, 2024
@ericstj
Copy link
Member

ericstj commented Aug 29, 2024

Need to enable this package for shipping.

@ericstj ericstj merged commit c458057 into release/8.0-staging Sep 4, 2024
109 checks passed
@steveharter steveharter deleted the backport/pr-105839-to-release/8.0-staging branch September 4, 2024 16:19
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants