This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 140eb75a1be [fix](function) Bound parse_url's HOST, PORT and USERINFO
at the authority (#68370)
140eb75a1be is described below
commit 140eb75a1be32636d1313d815f1e3c3f74690ff4
Author: Arpit Jain <[email protected]>
AuthorDate: Wed Sep 23 21:54:10 2026 -0400
[fix](function) Bound parse_url's HOST, PORT and USERINFO at the authority
(#68370)
### What problem does this PR solve?
Issue Number: none
Problem Summary:
`parse_url` hunts for the ':' that separates the port, and the '@' that
separates the userinfo, across the whole url rather than across its
authority. Anything in the path, the query or the fragment gets picked
up as a separator:
```sql
select parse_url('http://example.com/a:b', 'HOST'); -- example.com/a
select parse_url('http://example.com/a:b', 'PORT'); -- b
select parse_url('http://example.com/a@b:c', 'USERINFO'); -- example.com/a
select parse_url('http://example.com?x=1', 'AUTHORITY'); -- example.com?x=1
```
A ':' inside a path segment is perfectly legal (RFC 3986 pchar), and
Hive answers these through `java.net.URL`, which gives host
`example.com`, no port and no userinfo for all four.
Both of our implementations do it, since the FE constant folding path in
`StringArithmetic.parseUrlRaw` mirrors the BE `UrlParser` closely.
`AUTHORITY` was the one already close to right, because it cut at the
first '/', so HOST, PORT and USERINFO now go through that same step and
it 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` now agree with `java.net.URL` on urls whose path, query or
fragment contains one of those characters.
### Check List (For Author)
- Test: Unit Test
- Cases added to `be/test/exprs/function/function_url_test.cpp` and to
`StringArithmeticTest.java`, one per affected part.
- I do not have a full BE or FE build on this machine, so I checked the
changed code on its own instead: `url_parser.cpp` compiled and driven
standalone, and the FE `parseUrl*` methods extracted and run under a
JDK. Both now agree with `java.net.URL` on every case in the new tests.
CI is the real check here, and I would rather say that than imply I ran
the suites.
- Behavior changed: Yes, the four results above. Existing cases in
`nereids_function_p0` and `fold_constant_string_arithmatic` have no ':'
or '@' outside the authority, so they should be unaffected.
- Does this need documentation: No
Signed-off-by: Arpit Jain <[email protected]>
---
be/src/util/url_parser.cpp | 61 +++++++++++-----------
be/src/util/url_parser.h | 4 ++
be/test/exprs/function/function_url_test.cpp | 30 +++++++++++
.../functions/executable/StringArithmetic.java | 41 +++++++--------
.../functions/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 5f0b591440e..0f44757e442 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 790a5c536cb..41c876daeaf 100644
--- a/be/src/util/url_parser.h
+++ b/be/src/util/url_parser.h
@@ -72,6 +72,10 @@ private:
// '#' 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 7fc20fb104c..5ede3ee9821 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<void>(check_function<DataTypeString, true>(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:[email protected]:80/a:b";), STRING("HOST")},
+ STRING("example.com")},
+ {{STRING("http://user:[email protected]:80/a:b";), STRING("PORT")},
STRING("80")},
+ {{STRING("http://user:[email protected]:80/a:b";),
STRING("USERINFO")},
+ STRING("user:pass")},
+ {{STRING("http://user:[email protected]:80/a:b";),
STRING("AUTHORITY")},
+ STRING("user:[email protected]:80")},
+ };
+
+ static_cast<void>(check_function<DataTypeString, true>(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 c2679c1f862..04c737197ed 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 @@ public class StringArithmetic {
}
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 @@ public class StringArithmetic {
}
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 @@ public class StringArithmetic {
}
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 fda0a6f31b6..ba23bd11027 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 @@ class StringArithmeticTest {
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:[email protected]:80/a:b";, "HOST",
"example.com");
+ assertParseUrl("http://user:[email protected]:80/a:b";, "PORT", "80");
+ assertParseUrl("http://user:[email protected]:80/a:b";, "USERINFO",
"user:pass");
+ assertParseUrl("http://user:[email protected]:80/a:b";, "AUTHORITY",
+ "user:[email protected]: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"));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
