From 3b15b508dd29cd402b5cf6297412982b6b300ab3 Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Mon, 28 Dec 2020 13:08:24 -0800 Subject: [PATCH] Convert URIParser test to googletest Note: Adding gtest caused the sanitizer to reorder a few build files. --- CMakeLists.txt | 70 ++++---- build_autogenerated.yaml | 25 +-- test/core/uri/BUILD | 2 +- test/core/uri/uri_parser_test.cc | 250 +++++++++++++++------------ tools/run_tests/generated/tests.json | 48 ++--- 5 files changed, 221 insertions(+), 174 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6d799fefc00..883e187b410 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -698,7 +698,6 @@ if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_c udp_server_test) endif() - add_dependencies(buildtests_c uri_parser_test) add_dependencies(buildtests_c useful_test) add_dependencies(buildtests_c varint_test) @@ -937,6 +936,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx tls_security_connector_test) add_dependencies(buildtests_cxx too_many_pings_test) add_dependencies(buildtests_cxx unknown_frame_bad_client_test) + add_dependencies(buildtests_cxx uri_parser_test) add_dependencies(buildtests_cxx window_overflow_bad_client_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx work_serializer_test) @@ -8024,36 +8024,6 @@ endif() endif() if(gRPC_BUILD_TESTS) -add_executable(uri_parser_test - test/core/uri/uri_parser_test.cc -) - -target_include_directories(uri_parser_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} -) - -target_link_libraries(uri_parser_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util - grpc - gpr - address_sorting - upb -) - - -endif() -if(gRPC_BUILD_TESTS) - add_executable(useful_test test/core/gpr/useful_test.cc ) @@ -15148,6 +15118,44 @@ target_link_libraries(unknown_frame_bad_client_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(uri_parser_test + test/core/uri/uri_parser_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(uri_parser_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(uri_parser_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + grpc + gpr + address_sorting + upb +) + + endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 34e3f94c5b7..b6ab05fe790 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -4629,18 +4629,6 @@ targets: - linux - posix - mac -- name: uri_parser_test - build: test - language: c - headers: [] - src: - - test/core/uri/uri_parser_test.cc - deps: - - grpc_test_util - - grpc - - gpr - - address_sorting - - upb - name: useful_test build: test language: c @@ -7778,6 +7766,19 @@ targets: corpus_dirs: - test/core/uri/uri_corpus maxlen: 128 +- name: uri_parser_test + gtest: true + build: test + language: c++ + headers: [] + src: + - test/core/uri/uri_parser_test.cc + deps: + - grpc_test_util + - grpc + - gpr + - address_sorting + - upb - name: window_overflow_bad_client_test gtest: true build: test diff --git a/test/core/uri/BUILD b/test/core/uri/BUILD index 40ffea95361..aaa9b68cd15 100644 --- a/test/core/uri/BUILD +++ b/test/core/uri/BUILD @@ -36,9 +36,9 @@ grpc_fuzzer( grpc_cc_test( name = "uri_parser_test", srcs = ["uri_parser_test.cc"], + external_deps = ["gtest"], language = "C++", deps = [ - "//:gpr", "//:grpc", "//test/core/util:grpc_test_util", ], diff --git a/test/core/uri/uri_parser_test.cc b/test/core/uri/uri_parser_test.cc index 4db01f3e210..ec79a6374df 100644 --- a/test/core/uri/uri_parser_test.cc +++ b/test/core/uri/uri_parser_test.cc @@ -23,81 +23,113 @@ #include "absl/strings/str_join.h" #include "absl/strings/str_split.h" +#include #include #include +#include #include "test/core/util/test_config.h" -static void test_succeeds( +using ::testing::ContainerEq; +using ::testing::Contains; +using ::testing::ElementsAre; +using ::testing::Pair; + +static void TestSucceeds( absl::string_view uri_text, absl::string_view scheme, absl::string_view authority, absl::string_view path, - const std::map& query_param_map, - const std::vector query_param_pairs, + const std::map& query_param_map, + const std::vector& query_param_pairs, absl::string_view fragment) { absl::StatusOr uri = grpc_core::URI::Parse(uri_text); - GPR_ASSERT(uri.ok()); - GPR_ASSERT(scheme == uri->scheme()); - GPR_ASSERT(authority == uri->authority()); - GPR_ASSERT(path == uri->path()); - // query param map - GPR_ASSERT(uri->query_parameter_map().size() == query_param_map.size()); - for (const auto& expected_kv : query_param_map) { - const auto it = uri->query_parameter_map().find(expected_kv.first); - GPR_ASSERT(it != uri->query_parameter_map().end()); - GPR_ASSERT(it->second == expected_kv.second); - } - // query param pairs - GPR_ASSERT(uri->query_parameter_pairs() == query_param_pairs); - GPR_ASSERT(fragment == uri->fragment()); + ASSERT_TRUE(uri.ok()); + EXPECT_EQ(scheme, uri->scheme()); + EXPECT_EQ(authority, uri->authority()); + EXPECT_EQ(path, uri->path()); + EXPECT_THAT(uri->query_parameter_map(), ContainerEq(query_param_map)); + EXPECT_THAT(uri->query_parameter_pairs(), ContainerEq(query_param_pairs)); + EXPECT_EQ(fragment, uri->fragment()); } -static void test_fails(absl::string_view uri_text) { +static void TestFails(absl::string_view uri_text) { absl::StatusOr uri = grpc_core::URI::Parse(uri_text); - GPR_ASSERT(!uri.ok()); + ASSERT_FALSE(uri.ok()); } -static void test_query_param_map() { - test_succeeds("http://localhost:8080/whatzit?mi_casa=su_casa", "http", - "localhost:8080", "/whatzit", {{"mi_casa", "su_casa"}}, - {{"mi_casa", "su_casa"}}, ""); - test_succeeds("http://localhost:8080/whatzit?1=2#buckle/my/shoe", "http", - "localhost:8080", "/whatzit", {{"1", "2"}}, {{"1", "2"}}, - "buckle/my/shoe"); - test_succeeds( +TEST(URIParserTest, BasicExamplesAreParsedCorrectly) { + TestSucceeds("http://www.google.com", "http", "www.google.com", "", {}, {}, + ""); + TestSucceeds("dns:///foo", "dns", "", "/foo", {}, {}, ""); + TestSucceeds("http://www.google.com:90", "http", "www.google.com:90", "", {}, + {}, ""); + TestSucceeds("a192.4-df:foo.coom", "a192.4-df", "", "foo.coom", {}, {}, ""); + TestSucceeds("a+b:foo.coom", "a+b", "", "foo.coom", {}, {}, ""); + TestSucceeds("zookeeper://127.0.0.1:2181/foo/bar", "zookeeper", + "127.0.0.1:2181", "/foo/bar", {}, {}, ""); + TestSucceeds("dns:foo.com#fragment-all-the-things", "dns", "", "foo.com", {}, + {}, "fragment-all-the-things"); + TestSucceeds("http://localhost:8080/whatzit?mi_casa=su_casa", "http", + "localhost:8080", "/whatzit", {{"mi_casa", "su_casa"}}, + {{"mi_casa", "su_casa"}}, ""); + TestSucceeds("http://localhost:8080/whatzit?1=2#buckle/my/shoe", "http", + "localhost:8080", "/whatzit", {{"1", "2"}}, {{"1", "2"}}, + "buckle/my/shoe"); +} + +TEST(URIParserTest, UncommonValidExamplesAreParsedCorrectly) { + TestSucceeds("scheme:path//is/ok", "scheme", "", "path//is/ok", {}, {}, ""); + TestSucceeds("http:?legit", "http", "", "", {{"legit", ""}}, {{"legit", ""}}, + ""); + TestSucceeds("unix:#this-is-ok-too", "unix", "", "", {}, {}, + "this-is-ok-too"); + TestSucceeds("http:?legit#twice", "http", "", "", {{"legit", ""}}, + {{"legit", ""}}, "twice"); + TestSucceeds("fake:///", "fake", "", "/", {}, {}, ""); +} + +TEST(URIParserTest, VariousKeyValueAndNonKVQueryParamsAreParsedCorrectly) { + TestSucceeds("http://foo/path?a&b=B&c=&#frag", "http", "foo", "/path", + {{"c", ""}, {"a", ""}, {"b", "B"}}, + {{"a", ""}, {"b", "B"}, {"c", ""}}, "frag"); +} + +TEST(URIParserTest, ParserTreatsFirstEqualSignAsKVDelimiterInQueryString) { + TestSucceeds( "http://localhost:8080/?too=many=equals&are=present=here#fragged", "http", "localhost:8080", "/", {{"are", "present=here"}, {"too", "many=equals"}}, {{"too", "many=equals"}, {"are", "present=here"}}, "fragged"); - test_succeeds("http://foo/path?a&b=B&c=&#frag", "http", "foo", "/path", - {{"c", ""}, {"a", ""}, {"b", "B"}}, - {{"a", ""}, {"b", "B"}, {"c", ""}}, "frag"); - test_succeeds("http://auth/path?foo=bar=baz&foobar===", "http", "auth", - "/path", {{"foo", "bar=baz"}, {"foobar", "=="}}, - {{"foo", "bar=baz"}, {"foobar", "=="}}, ""); + TestSucceeds("http://auth/path?foo=bar=baz&foobar===", "http", "auth", + "/path", {{"foo", "bar=baz"}, {"foobar", "=="}}, + {{"foo", "bar=baz"}, {"foobar", "=="}}, ""); } -static void test_repeated_query_param_pairs() { +TEST(URIParserTest, + RepeatedQueryParamsAreSupportedInOrderedPairsButDeduplicatedInTheMap) { absl::StatusOr uri = grpc_core::URI::Parse("http://foo/path?a=2&a=1&a=3"); - GPR_ASSERT(uri.ok()); - GPR_ASSERT(uri->query_parameter_map().size() == 1); - GPR_ASSERT(uri->query_parameter_map().find("a")->second == "3"); - std::vector expected( - {{"a", "2"}, {"a", "1"}, {"a", "3"}}); - GPR_ASSERT(uri->query_parameter_pairs() == expected); + ASSERT_TRUE(uri.ok()); + // The map stores the last found value. + ASSERT_THAT(uri->query_parameter_map(), ElementsAre(Pair("a", "3"))); + // Order matters for query parameter pairs + ASSERT_THAT(uri->query_parameter_pairs(), + ElementsAre(grpc_core::URI::QueryParam{"a", "2"}, + grpc_core::URI::QueryParam{"a", "1"}, + grpc_core::URI::QueryParam{"a", "3"})); } -static void test_query_param_validity_after_move() { +TEST(URIParserTest, QueryParamMapRemainsValiditAfterMovingTheURI) { grpc_core::URI uri_copy; { absl::StatusOr uri = grpc_core::URI::Parse("http://foo/path?a=2&b=1&c=3"); - GPR_ASSERT(uri.ok()); + ASSERT_TRUE(uri.ok()); uri_copy = std::move(*uri); } - GPR_ASSERT(uri_copy.query_parameter_map().find("a")->second == "2"); + // ASSERT_EQ(uri_copy.query_parameter_map().find("a")->second, "2"); + ASSERT_THAT(uri_copy.query_parameter_map(), Contains(Pair("a", "2"))); } -static void test_query_param_validity_after_copy() { +TEST(URIParserTest, QueryParamMapRemainsValidAfterCopyingTheURI) { // Since the query parameter map points to objects stored in the param pair // vector, this test checks that the param map pointers remain valid after // a copy. Ideally {a,m}san will catch this if there's a problem. @@ -106,56 +138,18 @@ static void test_query_param_validity_after_copy() { { absl::StatusOr del_uri = grpc_core::URI::Parse("http://foo/path?a=2&b=1&c=3"); - GPR_ASSERT(del_uri.ok()); + ASSERT_TRUE(del_uri.ok()); uri_copy = *del_uri; } - GPR_ASSERT(uri_copy.query_parameter_map().find("a")->second == "2"); - // testing copy constructor: + ASSERT_THAT(uri_copy.query_parameter_map(), Contains(Pair("a", "2"))); grpc_core::URI* del_uri2 = new grpc_core::URI(uri_copy); grpc_core::URI uri_copy2(*del_uri2); delete del_uri2; - GPR_ASSERT(uri_copy2.query_parameter_map().find("a")->second == "2"); + ASSERT_THAT(uri_copy2.query_parameter_map(), Contains(Pair("a", "2"))); } -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(argc, argv); - grpc_init(); - test_succeeds("http://www.google.com", "http", "www.google.com", "", {}, {}, - ""); - test_succeeds("dns:///foo", "dns", "", "/foo", {}, {}, ""); - test_succeeds("http://www.google.com:90", "http", "www.google.com:90", "", {}, - {}, ""); - test_succeeds("a192.4-df:foo.coom", "a192.4-df", "", "foo.coom", {}, {}, ""); - test_succeeds("a+b:foo.coom", "a+b", "", "foo.coom", {}, {}, ""); - test_succeeds("zookeeper://127.0.0.1:2181/foo/bar", "zookeeper", - "127.0.0.1:2181", "/foo/bar", {}, {}, ""); - test_succeeds("http://www.google.com?yay-i'm-using-queries", "http", - "www.google.com", "", {{"yay-i'm-using-queries", ""}}, - {{"yay-i'm-using-queries", ""}}, ""); - test_succeeds("dns:foo.com#fragment-all-the-things", "dns", "", "foo.com", {}, - {}, "fragment-all-the-things"); - test_succeeds("http:?legit", "http", "", "", {{"legit", ""}}, {{"legit", ""}}, - ""); - test_succeeds("unix:#this-is-ok-too", "unix", "", "", {}, {}, - "this-is-ok-too"); - test_succeeds("http:?legit#twice", "http", "", "", {{"legit", ""}}, - {{"legit", ""}}, "twice"); - test_succeeds("http://foo?bar#lol?", "http", "foo", "", {{"bar", ""}}, - {{"bar", ""}}, "lol?"); - test_succeeds("http://foo?bar#lol?/", "http", "foo", "", {{"bar", ""}}, - {{"bar", ""}}, "lol?/"); - test_succeeds("ipv6:[2001:db8::1%252]:12345", "ipv6", "", - "[2001:db8::1%2]:12345", {}, {}, ""); - test_succeeds("ipv6:[fe80::90%eth1.sky1]:6010", "ipv6", "", - "[fe80::90%eth1.sky1]:6010", {}, {}, ""); - test_succeeds("https://www.google.com/?a=1%26b%3D2&c=3", "https", - "www.google.com", "/", {{"c", "3"}, {"a", "1&b=2"}}, - {{"a", "1&b=2"}, {"c", "3"}}, ""); - // Artificial examples to show that embedded nulls are supported. - test_succeeds(std::string("unix-abstract:\0should-be-ok", 27), - "unix-abstract", "", std::string("\0should-be-ok", 13), {}, {}, - ""); - test_succeeds( +TEST(URIParserTest, AWSExternalAccountRegressionTest) { + TestSucceeds( "https://foo.com:5555/v1/" "token-exchange?subject_token=eyJhbGciO&subject_token_type=urn:ietf:" "params:oauth:token-type:id_token", @@ -165,24 +159,68 @@ int main(int argc, char** argv) { {{"subject_token", "eyJhbGciO"}, {"subject_token_type", "urn:ietf:params:oauth:token-type:id_token"}}, ""); - test_succeeds("http:?dangling-pct-%0", "http", "", "", - {{"dangling-pct-%0", ""}}, {{"dangling-pct-%0", ""}}, ""); - test_succeeds("unix-abstract:%00x", "unix-abstract", "", - std::string("\0x", 2), {}, {}, ""); - test_succeeds("x:y?%xx", "x", "", "y", {{"%xx", ""}}, {{"%xx", ""}}, ""); - test_succeeds("scheme:path//is/ok", "scheme", "", "path//is/ok", {}, {}, ""); - test_succeeds("fake:///", "fake", "", "/", {}, {}, ""); - test_fails("xyz"); - test_fails("http://foo?[bar]"); - test_fails("http://foo?x[bar]"); - test_fails("http://foo?bar#lol#"); - test_fails(""); - test_fails(":no_scheme"); - test_fails("0invalid_scheme:must_start/with?alpha"); - test_query_param_map(); - test_repeated_query_param_pairs(); - test_query_param_validity_after_move(); - test_query_param_validity_after_copy(); +} + +TEST(URIParserTest, NonKeyValueQueryStringsWork) { + TestSucceeds("http://www.google.com?yay-i'm-using-queries", "http", + "www.google.com", "", {{"yay-i'm-using-queries", ""}}, + {{"yay-i'm-using-queries", ""}}, ""); +} + +TEST(URIParserTest, IPV6StringsAreParsedCorrectly) { + TestSucceeds("ipv6:[2001:db8::1%252]:12345", "ipv6", "", + "[2001:db8::1%2]:12345", {}, {}, ""); + TestSucceeds("ipv6:[fe80::90%eth1.sky1]:6010", "ipv6", "", + "[fe80::90%eth1.sky1]:6010", {}, {}, ""); +} + +TEST(URIParserTest, PreviouslyReservedCharactersInUnrelatedURIPartsAreIgnored) { + // The '?' and '/' characters are not reserved delimiter characters in the + // fragment. See http://go/rfc/3986#section-3.5 + TestSucceeds("http://foo?bar#lol?", "http", "foo", "", {{"bar", ""}}, + {{"bar", ""}}, "lol?"); + TestSucceeds("http://foo?bar#lol?/", "http", "foo", "", {{"bar", ""}}, + {{"bar", ""}}, "lol?/"); +} + +TEST(URIParserTest, EncodedCharactersInQueryStringAreParsedCorrectly) { + TestSucceeds("https://www.google.com/?a=1%26b%3D2&c=3", "https", + "www.google.com", "/", {{"c", "3"}, {"a", "1&b=2"}}, + {{"a", "1&b=2"}, {"c", "3"}}, ""); +} + +TEST(URIParserTest, InvalidPercentEncodingsArePassedThrough) { + TestSucceeds("x:y?%xx", "x", "", "y", {{"%xx", ""}}, {{"%xx", ""}}, ""); + TestSucceeds("http:?dangling-pct-%0", "http", "", "", + {{"dangling-pct-%0", ""}}, {{"dangling-pct-%0", ""}}, ""); +} + +TEST(URIParserTest, NullCharactersInURIStringAreSupported) { + // Artificial examples to show that embedded nulls are supported. + TestSucceeds(std::string("unix-abstract:\0should-be-ok", 27), "unix-abstract", + "", std::string("\0should-be-ok", 13), {}, {}, ""); +} + +TEST(URIParserTest, EncodedNullsInURIStringAreSupported) { + TestSucceeds("unix-abstract:%00x", "unix-abstract", "", std::string("\0x", 2), + {}, {}, ""); +} + +TEST(URIParserTest, InvalidURIsResultInFailureStatuses) { + TestFails("xyz"); + TestFails("http://foo?[bar]"); + TestFails("http://foo?x[bar]"); + TestFails("http://foo?bar#lol#"); + TestFails(""); + TestFails(":no_scheme"); + TestFails("0invalid_scheme:must_start/with?alpha"); +} + +int main(int argc, char** argv) { + testing::InitGoogleTest(&argc, argv); + grpc::testing::TestEnvironment env(argc, argv); + grpc_init(); + auto result = RUN_ALL_TESTS(); grpc_shutdown(); - return 0; + return result; } diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 1582289676a..48bfe19dc9b 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -3079,30 +3079,6 @@ ], "uses_polling": true }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "uri_parser_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false, @@ -6255,6 +6231,30 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "uri_parser_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, { "args": [], "benchmark": false,