Skip to content

Native parse_url accepts URLs and registry-based hosts that java.net.URI treats differently #7330

Open

Description

@comphead

Describe the bug

Native parse_url (and try_parse_url) runs by default (CometParseUrl is Compatible), but it accepts some URLs that Spark's java.net.URI treats differently, so Comet and Spark return different results.

Spark builds new URI(url) (ParseUrl in 3.5, UrlExpressionEvalUtils.getUrl in 4.x). On URISyntaxException it returns NULL, or raises INVALID_URL when failOnError (ANSI) is set, and HOST is URI.getHost. Comet's native path validates only spaces, {, }, <, > and control characters (has_invalid_uri_chars), then splits the URL with the RFC 3986 Appendix B regex. extract_host takes whatever follows the last @ in the authority.

Steps to reproduce

java.net.URI on JDK 17 for each input (local probe):

Input java.net.URI Spark parse_url Comet native (from reading parse_url.rs, not run)
http://a_b.com/p parses, getHost() is null (registry-based authority) HOST is NULL HOST is a_b.com
http://u@v@h/p parses, getHost() is null HOST is NULL HOST is h
http://h/%zz URISyntaxException NULL for every part, or INVALID_URL under ANSI returns the components, e.g. PATH is /%zz
http://h/p#a#b URISyntaxException NULL for every part, or INVALID_URL under ANSI returns the components, e.g. PATH is /p
SELECT parse_url('http://a_b.com/p', 'HOST'),
       parse_url('http://u@v@h/p', 'HOST'),
       parse_url('http://h/%zz', 'PATH'),
       parse_url('http://h/p#a#b', 'PATH');

The query needs to read the URLs from a table (or disable constant folding) for the expression to reach Comet.

Expected behavior

The same results as Spark: a null host for registry-based authorities, and NULL (or INVALID_URL under ANSI) for URLs that java.net.URI rejects, such as invalid % escapes and a second #. USERINFO of a registry-based authority may need the same check, since URI.getUserInfo is null there too.

Additional context

Found while reviewing #7329, which does not change this behavior: its new HTTP fast path matches the existing regex decomposition, so the gap is on main already. The current parse_url.sql fixtures use only well-formed URLs, so none of these inputs is covered.

cc @sunchao

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions