Handle overflow, underflow in XPath substring position, length.

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

 /*
  * Copyright (C) 2005 Frerich Raabe <raabe@kde.org>
  * Copyright (C) 2006, 2009 Apple Inc.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
@@ skipping 19 matching lines @@
 #include "core/XMLNames.h"
 #include "core/dom/Attr.h"
 #include "core/dom/Element.h"
 #include "core/dom/ProcessingInstruction.h"
 #include "core/dom/TreeScope.h"
 #include "core/xml/XPathUtil.h"
 #include "core/xml/XPathValue.h"
 #include "wtf/MathExtras.h"
 #include "wtf/text/StringBuilder.h"
 
+#include <algorithm>
+#include <limits>
+
 namespace blink {
 namespace XPath {
 
 static inline bool isWhitespace(UChar c) {
   return c == ' ' || c == '\n' || c == '\r' || c == '\t';
 }
 
 #define DEFINE_FUNCTION_CREATOR(Class) \
   static Function* create##Class() { return new Class; }
 
@@ skipping 473 matching lines @@
   String s1 = arg(0)->evaluate(context).toString();
   String s2 = arg(1)->evaluate(context).toString();
 
   size_t i = s1.find(s2);
   if (i == kNotFound)
     return "";
 
   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))
+  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 "";
-  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()))
-    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 {
   if (!argCount())
     return Value(context.node.get()).toString().length();
   return arg(0)->evaluate(context).toString().length();
 }
 
 Value FunNormalizeSpace::evaluate(EvaluationContext& context) const {
   if (!argCount()) {
@@ skipping 179 matching lines @@
     return nullptr;
 
   Function* function = functionRec->factoryFn();
   function->setArguments(args);
   function->setName(name);
   return function;
 }
 
 }  // namespace XPath
 }  // namespace blink