Fix an integer overflow in CFX_Path::GetBoundingBoxForStrokePath()
CL [1] allows a move operation that closes the path to be rendered,
which means points with "move + open" combo are no longer the only
moves to be anticipated in CFX_Path::GetBoundingBoxForStrokePath().
Due to [1], integer overflow is triggered when a "move + close" point
is the first point of path.
This CL makes sure "move + close" points are handled when updating
boundary of stroked path.
[1] https://pdfium.googlesource.com/pdfium/+/2ef13c3534c101c5267e48442b60b21ca8409d03
Bug: chromium:1414373, chromium:1414488
Change-Id: Ie11d1269bf116ac3f6c0290e7287c6e3c1ca4a58
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/103691
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Nigi <nigi@chromium.org>
diff --git a/core/fxge/cfx_path.cpp b/core/fxge/cfx_path.cpp
index 2e5ccd2..d52c21e 100644
--- a/core/fxge/cfx_path.cpp
+++ b/core/fxge/cfx_path.cpp
@@ -340,9 +340,14 @@
size_t iMiddlePoint = 0;
bool bJoin;
while (iPoint < m_Points.size()) {
- if (m_Points[iPoint].IsTypeAndOpen(CFX_Path::Point::Type::kMove)) {
- if (iPoint + 1 == m_Points.size())
+ if (m_Points[iPoint].m_Type == CFX_Path::Point::Type::kMove) {
+ if (iPoint + 1 == m_Points.size()) {
+ if (m_Points[iPoint].m_CloseFigure) {
+ // Update `rect` right away since this is the final point to be drawn.
+ rect.UpdateRect(m_Points[iPoint].m_Point);
+ }
break;
+ }
iStartPoint = iPoint + 1;
iEndPoint = iPoint;
@@ -358,7 +363,7 @@
iPoint += 2;
}
if (iPoint == m_Points.size() - 1 ||
- m_Points[iPoint + 1].IsTypeAndOpen(CFX_Path::Point::Type::kMove)) {
+ m_Points[iPoint + 1].m_Type == CFX_Path::Point::Type::kMove) {
iStartPoint = iPoint - 1;
iEndPoint = iPoint;
bJoin = false;
@@ -380,7 +385,7 @@
UpdateLineEndPoints(&rect, m_Points[iStartPoint].m_Point,
m_Points[iEndPoint].m_Point, half_width);
}
- iPoint++;
+ ++iPoint;
}
return rect;
}
diff --git a/core/fxge/cfx_path_unittest.cpp b/core/fxge/cfx_path_unittest.cpp
index eebad72..4ccb1c2 100644
--- a/core/fxge/cfx_path_unittest.cpp
+++ b/core/fxge/cfx_path_unittest.cpp
@@ -4,6 +4,7 @@
#include "core/fxge/cfx_path.h"
+#include "core/fxcrt/fx_coordinates.h"
#include "testing/gtest/include/gtest/gtest.h"
TEST(CFX_Path, BasicTest) {
@@ -371,3 +372,36 @@
EXPECT_EQ(CFX_PointF(65, 82), path.GetPoint(2));
EXPECT_EQ(CFX_PointF(65, 82), path.GetPoint(3));
}
+
+TEST(CFX_Path, GetBoundingBoxForStrokePath) {
+ static constexpr float kLineWidth = 1.0f;
+ static constexpr float kMiterLimit = 1.0f;
+
+ {
+ // Test the case that the first/last point is "move" and it closes the
+ // paths.
+ CFX_Path path;
+ path.AppendPoint({2, 0}, CFX_Path::Point::Type::kMove);
+ path.ClosePath();
+ EXPECT_EQ(CFX_FloatRect(2, 0, 2, 0),
+ path.GetBoundingBoxForStrokePath(kLineWidth, kMiterLimit));
+ }
+
+ {
+ // Test on a regular rect path.
+ CFX_Path path;
+ path.AppendPoint({2, 0}, CFX_Path::Point::Type::kMove);
+ path.AppendPoint({2, 1}, CFX_Path::Point::Type::kLine);
+ path.AppendPoint({0, 1}, CFX_Path::Point::Type::kLine);
+ path.AppendPoint({0, 0}, CFX_Path::Point::Type::kLine);
+ path.ClosePath();
+ EXPECT_EQ(CFX_FloatRect(-1, -1, 3, 2),
+ path.GetBoundingBoxForStrokePath(kLineWidth, kMiterLimit));
+
+ // If the final point is "move" and the path remains open, it should not
+ // affect the bounding rect.
+ path.AppendPoint({20, 20}, CFX_Path::Point::Type::kMove);
+ EXPECT_EQ(CFX_FloatRect(-1, -1, 3, 2),
+ path.GetBoundingBoxForStrokePath(kLineWidth, kMiterLimit));
+ }
+}