fix: unescape keys to handle escaped special characters

[paulirwin]

Describe the change The App Config SDK sends i.e. feature flag keys as URL-encoded, even though their API also supports them being not URL-encoded. This URL-decodes them to match the behavior of live Azure App Config.

Current behavior The app treats kv URLs like /kv/.appconfig.featureflag%2FMyFlag and /kv/.appconfig.featureflag/MyFlag as two distinct keys, i.e. creating a key with the %2F if encoded that way.

New behavior The app treats the two URLs as equivalent, as Azure App Config does.

Additional context N/A

[tnc1997]

Hi @paulirwin, thank you for your contribution!

After doing some testing, it looks like this causes issues with keys that contain a "+" character.

Input	Expected	Actual
.appconfig.featureflag/hello+world	.appconfig.featureflag/hello+world	.appconfig.featureflag/hello world
.appconfig.featureflag%2Fhello+world	.appconfig.featureflag/hello+world	.appconfig.featureflag/hello world
.appconfig.featureflag%2Fhello%2Bworld	.appconfig.featureflag/hello+world	.appconfig.featureflag/hello world

[tnc1997]

It looks like the decoding of the "/" character is a known unresolved issue in .NET according to this issue.

[paulirwin]

Well that is strange behavior with App Config! I've updated this PR to account for that and added unit tests.

[tnc1997]

Unfortunately I think that there might still be an issue with the changes.

Input	Expected	Actual
.appconfig.featureflag/ab%cd	.appconfig.featureflag/ab%cd	.appconfig.featureflag/ab�

[paulirwin]

We're starting to get into edge case territory. I highly doubt many people are using percent signs in their keys. I also highly doubt that at least the .NET SDK (if not others) is not URL-encoding those percents anyways. I could just replace a hard-coded list of URL-encoded tokens like %2B and %2F, but then there's likely going to be some we miss and break in other ways. My vote would be to proceed with this and let users report issues of their use cases that don't work.

[paulirwin]

Also note that while the App Config API allows feature flag keys with unencoded special characters like + and %cd, it does not display them as such in the portal as the flag name, leaving off everything after and including the special character. I don't believe this is an entirely supported scenario by Microsoft. Creating one manually with a special character seems to encode it.

I also confirmed that % and : are not supported characters in keys, even though the API allows them if unencoded and not matching certain encoded characters:

The emulator does not work today with feature flags and the .NET SDK, so this is a net incremental improvement over the current state that could be improved in the future if anyone runs into cases that are supported by Microsoft but do not work here.

[tnc1997]

I also confirmed that % and : are not supported characters in keys

Funnily enough after using the Azure Portal myself I was just about to reply with the same thing!

I hope you don't mind me pushing a small refactor to use Uri.UnescapeDataString instead of HttpUtility.UrlDecode.

[paulirwin]

Awesome, thanks. It appears that your latest commit removes the unit tests though. I think it would be valuable to keep those to ensure compatibility, adding test cases in the future if needed. The extension method could just simply delegate to Uri.UnescapeDataString.

[tnc1997]

I think it would be valuable to keep those to ensure compatibility

In theory I think that those unit tests would realistically only be testing the behaviour of Uri.UnescapeDataString.

I am working to add some unit tests that cover the logic applied to the handler inputs.

[paulirwin]

As long as those handler unit tests cover these key scenarios, I'm good with that.

[tnc1997]

If you're happy for those unit tests to be covered by #45 then I think that we can go ahead and merge this.