From 757a149344b2106a993ef6d5d0f7243672177f15 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Tue, 22 Sep 2026 01:11:07 -0400 Subject: [PATCH] [fix](function) Bound parse_url's HOST, PORT and USERINFO at the authority ### What problem does this PR solve? Issue Number: none Problem Summary: parse_url searches for the ':' that separates the port, and the '@' that separates the userinfo, across the whole url rather than across its authority component, so a ':' or an '@' in the path, the query or the fragment is read as a separator. parse_url('http://example.com/a:b', 'HOST') returns example.com/a and the PORT of the same url returns b. Both the BE UrlParser and the FE StringArithmetic constant-folding mirror have it. HOST, PORT and USERINFO now go through the same authority step AUTHORITY used, which is extended to stop at '?' and '#' as well. ### Release note parse_url no longer treats a ':' or an '@' outside the authority as a port or userinfo separator, so HOST, PORT, USERINFO and AUTHORITY agree with java.net.URL for urls whose path, query or fragment contains one of those characters. ### Check List (For Author) - Test: Unit Test - Behavior changed: Yes, as described in the release note - Does this need documentation: No Signed-off-by: Arpit Jain --- be/src/util/url_parser.cpp | 61 +++++++++---------- be/src/util/url_parser.h | 4 ++ be/test/exprs/function/function_url_test.cpp | 30 +++++++++ .../executable/StringArithmetic.java | 41 ++++++------- .../executable/StringArithmeticTest.java | 34 +++++++++++ 5 files changed, 117 insertions(+), 53 deletions(-) diff --git a/be/src/util/url_parser.cpp b/be/src/util/url_parser.cpp index 5f0b591440e83a..0f44757e4424f9 100644 --- a/be/src/util/url_parser.cpp +++ b/be/src/util/url_parser.cpp @@ -73,6 +73,21 @@ bool UrlParser::find_query_component(const StringRef& url, StringRef* query) { return true; } +StringRef UrlParser::find_authority(const StringRef& protocol_end) { + // The authority component runs from the end of '://' up to the first '/', '?' or '#', + // whichever comes first. + int32_t end_pos = _s_slash_search.search(&protocol_end); + int32_t question_pos = _s_question_search.search(&protocol_end); + if (question_pos >= 0 && (end_pos < 0 || question_pos < end_pos)) { + end_pos = question_pos; + } + int32_t hash_pos = _s_hash_search.search(&protocol_end); + if (hash_pos >= 0 && (end_pos < 0 || hash_pos < end_pos)) { + end_pos = hash_pos; + } + return protocol_end.substring(0, end_pos); +} + bool UrlParser::parse_url(const StringRef& url, UrlPart part, StringRef* result) { result->data = nullptr; result->size = 0; @@ -90,9 +105,7 @@ bool UrlParser::parse_url(const StringRef& url, UrlPart part, StringRef* result) switch (part) { case AUTHORITY: { - // Find first '/'. - int32_t end_pos = _s_slash_search.search(&protocol_end); - *result = protocol_end.substring(0, end_pos); + *result = find_authority(protocol_end); break; } @@ -127,31 +140,21 @@ bool UrlParser::parse_url(const StringRef& url, UrlPart part, StringRef* result) } case HOST: { - // Find '@'. - int32_t start_pos = _s_at_search.search(&protocol_end); + StringRef authority = find_authority(protocol_end); + // Find '@' to strip out the userinfo. + int32_t start_pos = _s_at_search.search(&authority); if (start_pos < 0) { - // No '@' was found, i.e., no user:pass info was given, start after _s_protocol. + // No '@' was found, i.e., no user:pass info was given. start_pos = 0; } else { // Skip '@'. start_pos += _s_at.size; } - StringRef host_start = protocol_end.substring(start_pos); - // Find first '?'. - int32_t query_start_pos = _s_question_search.search(&host_start); - if (query_start_pos > 0) { - host_start = host_start.substring(0, query_start_pos); - } + StringRef host_start = authority.substring(start_pos); // Find ':' to strip out port. int32_t end_pos = _s_colon_search.search(&host_start); - - if (end_pos < 0) { - // No port was given. search for '/' to determine ending position. - end_pos = _s_slash_search.search(&host_start); - } - *result = host_start.substring(0, end_pos); break; } @@ -188,31 +191,33 @@ bool UrlParser::parse_url(const StringRef& url, UrlPart part, StringRef* result) } case USERINFO: { + StringRef authority = find_authority(protocol_end); // Find '@'. - int32_t end_pos = _s_at_search.search(&protocol_end); + int32_t end_pos = _s_at_search.search(&authority); if (end_pos < 0) { // Indicate no user and pass were given. return false; } - *result = protocol_end.substring(0, end_pos); + *result = authority.substring(0, end_pos); break; } case PORT: { - // Find '@'. - int32_t start_pos = _s_at_search.search(&protocol_end); + StringRef authority = find_authority(protocol_end); + // Find '@' to strip out the userinfo. + int32_t start_pos = _s_at_search.search(&authority); if (start_pos < 0) { - // No '@' was found, i.e., no user:pass info was given, start after _s_protocol. + // No '@' was found, i.e., no user:pass info was given. start_pos = 0; } else { // Skip '@'. start_pos += _s_at.size; } - StringRef host_start = protocol_end.substring(start_pos); + StringRef host_start = authority.substring(start_pos); // Find ':' to strip out port. int32_t end_pos = _s_colon_search.search(&host_start); //no port found @@ -220,13 +225,7 @@ bool UrlParser::parse_url(const StringRef& url, UrlPart part, StringRef* result) return false; } - StringRef port_start_str = host_start.substring(end_pos + _s_colon.size); - int32_t port_end_pos = _s_slash_search.search(&port_start_str); - //if '/' not found, try to find '?' - if (port_end_pos < 0) { - port_end_pos = _s_question_search.search(&port_start_str); - } - *result = port_start_str.substring(0, port_end_pos); + *result = host_start.substring(end_pos + _s_colon.size); break; } diff --git a/be/src/util/url_parser.h b/be/src/util/url_parser.h index 790a5c536cb090..41c876daeaf797 100644 --- a/be/src/util/url_parser.h +++ b/be/src/util/url_parser.h @@ -72,6 +72,10 @@ class UrlParser { // '#' comes before the first '?' because the '?' then belongs to the fragment. static bool find_query_component(const StringRef& url, StringRef* query); + // Returns the authority component of url, which has already had its protocol stripped. + // The authority ends at the first '/', '?' or '#'. + static StringRef find_authority(const StringRef& protocol_end); + // Constants representing parts of a URL. static const StringRef _s_url_authority; static const StringRef _s_url_file; diff --git a/be/test/exprs/function/function_url_test.cpp b/be/test/exprs/function/function_url_test.cpp index 7fc20fb104cefc..5ede3ee9821cc2 100644 --- a/be/test/exprs/function/function_url_test.cpp +++ b/be/test/exprs/function/function_url_test.cpp @@ -161,4 +161,34 @@ TEST(FunctionUrlTEST, ParseUrlQueryTest) { static_cast(check_function(func_name, input_types, data_set)); } +TEST(FunctionUrlTEST, ParseUrlAuthorityTest) { + std::string func_name = "parse_url"; + InputTypeSet input_types = {PrimitiveType::TYPE_VARCHAR, PrimitiveType::TYPE_VARCHAR}; + + DataSet data_set = { + // A ':' in the path is not a port separator, and an '@' in the path is not a + // userinfo separator. + {{STRING("http://example.com/a:b"), STRING("HOST")}, STRING("example.com")}, + {{STRING("http://example.com/a:b"), STRING("PORT")}, Null()}, + {{STRING("http://example.com/a:b"), STRING("AUTHORITY")}, STRING("example.com")}, + {{STRING("http://example.com/a@b:c"), STRING("HOST")}, STRING("example.com")}, + {{STRING("http://example.com/a@b:c"), STRING("USERINFO")}, Null()}, + // A ':' in the query or the fragment is not a port separator either. + {{STRING("http://example.com/p?r=http:8080"), STRING("PORT")}, Null()}, + {{STRING("http://example.com#f:1"), STRING("HOST")}, STRING("example.com")}, + {{STRING("http://example.com#f:1"), STRING("PORT")}, Null()}, + {{STRING("http://example.com?x=1"), STRING("AUTHORITY")}, STRING("example.com")}, + // A real port and a real userinfo are still returned. + {{STRING("http://user:pass@example.com:80/a:b"), STRING("HOST")}, + STRING("example.com")}, + {{STRING("http://user:pass@example.com:80/a:b"), STRING("PORT")}, STRING("80")}, + {{STRING("http://user:pass@example.com:80/a:b"), STRING("USERINFO")}, + STRING("user:pass")}, + {{STRING("http://user:pass@example.com:80/a:b"), STRING("AUTHORITY")}, + STRING("user:pass@example.com:80")}, + }; + + static_cast(check_function(func_name, input_types, data_set)); +} + } // namespace doris diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmetic.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmetic.java index c2679c1f862c8a..04c737197edae5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmetic.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmetic.java @@ -997,7 +997,14 @@ private static String substringEnd(String value, int end) { } private static String parseUrlAuthority(String protocolEnd) { - return substringEnd(protocolEnd, protocolEnd.indexOf('/')); + // The authority component runs from the end of "://" up to the first '/', '?' or '#', + // whichever comes first. + int endPos = firstIndexOf(protocolEnd, '?', '#'); + int slashPos = protocolEnd.indexOf('/'); + if (slashPos >= 0 && (endPos < 0 || slashPos < endPos)) { + endPos = slashPos; + } + return substringEnd(protocolEnd, endPos); } private static String parseUrlPath(String protocolEnd) { @@ -1019,18 +1026,11 @@ private static String parseUrlFile(String protocolEnd) { } private static String parseUrlHost(String protocolEnd) { - int startPos = protocolEnd.indexOf('@'); + String authority = parseUrlAuthority(protocolEnd); + int startPos = authority.indexOf('@'); startPos = startPos < 0 ? 0 : startPos + 1; - String hostStart = protocolEnd.substring(startPos); - int queryStartPos = hostStart.indexOf('?'); - if (queryStartPos > 0) { - hostStart = hostStart.substring(0, queryStartPos); - } - int endPos = hostStart.indexOf(':'); - if (endPos < 0) { - endPos = hostStart.indexOf('/'); - } - return substringEnd(hostStart, endPos); + String hostStart = authority.substring(startPos); + return substringEnd(hostStart, hostStart.indexOf(':')); } private static String parseUrlQuery(String protocolEnd) { @@ -1057,27 +1057,24 @@ private static String parseUrlRef(String protocolEnd) { } private static String parseUrlUserInfo(String protocolEnd) { - int endPos = protocolEnd.indexOf('@'); + String authority = parseUrlAuthority(protocolEnd); + int endPos = authority.indexOf('@'); if (endPos < 0) { return null; } - return protocolEnd.substring(0, endPos); + return authority.substring(0, endPos); } private static String parseUrlPort(String protocolEnd) { - int startPos = protocolEnd.indexOf('@'); + String authority = parseUrlAuthority(protocolEnd); + int startPos = authority.indexOf('@'); startPos = startPos < 0 ? 0 : startPos + 1; - String hostStart = protocolEnd.substring(startPos); + String hostStart = authority.substring(startPos); int endPos = hostStart.indexOf(':'); if (endPos < 0) { return null; } - String portStart = hostStart.substring(endPos + 1); - int portEndPos = portStart.indexOf('/'); - if (portEndPos < 0) { - portEndPos = portStart.indexOf('?'); - } - return substringEnd(portStart, portEndPos); + return hostStart.substring(endPos + 1); } /** diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmeticTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmeticTest.java index fda0a6f31b6e17..ba23bd1102716f 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmeticTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/executable/StringArithmeticTest.java @@ -120,6 +120,40 @@ void testExtractUrlParameterStopsAtFragment() { assertExtractUrlParameter("http://h/p?k1=aa&k2=bb#f", "k2", "bb"); } + @Test + void testParseUrlStopsAtTheAuthority() { + // A ':' in the path is not a port separator, and an '@' in the path is not a + // userinfo separator. + assertParseUrl("http://example.com/a:b", "HOST", "example.com"); + assertParseUrlIsNull("http://example.com/a:b", "PORT"); + assertParseUrl("http://example.com/a:b", "AUTHORITY", "example.com"); + assertParseUrl("http://example.com/a@b:c", "HOST", "example.com"); + assertParseUrlIsNull("http://example.com/a@b:c", "USERINFO"); + // A ':' in the query or the fragment is not a port separator either. + assertParseUrlIsNull("http://example.com/p?r=http:8080", "PORT"); + assertParseUrl("http://example.com#f:1", "HOST", "example.com"); + assertParseUrlIsNull("http://example.com#f:1", "PORT"); + assertParseUrl("http://example.com?x=1", "AUTHORITY", "example.com"); + // A real port and a real userinfo are still returned. + assertParseUrl("http://user:pass@example.com:80/a:b", "HOST", "example.com"); + assertParseUrl("http://user:pass@example.com:80/a:b", "PORT", "80"); + assertParseUrl("http://user:pass@example.com:80/a:b", "USERINFO", "user:pass"); + assertParseUrl("http://user:pass@example.com:80/a:b", "AUTHORITY", + "user:pass@example.com:80"); + } + + private void assertParseUrl(String url, String part, String expected) { + Expression result = StringArithmetic.parseurl( + new StringLiteral(url), new StringLiteral(part)); + Assertions.assertEquals(expected, ((StringLikeLiteral) result).getValue()); + } + + private void assertParseUrlIsNull(String url, String part) { + Expression result = StringArithmetic.parseurl( + new StringLiteral(url), new StringLiteral(part)); + Assertions.assertTrue(result instanceof NullLiteral, url + " " + part); + } + private void assertParseUrlQuery(String url, String expected) { Expression result = StringArithmetic.parseurl( new StringLiteral(url), new StringLiteral("QUERY"));