Bug 1388789 - clean up \0 emission in nsTextFormatter; r?froydnj
nsTextFormatter unconditionally emitted a trailing \0, leading some code
elsewhere to have to work around this. This changes the code to only
emit it in snprintf.
MozReview-Commit-ID: G3CBpAPp9Tn
--- a/dom/svg/SVGPathSegUtils.cpp
+++ b/dom/svg/SVGPathSegUtils.cpp
@@ -70,25 +70,16 @@ SVGPathSegUtils::GetValueAsString(const
break;
default:
MOZ_ASSERT(false, "Unknown segment type");
aValue = u"<unknown-segment-type>";
return;
}
}
-
- // nsTextFormatter::ssprintf is one of the nsTextFormatter methods that
- // randomly appends '\0' to its output string, which means that the length
- // of the output string is one too long. We need to manually remove that '\0'
- // until nsTextFormatter is fixed.
- //
- if (aValue[aValue.Length() - 1] == char16_t('\0')) {
- aValue.SetLength(aValue.Length() - 1);
- }
}
static float
CalcDistanceBetweenPoints(const Point& aP1, const Point& aP2)
{
return NS_hypot(aP2.x - aP1.x, aP2.y - aP1.y);
}
--- a/xpcom/string/nsTextFormatter.cpp
+++ b/xpcom/string/nsTextFormatter.cpp
@@ -473,24 +473,25 @@ nsTextFormatter::dosprintf(SprintfStateS
static const char16_t hex[] = u"0123456789abcdef";
static char16_t HEX[] = u"0123456789ABCDEF";
static const BoxedValue emptyString(u"");
char16_t c;
int flags, width, prec, radix;
const char16_t* hexp;
- int rv;
// Next argument for non-numbered arguments.
size_t nextNaturalArg = 0;
// True if we ever saw a numbered argument.
bool sawNumberedArg = false;
while ((c = *aFmt++) != 0) {
+ int rv;
+
if (c != '%') {
rv = (*aState->stuff)(aState, aFmt - 1, 1);
if (rv < 0) {
return rv;
}
continue;
}
@@ -823,34 +824,25 @@ nsTextFormatter::dosprintf(SprintfStateS
MOZ_ASSERT(0);
}
if (rv < 0) {
return rv;
}
}
- /* Stuff trailing NUL */
- char16_t null = '\0';
-
- rv = (*aState->stuff)(aState, &null, 1);
-
- return rv;
+ return 0;
}
/************************************************************************/
int
nsTextFormatter::StringStuff(nsTextFormatter::SprintfStateStr* aState, const char16_t* aStr,
uint32_t aLen)
{
- if (*aStr == '\0') {
- return 0;
- }
-
ptrdiff_t off = aState->cur - aState->base;
nsAString* str = static_cast<nsAString*>(aState->stuffclosure);
str->Append(aStr, aLen);
aState->base = str->BeginWriting();
aState->cur = aState->base + off;
@@ -890,33 +882,39 @@ nsTextFormatter::LimitStuff(SprintfState
return 0;
}
uint32_t
nsTextFormatter::vsnprintf(char16_t* aOut, uint32_t aOutLen,
const char16_t* aFmt, mozilla::Span<BoxedValue> aValues)
{
SprintfStateStr ss;
- uint32_t n;
MOZ_ASSERT((int32_t)aOutLen > 0);
if ((int32_t)aOutLen <= 0) {
return 0;
}
ss.stuff = LimitStuff;
ss.base = aOut;
ss.cur = aOut;
ss.maxlen = aOutLen;
int result = dosprintf(&ss, aFmt, aValues);
- /* If we added chars, and we didn't append a null, do it now. */
- if ((ss.cur != ss.base) && (*(ss.cur - 1) != '\0')) {
- *(--ss.cur) = '\0';
+ if (ss.cur == ss.base) {
+ return 0;
}
+ // Append a NUL. However, be sure not to count it in the returned
+ // length.
+ if (ss.cur - ss.base >= ptrdiff_t(ss.maxlen)) {
+ --ss.cur;
+ }
+ *ss.cur = '\0';
+
+ // Check the result now, so that an unterminated string can't
+ // possibly escape.
if (result < 0) {
return -1;
}
- n = ss.cur - ss.base;
- return n ? n - 1 : n;
+ return ss.cur - ss.base;
}
--- a/xpcom/tests/gtest/TestTextFormatter.cpp
+++ b/xpcom/tests/gtest/TestTextFormatter.cpp
@@ -211,8 +211,22 @@ TEST(TextFormatterTestMismatch, format_c
// just for completeness, this is our format, and works
nsTextFormatter::ssprintf(out, u"%c", 'c');
EXPECT_EQ(1u, out.Length());
EXPECT_EQ(u'c', out.CharAt(0));
nsTextFormatter::ssprintf(out, u"%c", u'c');
EXPECT_EQ(1u, out.Length());
EXPECT_EQ(u'c', out.CharAt(0));
}
+
+TEST(TextFormatterTestResults, Tests)
+{
+ char16_t buf[10];
+
+ EXPECT_EQ(nsTextFormatter::snprintf(buf, 10, u"%s", "more than 10 characters"), 9u);
+ EXPECT_EQ(buf[9], '\0');
+ EXPECT_STREQ("more than", NS_ConvertUTF16toUTF8(buf).get());
+
+ nsString out;
+ nsTextFormatter::ssprintf(out, u"%s", "more than 10 characters");
+ // The \0 isn't written here.
+ EXPECT_EQ(out.Length(), 23u);
+}