gene's encode_uri added/adapted to python - #408
Conversation
Signed-off-by: Amber Torrise <ambertorrise@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #408 +/- ##
==========================================
+ Coverage 83.06% 84.82% +1.75%
==========================================
Files 49 49
Lines 2982 3011 +29
==========================================
+ Hits 2477 2554 +77
+ Misses 505 457 -48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: ATorrise <ambertorrise@gmail.com>
Signed-off-by: ATorrise <ambertorrise@gmail.com>
Signed-off-by: ATorrise <ambertorrise@gmail.com>
| if next_char in _USS_CHARS_TO_ENCODE: | ||
| encoded_path.append(_USS_CHARS_TO_ENCODE[next_char]) |
There was a problem hiding this comment.
Since % is included in the map of _USS_CHARS_TO_ENCODE, if a URI contains characters that are already encoded such as %20 for space, it will get converted to %2520 which is not valid.
Not sure if it's in scope to fix in this PR, given that the implementation in the Node SDK seems to have the same problem, and the input passed to this method should typically not be encoded yet. Thoughts @zFernand0 @traeok?
There was a problem hiding this comment.
Would this mean that merging would cause a regression in behavior given the scope of expected URIs?
I think we could address this separately since the Node.js SDK already suffers from the same problem - but since we've now identified it can be an issue, maybe we can proactively add a note for developers to clarify that pre-encoded URIs are not accepted (until this is resolved).
There was a problem hiding this comment.
Filed this as a separate issue (we can label as low priority) #413
What It Does
Ports the URI-encoding fixes from Gene's Zowe CLI PR zowe/zowe-cli#2758 to the Python SDK, adding EncodeUri logic
_encode_uri_path_for_uss) and ds(with ds being less restrictive)(_encode_uri_path_for_zos)Console.__init__put the literal string "defcn" (the default console name) right into the base URL, then str.replace replaced every instance of defcn in the string, so if for some reason there was a valid defcn in the URL, that would get replaced and break the URLHow to Test
Review Checklist
I certify that I have:
Additional Comments