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:
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:
and load it using V5 .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:
Uri.EscapeDataStrin
instead ofEscapeUriString
? https://stackoverflow.com/questions/4396598/whats-the-difference-between-escapeuristring-and-escapedatastring https://stackoverflow.com/questions/602642/server-urlencode-vs-httputility-urlencode/47877559#47877559 TL;DR: There is no valid reason to ever use Uri.EscapeUriString. If you need to percent-encode a string, always use Uri.EscapeDataString.