Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match Presto's behavior for invalid UTF-8 in url_encode #11518

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kevinwilfong
Copy link
Contributor

Summary:
Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 13, 2024
Copy link

netlify bot commented Nov 13, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit ba7d5fe
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6737f1ab9b5fa70008b7ca64

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65856104

kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 13, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65856104

kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 14, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 14, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 14, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 14, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 15, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65856104

kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 15, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65856104

kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Nov 16, 2024
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65856104

Kevin Wilfong added 2 commits November 15, 2024 17:13
…facebookincubator#11488)

Summary:

Today the Presto URL functions rely on a regex to extract features of a URL.  Due to the limitations of
regexes this is not sufficient to validate that a URL is indeed valid. This leads to a number of cases
where Presto Java will return NULL due to an invalid URL and Velox will return some substring.

To address this I've implemented a parser for URIs based on the RFC 3986 spec which can both
validate a URL and extract features of it.

While testing this change I noticed a number of other discrepancies between Presto Java's and
Velox's implementations of these UDFs (mostly related to unescaping/decoding URLs or portions
thereof) that had been missed, likely due to the noise from the different handling of invalid URLs.

Those are addressed in this diff as well so that I could effectively test it.

Differential Revision: D65695961
…bator#11518)

Summary:

Presto Java converts the URL to a Java String before encoding it in url_encode. Java replaces bytes
in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox encodes invalid UTF-8 characters as is, which leads to differences in results from Java and 
C++.

This diff adds a check when encoding URLs for invalid UTF-8 characters and does the same
replacement as Java.

Differential Revision: D65856104
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65856104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants