Skip to content

Commit 3ad6f77

Browse files
committed
Fix qualified PackageSpec parse hang.
See microsoft/vcpkg#48578 (comment)
1 parent e0f5cf6 commit 3ad6f77

File tree

3 files changed

+106
-14
lines changed

3 files changed

+106
-14
lines changed

src/vcpkg-test/platform-expression.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,7 @@ TEST_CASE ("missing closing )", "[platform-expression]")
526526
{
527527
CHECK_FALSE(parse_expr("(windows & arm | linux"));
528528
CHECK_FALSE(parse_expr("( (windows & arm) | (osx & arm64) | linux"));
529+
CHECK_FALSE(parse_expr("(!((windows & x64 & !uwp) & !(linux & x64))"));
529530
}
530531

531532
TEST_CASE ("missing or invalid identifier", "[platform-expression]")

src/vcpkg-test/specifier.cpp

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,4 +367,76 @@ TEST_CASE ("specifier parsing", "[specifier]")
367367
on expression: zlib (windows)
368368
^)"));
369369
}
370+
371+
SECTION ("parsed specifier from string with unclosed empty qualifier")
372+
{
373+
auto s = vcpkg::parse_qualified_specifier(
374+
"zlib(", AllowFeatures::Yes, ParseExplicitTriplet::Allow, AllowPlatformSpec::Yes);
375+
if (s.has_value())
376+
{
377+
FAIL();
378+
}
379+
else
380+
{
381+
REQUIRE(s.error() == LocalizedString::from_raw(
382+
R"(error: missing closing )
383+
on expression: zlib(
384+
^)"));
385+
}
386+
}
387+
388+
SECTION ("parsed specifier from string with unclosed empty qualifier newline")
389+
{
390+
auto s = vcpkg::parse_qualified_specifier(
391+
"zlib(\n", AllowFeatures::Yes, ParseExplicitTriplet::Allow, AllowPlatformSpec::Yes);
392+
if (s.has_value())
393+
{
394+
FAIL();
395+
}
396+
else
397+
{
398+
REQUIRE(s.error() == LocalizedString::from_raw(
399+
R"(error: missing closing )
400+
on expression: zlib(
401+
^)"));
402+
}
403+
}
404+
405+
SECTION ("parsed specifier from string with unclosed qualifier")
406+
{
407+
auto s = vcpkg::parse_qualified_specifier("zlib:x64-windows(!((windows & x64 & !uwp) & !(linux & x64))",
408+
AllowFeatures::Yes,
409+
ParseExplicitTriplet::Allow,
410+
AllowPlatformSpec::Yes);
411+
if (s.has_value())
412+
{
413+
FAIL();
414+
}
415+
else
416+
{
417+
REQUIRE(s.error() == LocalizedString::from_raw(
418+
R"(error: missing closing )
419+
on expression: zlib:x64-windows(!((windows & x64 & !uwp) & !(linux & x64))
420+
^)"));
421+
}
422+
}
423+
424+
SECTION ("parsed specifier from string with unclosed qualifier newline")
425+
{
426+
auto s = vcpkg::parse_qualified_specifier("zlib:x64-windows(!((windows & x64 & !uwp) & !(linux & x64))\n",
427+
AllowFeatures::Yes,
428+
ParseExplicitTriplet::Allow,
429+
AllowPlatformSpec::Yes);
430+
if (s.has_value())
431+
{
432+
FAIL();
433+
}
434+
else
435+
{
436+
REQUIRE(s.error() == LocalizedString::from_raw(
437+
R"(error: missing closing )
438+
on expression: zlib:x64-windows(!((windows & x64 & !uwp) & !(linux & x64))
439+
^)"));
440+
}
441+
}
370442
}

src/vcpkg/packagespec.cpp

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -376,32 +376,51 @@ namespace vcpkg
376376
return nullopt;
377377
}
378378

379-
auto loc = parser.cur_loc();
380-
std::string platform_string;
381-
int depth = 1;
382-
while (depth > 0 && (ch = parser.next()) != 0)
379+
ch = parser.next();
380+
if (ch == '\r' || ch == '\n' || ch == Unicode::end_of_file)
383381
{
384-
if (ch == '(') ++depth;
385-
if (ch == ')') --depth;
382+
parser.add_error(msg::format(msgMissingClosingParen));
383+
return nullopt;
386384
}
387-
if (depth > 0)
385+
386+
auto expr_loc = parser.cur_loc();
387+
int depth = 1;
388+
for (;;)
388389
{
389-
parser.add_error(msg::format(msgMissingClosingParen), loc);
390-
return nullopt;
390+
if (ch == '(')
391+
{
392+
++depth;
393+
}
394+
else if (ch == ')')
395+
{
396+
--depth;
397+
if (depth == 0)
398+
{
399+
break;
400+
}
401+
}
402+
403+
ch = parser.next();
404+
if (ch == '\r' || ch == '\n' || ch == Unicode::end_of_file)
405+
{
406+
parser.add_error(msg::format(msgMissingClosingParen));
407+
return nullopt;
408+
}
391409
}
392-
platform_string.append((++loc.it).pointer_to_current(), parser.it().pointer_to_current());
410+
393411
auto platform_opt = PlatformExpression::parse_platform_expression(
394-
platform_string, PlatformExpression::MultipleBinaryOperators::Allow);
412+
std::string(expr_loc.it.pointer_to_current(), parser.it().pointer_to_current()),
413+
PlatformExpression::MultipleBinaryOperators::Allow);
395414
if (auto platform = platform_opt.get())
396415
{
397-
ret.platform.emplace(loc, std::move(*platform));
416+
ret.platform.emplace(expr_loc, std::move(*platform));
398417
}
399418
else
400419
{
401-
parser.add_error(std::move(platform_opt).error(), loc);
420+
parser.add_error(std::move(platform_opt).error(), expr_loc);
402421
}
403422

404-
parser.next();
423+
parser.next(); // consume ')'
405424
}
406425
// This makes the behavior of the parser more consistent -- otherwise, it will skip tabs and spaces only if
407426
// there isn't a qualifier.

0 commit comments

Comments
 (0)