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

Dispose MemoryStream in DataProtectorTokenProvider #9339

Closed
wants to merge 3 commits into from

Conversation

CBaud
Copy link
Contributor

@CBaud CBaud commented Apr 12, 2019

Addresses #5810

@davidfowl
Copy link
Member

Dispose is a noop. Not sure this is required

@gfoidl
Copy link
Member

gfoidl commented Apr 14, 2019

I wouldn't dispose the MS here. It makes the code* bigger, without any real win, and smaller code is preferable.

The updated internal state is not relevant, as the MS is used only locally and is not shared to other consumers.

* not only the C# code, but also the IL, and hence it has more work for the JIT.

@natemcmaster natemcmaster added the area-identity Includes: Identity and providers label Apr 15, 2019
@HaoK
Copy link
Member

HaoK commented Apr 15, 2019

Agree to close @ajcvickers ?

@ajcvickers ajcvickers closed this Apr 15, 2019
@ajcvickers ajcvickers added the ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. label Apr 15, 2019
rynowak pushed a commit that referenced this pull request Jun 1, 2019
Fixes: #8493
Fixes: #9632
Fixes: #9339
Fixes: #8385
Fixes: 10077

This fix adds support for type converters as well as a few other minor
things we were missing from binding. The key thing about supporting
conversions is that we new can support arbitrary types with `@bind`.
This means you can use it with generics, which is something many users
have tried.

Along with type converters we get Guid and TimeSpan from the BCL. The
BCL also includes converters for types we're less interested in like
`short`.
rynowak pushed a commit that referenced this pull request Jun 3, 2019
Fixes: #8493
Fixes: #9632
Fixes: #9339
Fixes: #8385
Fixes: 10077

This fix adds support for type converters as well as a few other minor
things we were missing from binding. The key thing about supporting
conversions is that we new can support arbitrary types with `@bind`.
This means you can use it with generics, which is something many users
have tried.

Along with type converters we get Guid and TimeSpan from the BCL. The
BCL also includes converters for types we're less interested in like
`short`.
rynowak added a commit that referenced this pull request Jun 5, 2019
* Add support for TypeConverter

Fixes: #8493
Fixes: #9632
Fixes: #9339
Fixes: #8385
Fixes: 10077

This fix adds support for type converters as well as a few other minor
things we were missing from binding. The key thing about supporting
conversions is that we new can support arbitrary types with `@bind`.
This means you can use it with generics, which is something many users
have tried.

Along with type converters we get Guid and TimeSpan from the BCL. The
BCL also includes converters for types we're less interested in like
`short`.

* Use correct NumberStyles

* Fix culture

* Core check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-identity Includes: Identity and providers ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants