From da129ab38c3fb6ed3de85ffb6f8938eb31130a53 Mon Sep 17 00:00:00 2001
From: Ryan Harrison <rharrison@chromium.org>
Date: Tue, 1 Aug 2017 15:16:59 -0400
Subject: Replace raw value for constant error value in string operations

Currently Find() and other methods that return a FX_STRSIZE return -1
to indicate error/failure. This means that there is a lot of magic
numbers and magic checks floating around. The standard library for
similar operations uses a npos constant. This CL implements
FX_STRNPOS, and replaces usages of magic number checking. It also does
some type cleanup along the way where it was obvious that FX_STRSIZE
should be being used.

Removing the magic numbers should make eventually changing FX_STRSIZE
to be unsigned easier in the future.

BUG=pdfium:828

Change-Id: I67e481e44cf2f75a1698afa8fbee4f375a74c490
Reviewed-on: https://pdfium-review.googlesource.com/9651
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
---
 core/fpdftext/cpdf_linkextract.cpp  | 20 ++++++++++----------
 core/fpdftext/cpdf_textpagefind.cpp |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

(limited to 'core/fpdftext')

diff --git a/core/fpdftext/cpdf_linkextract.cpp b/core/fpdftext/cpdf_linkextract.cpp
index e0bd4ae49f..d795e71639 100644
--- a/core/fpdftext/cpdf_linkextract.cpp
+++ b/core/fpdftext/cpdf_linkextract.cpp
@@ -22,7 +22,7 @@ namespace {
 FX_STRSIZE FindWebLinkEnding(const CFX_WideString& str,
                              FX_STRSIZE start,
                              FX_STRSIZE end) {
-  if (str.Find(L'/', start) != -1) {
+  if (str.Find(L'/', start) != FX_STRNPOS) {
     // When there is a path and query after '/', most ASCII chars are allowed.
     // We don't sanitize in this case.
     return end;
@@ -197,7 +197,7 @@ bool CPDF_LinkExtract::CheckWebLink(CFX_WideString* strBeCheck,
   FX_STRSIZE len = str.GetLength();
   // First, try to find the scheme.
   FX_STRSIZE start = str.Find(kHttpScheme);
-  if (start != -1) {
+  if (start != FX_STRNPOS) {
     FX_STRSIZE off = start + kHttpSchemeLen;  // move after "http".
     if (len > off + 4) {                      // At least "://<char>" follows.
       if (str[off] == L's')                   // "https" scheme is accepted.
@@ -219,7 +219,7 @@ bool CPDF_LinkExtract::CheckWebLink(CFX_WideString* strBeCheck,
 
   // When there is no scheme, try to find url starting with "www.".
   start = str.Find(kWWWAddrStart);
-  if (start != -1 && len > start + kWWWAddrStartLen) {
+  if (start != FX_STRNPOS && len > start + kWWWAddrStartLen) {
     FX_STRSIZE end =
         TrimExternalBracketsFromWebLink(str, start, str.GetLength() - 1);
     end = FindWebLinkEnding(str, start, end);
@@ -234,9 +234,9 @@ bool CPDF_LinkExtract::CheckWebLink(CFX_WideString* strBeCheck,
 }
 
 bool CPDF_LinkExtract::CheckMailLink(CFX_WideString* str) {
-  int aPos = str->Find(L'@');
-  // Invalid when no '@'.
-  if (aPos < 1)
+  FX_STRSIZE aPos = str->Find(L'@');
+  // Invalid when no '@' or when starts/ends with '@'.
+  if (aPos == FX_STRNPOS || aPos == 0 || aPos == str->GetLength() - 1)
     return false;
 
   // Check the local part.
@@ -263,15 +263,15 @@ bool CPDF_LinkExtract::CheckMailLink(CFX_WideString* str) {
 
   // Check the domain name part.
   aPos = str->Find(L'@');
-  if (aPos < 1)
+  if (aPos < 1 || aPos == FX_STRNPOS)
     return false;
 
   str->TrimRight(L'.');
   // At least one '.' in domain name, but not at the beginning.
   // TODO(weili): RFC5322 allows domain names to be a local name without '.'.
   // Check whether we should remove this check.
-  int ePos = str->Find(L'.', aPos + 1);
-  if (ePos == -1 || ePos == aPos + 1)
+  FX_STRSIZE ePos = str->Find(L'.', aPos + 1);
+  if (ePos == FX_STRNPOS || ePos == aPos + 1)
     return false;
 
   // Validate all other chars in domain name.
@@ -295,7 +295,7 @@ bool CPDF_LinkExtract::CheckMailLink(CFX_WideString* str) {
     pPos = i;
   }
 
-  if (str->Find(L"mailto:") == -1)
+  if (str->Find(L"mailto:") == FX_STRNPOS)
     *str = L"mailto:" + *str;
 
   return true;
diff --git a/core/fpdftext/cpdf_textpagefind.cpp b/core/fpdftext/cpdf_textpagefind.cpp
index fe610abcd4..adef9f6c0c 100644
--- a/core/fpdftext/cpdf_textpagefind.cpp
+++ b/core/fpdftext/cpdf_textpagefind.cpp
@@ -164,7 +164,7 @@ bool CPDF_TextPageFind::FindNext() {
     }
     int endIndex;
     nResultPos = m_strText.Find(csWord.c_str(), nStartPos);
-    if (nResultPos == -1) {
+    if (nResultPos == FX_STRNPOS) {
       m_IsFind = false;
       return m_IsFind;
     }
-- 
cgit v1.2.3