Fix CFXJSE_FormCalcContext::Substr() behavior.
- Update tests according to the XFA spec.
- Make it behave as described in the XFA spec so tests pass.
- This makes an integer underflow in the implementation go away.
BUG=chromium:956985
Change-Id: I5c488de0b04cca5c93fc75e99d4aa5ad09b4c0fe
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/53618
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/xfa/cfxjse_formcalc_context.cpp b/fxjs/xfa/cfxjse_formcalc_context.cpp
index 387879c..b9971c9 100644
--- a/fxjs/xfa/cfxjse_formcalc_context.cpp
+++ b/fxjs/xfa/cfxjse_formcalc_context.cpp
@@ -4272,29 +4272,34 @@
return;
}
- std::unique_ptr<CFXJSE_Value> stringValue = GetSimpleValue(pThis, args, 0);
- std::unique_ptr<CFXJSE_Value> startValue = GetSimpleValue(pThis, args, 1);
- std::unique_ptr<CFXJSE_Value> endValue = GetSimpleValue(pThis, args, 2);
- if (ValueIsNull(pThis, stringValue.get()) ||
- (ValueIsNull(pThis, startValue.get())) ||
- (ValueIsNull(pThis, endValue.get()))) {
+ std::unique_ptr<CFXJSE_Value> string_value = GetSimpleValue(pThis, args, 0);
+ std::unique_ptr<CFXJSE_Value> start_value = GetSimpleValue(pThis, args, 1);
+ std::unique_ptr<CFXJSE_Value> end_value = GetSimpleValue(pThis, args, 2);
+ if (ValueIsNull(pThis, string_value.get()) ||
+ ValueIsNull(pThis, start_value.get()) ||
+ ValueIsNull(pThis, end_value.get())) {
args.GetReturnValue()->SetNull();
return;
}
- ByteString bsSource = ValueToUTF8String(stringValue.get());
- int32_t iLength = bsSource.GetLength();
+ ByteString bsSource = ValueToUTF8String(string_value.get());
+ size_t iLength = bsSource.GetLength();
if (iLength == 0) {
args.GetReturnValue()->SetString("");
return;
}
- int32_t iStart = pdfium::clamp(
- iLength, 1, static_cast<int32_t>(ValueToFloat(pThis, startValue.get())));
- int32_t iCount =
- std::max(0, static_cast<int32_t>(ValueToFloat(pThis, endValue.get())));
+ // |start_value| is 1-based. Assume first character if |start_value| is less
+ // than 1, per spec. Subtract 1 since |iStart| is 0-based.
+ size_t iStart = std::max(ValueToInteger(pThis, start_value.get()), 1) - 1;
+ if (iStart >= iLength) {
+ args.GetReturnValue()->SetString("");
+ return;
+ }
- --iStart;
+ // Negative values are treated as 0. Can't clamp() due to sign mismatches.
+ size_t iCount = std::max(ValueToInteger(pThis, end_value.get()), 0);
+ iCount = std::min(iCount, iLength - iStart);
args.GetReturnValue()->SetString(bsSource.Mid(iStart, iCount).AsStringView());
}
diff --git a/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp b/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
index ff2c47d..77e3880 100644
--- a/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
+++ b/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
@@ -1402,13 +1402,13 @@
struct {
const char* program;
const char* result;
- } static const kTests[] = {{"Substr(\"ABCDEFG\", -1, 4)", ""},
- {"Substr(\"ABCDEFG\", 0, 4)", ""},
+ } static const kTests[] = {{"Substr(\"ABCDEFG\", -1, 4)", "ABCD"},
+ {"Substr(\"ABCDEFG\", 0, 4)", "ABCD"},
{"Substr(\"ABCDEFG\", 3, 4)", "CDEF"},
{"Substr(\"ABCDEFG\", 4, 4)", "DEFG"},
- {"Substr(\"ABCDEFG\", 5, 4)", ""},
- {"Substr(\"ABCDEFG\", 6, 4)", ""},
- {"Substr(\"ABCDEFG\", 7, 4)", ""},
+ {"Substr(\"ABCDEFG\", 5, 4)", "EFG"},
+ {"Substr(\"ABCDEFG\", 6, 4)", "FG"},
+ {"Substr(\"ABCDEFG\", 7, 4)", "G"},
{"Substr(\"ABCDEFG\", 8, 4)", ""},
{"Substr(\"ABCDEFG\", 5, -1)", ""},
{"Substr(\"ABCDEFG\", 5, 0)", ""},
