Skip to content

Commit

Permalink
fix: Throw on white space in percent encoded values in url_decode (#1…
Browse files Browse the repository at this point in the history
…1749)

Summary:
Pull Request resolved: #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
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Dec 5, 2024
1 parent 71d1eca commit e7fbaea
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
17 changes: 9 additions & 8 deletions velox/functions/prestosql/URLFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions velox/functions/prestosql/tests/URLFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e7fbaea

Please sign in to comment.