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

Support reading a byte array from a base64 string in JSON configuration #36034

Closed
thomaslevesque opened this issue Mar 25, 2020 · 6 comments · Fixed by #43150
Closed

Support reading a byte array from a base64 string in JSON configuration #36034

thomaslevesque opened this issue Mar 25, 2020 · 6 comments · Fixed by #43150

Comments

@thomaslevesque
Copy link
Member

Is your feature request related to a problem? Please describe.

Currently, the configuration binder can't read a byte array from a base64 string.

For instance, if I have this:

    public class MyOptions
    {
        public byte[] Key { get; set; }
    }
    ...
    services.Configure<MyOptions>(Configuration.GetSection("My"));

and my appsettings.json file specifies the value for Key as a base64 string, it silently fails to bind my property, which remains null.

Describe the solution you'd like

I'd like to be able to use base64 to specify byte array values in my configuration file. Instead, I have to declare the property as string, and parse it myself, which isn't very convenient.
(or write the values as a JSON array, which is even worse)

Describe alternatives you've considered

As mentioned above, declaring the property as a string and parsing it manually works, but isn't convenient.

Additional context

N/A

@analogrelay analogrelay transferred this issue from dotnet/extensions May 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels May 7, 2020
@analogrelay analogrelay added this to the Future milestone May 7, 2020
@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Jul 1, 2020
@safern
Copy link
Member

safern commented Oct 6, 2020

Thanks for the proposal @thomaslevesque.

The problem that I see with this is, how would we identify you have a base64 string to decode in the config file?

Would we have to look at the property that we're binding and if it's type is byte[] then try and decode the base64 string as a byte[]?

@thomaslevesque
Copy link
Member Author

Would we have to look at the property that we're binding and if it's type is byte[] then try and decode the base64 string as a byte[]?

Yes, at least that's what I had in mind.

Actually, I probably shouldn't have mentioned JSON in the issue description, since it's not actually specific to JSON configuration. It should apply to any configuration source, since AFAIK the configuration binder doesn't care where the values come from, it just sees key/value pairs.

@thomaslevesque
Copy link
Member Author

Looks like TryConvertValue uses the TypeConverter for the property type to parse the value, but there isn't a specific TypeConverter for byte[].
It could easily be solved by adding a special case to TryConvertValue. Something like this:

if (type == typeof(byte[]))
{
    if (value is null)
        return null;
    return Convert.FromBase64String(value);
}

(with appropriate error handling, of course)

I can submit a PR if this approach is acceptable.

@safern
Copy link
Member

safern commented Oct 6, 2020

I don't have an objection to doing that.

cc: @maryamariyan @davidfowl ?

@thomaslevesque
Copy link
Member Author

@safern I sent a PR for this: #43150

@davidfowl
Copy link
Member

This seems reasonable.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants