cyberark / conjur-api-dotnet

.NET client for the CyberArk Conjur API
Apache License 2.0
15 stars 11 forks source link

fix: Switch WebUtility with Uri.EscapeDataString space to %20 (V4) #26

Closed DvirCyberArk closed 5 years ago

DvirCyberArk commented 6 years ago

Same as https://github.com/cyberark/conjur-api-dotnet/pull/25 But for V4 SDK.

What does this pull request do? Fix space issue while loading policy which contains space at URL

What background context can you provide? Until now we used WebUtility.UrlEncode(" ") which yields "+" instead of %20 resulting in Conjur Ngnix error and trimming of URL.

Where should the reviewer start? It seems like many classes to review, however the critical change was at the C'tor at path member. Notice that not all the URL get encoded, just the input variables. Therefore there will be no change to other special char like "?" or "&".

Look at the variableTest for a good example.

How should this be manually tested? Create simple policy with: policy, variable resources that contain spaces. e.g:

- !policy
   id: demo vault

- !variable
   id: demo variable

and load it using V4 .NET SDK and compare it to same policy loaded from CLI viewing result in UI and Conjur Ngnix log.

Link to build in Jenkins (if appropriate) https://jenkins.conjur.net/view/Conjur%205.x/job/cyberark--conjur-api-dotnet/

Questions:

dividedmind commented 5 years ago

Can we add tests to cover this change?

alexkalish commented 5 years ago

Closing due to inactivity.