Handle overflow, underflow in XPath substring position, length.

third_party/WebKit/Source/core/xml/XPathFunctions.cpp

Index: third_party/WebKit/Source/core/xml/XPathFunctions.cpp
diff --git a/third_party/WebKit/Source/core/xml/XPathFunctions.cpp b/third_party/WebKit/Source/core/xml/XPathFunctions.cpp
index 8fe6c2a563024496d1667ab559ea5a7ff1642203..239eabf6eaf9d129893b5abbd0337df2bcbf6823 100644
--- a/third_party/WebKit/Source/core/xml/XPathFunctions.cpp
+++ b/third_party/WebKit/Source/core/xml/XPathFunctions.cpp
@@ -37,6 +37,9 @@
 #include "wtf/MathExtras.h"
 #include "wtf/text/StringBuilder.h"
 
+#include <algorithm>
+#include <limits>
+
 namespace blink {
 namespace XPath {
 
@@ -530,34 +533,45 @@ Value FunSubstringAfter::evaluate(EvaluationContext& context) const {
   return s1.substring(i + s2.length());
 }
 
+// Returns |value| clipped to the range [lo, hi].
+static double clip(const double& lo, const double& value, const double& hi) {

[yosin_UTC9, 2016/10/24 05:45:36]

It is better to use |double| instead of |const double&|.
x64 ABI passes first four(MSVC)/eight(GCC) floating-point value in register.

[yosin_UTC9, 2016/10/24 06:07:42]

Oops, there is |std::clamp(v, lo, hi)|; all parameters should be finite number.
http://en.cppreference.com/w/cpp/algorithm/clamp

[yosin_UTC9, 2016/10/24 06:08:31]

Sorry for confusion. |std::clamp()| is available from C++17 

+  return std::min(hi, std::max(lo, value));
+}
+
+// Computes the 1-based start and end (exclusive) string indices for
+// substring. This is all the positions [1, maxLen (inclusive)] where
+// start <= position < start + len
+static std::pair<unsigned, unsigned> findSubstringStartEnd(double start,

[yosin_UTC9, 2016/10/24 05:45:36]

|computeSubstringStartEnd()|? as comment says?

+                                                           double len,
+                                                           double maxLen) {
+  DCHECK(std::isfinite(maxLen));
+  double end = start + len;
+  if (std::isnan(start) || std::isnan(end))
+    return std::make_pair(1, 1);
+  // Neither start nor end are NaN, but may still be +/- Inf
+  start = clip(1., start, maxLen + 1);

[yosin_UTC9, 2016/10/24 05:45:36]

nit: s/1./1.0/ to avoid sequential punctuation.

+  end = clip(start, end, maxLen + 1);

[yosin_UTC9, 2016/10/24 05:45:36]

Better to use another variable rather than changing variable, e.g. clippedStart
and clippedEnd 

+  return std::make_pair(static_cast<unsigned>(start),
+                        static_cast<unsigned>(end));
+}
+
+// substring(string, number pos, number? len)
+//
+// Characters in string are indexed from 1. Numbers are doubles and
+// substring is specified to work with IEEE-754 infinity, NaN, and
+// XPath's bespoke rounding function, round.
+//
+// <https://www.w3.org/TR/xpath/#function-substring>
 Value FunSubstring::evaluate(EvaluationContext& context) const {
-  String s = arg(0)->evaluate(context).toString();
-  double doublePos = arg(1)->evaluate(context).toNumber();
-  if (std::isnan(doublePos))
-    return "";
-  long pos = static_cast<long>(FunRound::round(doublePos));
-  bool haveLength = argCount() == 3;
-  long len = -1;
-  if (haveLength) {
-    double doubleLen = arg(2)->evaluate(context).toNumber();
-    if (std::isnan(doubleLen))
-      return "";
-    len = static_cast<long>(FunRound::round(doubleLen));
-  }
-
-  if (pos > long(s.length()))
+  String sourceString = arg(0)->evaluate(context).toString();
+  double pos = FunRound::round(arg(1)->evaluate(context).toNumber());

[yosin_UTC9, 2016/10/24 05:45:36]

nit: s/double/const double/

+  double len = argCount() == 3

[yosin_UTC9, 2016/10/24 05:45:36]

nit: s/double/const double/

+                   ? FunRound::round(arg(2)->evaluate(context).toNumber())
+                   : std::numeric_limits<double>::infinity();
+  auto bounds = findSubstringStartEnd(pos, len, sourceString.length());

[yosin_UTC9, 2016/10/24 05:45:36]

nit: s/auto/const auto/

+  if (bounds.second <= bounds.first)
     return "";
-
-  if (pos < 1) {
-    if (haveLength) {
-      len -= 1 - pos;
-      if (len < 1)
-        return "";
-    }
-    pos = 1;
-  }
-
-  return s.substring(pos - 1, len);
+  return sourceString.substring(bounds.first - 1, bounds.second - bounds.first);
 }
 
 Value FunStringLength::evaluate(EvaluationContext& context) const {