From e7fbaea863e7f9f808701af1793b72919df3eee0 Mon Sep 17 00:00:00 2001 From: Kevin Wilfong Date: Thu, 5 Dec 2024 15:27:42 -0800 Subject: [PATCH] fix: Throw on white space in percent encoded values in url_decode (#11749) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/11749 url_decode currently accepts white space in percent encoded values, e.g. url_decode('% 1') will return the same results as url_decode('%01') Presto Java throws an exception in these cases. This change ensures that Velox does the same. Note that according to the standard the two characters following the percent should be hex digits, so this is consistent. Reviewed By: spershin Differential Revision: D66785876 fbshipit-source-id: 9d345e89deea71866116f439e1982975b86dcf7a --- velox/functions/prestosql/URLFunctions.h | 17 +++++++++-------- .../prestosql/tests/URLFunctionsTest.cpp | 2 ++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/velox/functions/prestosql/URLFunctions.h b/velox/functions/prestosql/URLFunctions.h index b57a0510e8f1..1fb1cd6befe4 100644 --- a/velox/functions/prestosql/URLFunctions.h +++ b/velox/functions/prestosql/URLFunctions.h @@ -163,14 +163,15 @@ FOLLY_ALWAYS_INLINE char decodeByte(const char* p, const char* end) { char* endptr; auto val = strtol(buf, &endptr, 16); - if (endptr != buf + 2) { - VELOX_USER_FAIL("Illegal hex characters in escape (%) pattern: {}", buf); - } - - if (val < 0) { - VELOX_USER_FAIL( - "Illegal hex characters in escape (%) pattern - negative value"); - } + VELOX_USER_CHECK( + endptr == buf + 2 && !std::isspace(buf[0]) && !std::isspace(buf[1]), + "Illegal hex characters in escape (%) pattern: {}", + buf); + + VELOX_USER_CHECK_GE( + val, + 0, + "Illegal hex characters in escape (%) pattern - negative value"); return val; } else { diff --git a/velox/functions/prestosql/tests/URLFunctionsTest.cpp b/velox/functions/prestosql/tests/URLFunctionsTest.cpp index 2e3f68082cbe..e217586c5535 100644 --- a/velox/functions/prestosql/tests/URLFunctionsTest.cpp +++ b/velox/functions/prestosql/tests/URLFunctionsTest.cpp @@ -649,6 +649,8 @@ TEST_F(URLFunctionsTest, urlDecode) { EXPECT_THROW(urlDecode("http%3A%2F%"), VeloxUserError); EXPECT_THROW(urlDecode("http%3A%2F%2H"), VeloxUserError); EXPECT_THROW(urlDecode("%-1"), VeloxUserError); + EXPECT_THROW(urlDecode("% 1"), VeloxUserError); + EXPECT_THROW(urlDecode("%1 "), VeloxUserError); } } // namespace