Ipv6 host validation support #3250
Conversation
| if (segment.empty() || segment.length() > 4){ | ||
| return false; | ||
| } | ||
| return std::all_of(segment.begin(), segment.end(), isxdigit); |
There was a problem hiding this comment.
Like all other functions from , the behavior of std::isxdigit is undefined if the argument's value is neither representable as unsigned char nor equal to EOF. To use these functions safely with plain chars (or signed chars), the argument should first be converted to unsigned char...
There was a problem hiding this comment.
Thanks for the comment. I have raised a change to address the corner case.
| @@ -49,7 +50,7 @@ namespace Aws | |||
| return false; | |||
| } | |||
|
|
|||
| return !std::any_of(labels.begin(), labels.end(), [](const Aws::String& label){ return !IsValidDnsLabel(label); }); | |||
| return !std::any_of(labels.begin(), labels.end(), [](const Aws::String& label){ return !IsValidDnsLabel(label); }) || aws_host_utils_is_ipv6(aws_byte_cursor_from_c_str(host.c_str()), false); | |||
There was a problem hiding this comment.
two questions:
-
why are we using the C function
aws_byte_cursor_from_c_strinstead of the cpp bindingByteBufFromCString. when using the CRT we should prefer C++ binding in the CRT as opposed to direct C calls. -
Why not create a CRT C++ binding for
aws_host_utils_is_ipv6instead of just calling the C function? We should create small and reusable bindings when we can. Calling the CRT C directly is something that we want to try to avoid.
Issue #, if available:
#3244
Description of changes:
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.