From 0839a8ef2a7ac7ae6e7474ca8978b6c2fbd5a776 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 29 Nov 2017 19:22:20 +0000 Subject: Fix some nits in FPDFText_GetText(). Use more variables to avoid redundant calculations. Add one more edge test case. Change-Id: I6c8a0aca9de3bdd1a394c39304fd9a75009f9489 Reviewed-on: https://pdfium-review.googlesource.com/19690 Commit-Queue: Ryan Harrison Reviewed-by: Ryan Harrison --- fpdfsdk/fpdftext.cpp | 30 +++++++++++++++--------------- fpdfsdk/fpdftext_embeddertest.cpp | 6 ++++++ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/fpdfsdk/fpdftext.cpp b/fpdfsdk/fpdftext.cpp index 022b7caa8d..5a2deb9a81 100644 --- a/fpdfsdk/fpdftext.cpp +++ b/fpdfsdk/fpdftext.cpp @@ -164,22 +164,21 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFText_GetText(FPDF_TEXTPAGE page, int char_start, int char_count, unsigned short* result) { - if (char_start < 0 || char_count < 0 || !result || !page) + if (!page || char_start < 0 || char_count < 0 || !result) return 0; CPDF_TextPage* textpage = CPDFTextPageFromFPDFTextPage(page); - if (char_start >= textpage->CountChars()) + int char_available = textpage->CountChars() - char_start; + if (char_available <= 0) return 0; - char_count = std::min(char_count, textpage->CountChars() - char_start); + char_count = std::min(char_count, char_available); if (char_count == 0) { - // Writting out "" - *result = 0; + // Writing out "", which has a character count of 1 due to the NUL. + *result = '\0'; return 1; } - int char_last = char_start + char_count - 1; - // char_* values are for a data structure that includes non-printing unicode // characters, where the text_* values are from a data structure that doesn't // include these characters, so translation is needed. @@ -187,13 +186,14 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFText_GetText(FPDF_TEXTPAGE page, if (text_start == -1) return 0; + int char_last = char_start + char_count - 1; int text_last = textpage->TextIndexFromCharIndex(char_last); if (text_last == -1) return 0; - if (text_start > text_last) - return 0; int text_count = text_last - text_start + 1; + if (text_count < 1) + return 0; WideString str = textpage->GetPageText(text_start, text_count); if (str.GetLength() > static_cast(text_count)) @@ -202,12 +202,12 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFText_GetText(FPDF_TEXTPAGE page, // UFT16LE_Encode doesn't handle surrogate pairs properly, so it is expected // the number of items to stay the same. ByteString byte_str = str.UTF16LE_Encode(); - ASSERT((byte_str.GetLength()) / kBytesPerCharacter <= - static_cast(char_count + 1)); // +1 to account for the string - // terminator - memcpy(result, byte_str.GetBuffer(byte_str.GetLength()), - byte_str.GetLength()); - return (byte_str.GetLength() / kBytesPerCharacter); + size_t byte_str_len = byte_str.GetLength(); + int ret_count = byte_str_len / kBytesPerCharacter; + + ASSERT(ret_count <= char_count + 1); // +1 to account for the NUL terminator. + memcpy(result, byte_str.GetBuffer(byte_str_len), byte_str_len); + return ret_count; } FPDF_EXPORT int FPDF_CALLCONV FPDFText_CountRects(FPDF_TEXTPAGE text_page, diff --git a/fpdfsdk/fpdftext_embeddertest.cpp b/fpdfsdk/fpdftext_embeddertest.cpp index 4b82dc081e..51216b9818 100644 --- a/fpdfsdk/fpdftext_embeddertest.cpp +++ b/fpdfsdk/fpdftext_embeddertest.cpp @@ -49,6 +49,12 @@ TEST_F(FPDFTextEmbeddertest, Text) { EXPECT_EQ(1, FPDFText_GetText(textpage, 0, 0, fixed_buffer)); EXPECT_EQ(0, fixed_buffer[0]); + // Keep going and check the next case. + memset(fixed_buffer, 0xbd, sizeof(fixed_buffer)); + EXPECT_EQ(2, FPDFText_GetText(textpage, 0, 1, fixed_buffer)); + EXPECT_EQ(expected[0], fixed_buffer[0]); + EXPECT_EQ(0, fixed_buffer[1]); + // Check includes the terminating NUL that is provided. int num_chars = FPDFText_GetText(textpage, 0, 128, fixed_buffer); ASSERT_GE(num_chars, 0); -- cgit v1.2.3