DCHECK vswprintf format string cross-platform portability. Use %ls, not %s, for wchar_t* fields.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@849 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 118 insertions, 9 deletions

diff --git a/base/string_util.cc b/base/string_util.cc
index 7208cd0..0a930d9 100644
--- a/base/string_util.cc
+++ b/base/string_util.cc
@@ -259,6 +259,48 @@ class WStringToDoubleTraits {
 }  // namespace
 
 
+namespace base {
+
+bool IsWprintfFormatPortable(const wchar_t* format) {
+  for (const wchar_t* position = format; *position != '\0'; ++position) {
+
+    if (*position == '%') {
+      bool in_specification = true;
+      bool modifier_l = false;
+      while (in_specification) {
+        // Eat up characters until reaching a known specifier.
+        if (*++position == '\0') {
+          // The format string ended in the middle of a specification.  Call
+          // it portable because no unportable specifications were found.  The
+          // string is equally broken on all platforms.
+          return true;
+        }
+
+        if (*position == 'l') {
+          // 'l' is the only thing that can save the 's' and 'c' specifiers.
+          modifier_l = true;
+        } else if (((*position == 's' || *position == 'c') && !modifier_l) ||
+                   *position == 'S' || *position == 'C' || *position == 'F' ||
+                   *position == 'D' || *position == 'O' || *position == 'U') {
+          // Not portable.
+          return false;
+        }
+
+        if (wcschr(L"diouxXeEfgGaAcspn%", *position)) {
+          // Portable, keep scanning the rest of the format string.
+          in_specification = false;
+        }
+      }
+    }
+
+  }
+
+  return true;
+}
+
+}  // namespace base
+
+
 const std::string& EmptyString() {
   return *Singleton<std::string>::get();
 }
diff --git a/base/string_util.h b/base/string_util.h
index 5faa9cf..ca0496c 100644
--- a/base/string_util.h
+++ b/base/string_util.h
@@ -92,6 +92,29 @@ inline int swprintf(wchar_t* buffer, size_t size, const wchar_t* format, ...) {
 size_t strlcpy(char* dst, const char* src, size_t dst_size);
 size_t wcslcpy(wchar_t* dst, const wchar_t* src, size_t dst_size);
 
+// Scan a wprintf format string to determine whether it's portable across a
+// variety of systems.  This function only checks that the conversion
+// specifiers used by the format string are supported and have the same meaning
+// on a variety of systems.  It doesn't check for other errors that might occur
+// within a format string.
+//
+// Nonportable conversion specifiers for wprintf are:
+//  - 's' and 'c' without an 'l' length modifier.  %s and %c operate on char
+//     data on all systems except Windows, which treat them as wchar_t data.
+//     Use %ls and %lc for wchar_t data instead.
+//  - 'S' and 'C', which operate on wchar_t data on all systems except Windows,
+//     which treat them as char data.  Use %ls and %lc for wchar_t data
+//     instead.
+//  - 'F', which is not identified by Windows wprintf documentation.
+//  - 'D', 'O', and 'U', which are deprecated and not available on all systems.
+//     Use %ld, %lo, and %lu instead.
+//
+// Note that there is no portable conversion specifier for char data when
+// working with wprintf.
+//
+// This function is intended to be called from base::vswprintf.
+bool IsWprintfFormatPortable(const wchar_t* format);
+
 }  // namespace base
 
 #if defined(OS_WIN)
diff --git a/base/string_util_posix.h b/base/string_util_posix.h
index b3df2ad..a9e3646 100644
--- a/base/string_util_posix.h
+++ b/base/string_util_posix.h
@@ -35,6 +35,8 @@
 #include <string.h>
 #include <wchar.h>
 
+#include "base/logging.h"
+
 namespace base {
 
 inline int strncasecmp(const char* string1, const char* string2, size_t count) {
@@ -48,6 +50,7 @@ inline int vsnprintf(char* buffer, size_t size,
 
 inline int vswprintf(wchar_t* buffer, size_t size,
                      const wchar_t* format, va_list arguments) {
+  DCHECK(IsWprintfFormatPortable(format));
   return ::vswprintf(buffer, size, format, arguments);
 }
 
diff --git a/base/string_util_unittest.cc b/base/string_util_unittest.cc
index 7cc9764..c286222 100644
--- a/base/string_util_unittest.cc
+++ b/base/string_util_unittest.cc
@@ -978,7 +978,7 @@ TEST(StringUtilTest, StringPrintfEmptyFormat) {
 
 TEST(StringUtilTest, StringPrintfMisc) {
   EXPECT_EQ("123hello w", StringPrintf("%3d%2s %1c", 123, "hello", 'w'));
-  EXPECT_EQ(L"123hello w", StringPrintf(L"%3d%2ls %1c", 123, L"hello", 'w'));
+  EXPECT_EQ(L"123hello w", StringPrintf(L"%3d%2ls %1lc", 123, L"hello", 'w'));
 }
 
 TEST(StringUtilTest, StringAppendfStringEmptyParam) {
@@ -997,7 +997,7 @@ TEST(StringUtilTest, StringAppendfEmptyString) {
   EXPECT_EQ("Hello", value);
 
   std::wstring valuew(L"Hello");
-  StringAppendF(&valuew, L"%s", L"");
+  StringAppendF(&valuew, L"%ls", L"");
   EXPECT_EQ(L"Hello", valuew);
 }
 
@@ -1306,3 +1306,35 @@ TEST(StringUtilTest, LcpyTest) {
     EXPECT_EQ(0, memcmp(wdst, L"ab", sizeof(wchar_t) * 3));
   }
 }
+
+TEST(StringUtilTest, WprintfFormatPortabilityTest) {
+  struct TestData {
+    const wchar_t* input;
+    bool portable;
+  } cases[] = {
+    { L"%ls", true },
+    { L"%s", false },
+    { L"%S", false },
+    { L"%lS", false },
+    { L"Hello, %s", false },
+    { L"%lc", true },
+    { L"%c", false },
+    { L"%C", false },
+    { L"%lC", false },
+    { L"%ls %s", false },
+    { L"%s %ls", false },
+    { L"%s %ls %s", false },
+    { L"%f", true },
+    { L"%f %F", false },
+    { L"%d %D", false },
+    { L"%o %O", false },
+    { L"%u %U", false },
+    { L"%f %d %o %u", true },
+    { L"%-8d (%02.1f%)", true },
+    { L"% 10s", false },
+    { L"% 10ls", true }
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+    EXPECT_EQ(cases[i].portable, base::IsWprintfFormatPortable(cases[i].input));
+  }
+}
diff --git a/base/string_util_win.h b/base/string_util_win.h
index bec84042..80ebd38 100644
--- a/base/string_util_win.h
+++ b/base/string_util_win.h
@@ -27,14 +27,16 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef BASE_STRING_UTIL_WIN_H__
-#define BASE_STRING_UTIL_WIN_H__
+#ifndef BASE_STRING_UTIL_WIN_H_
+#define BASE_STRING_UTIL_WIN_H_
 
 #include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
 #include <wchar.h>
 
+#include "base/logging.h"
+
 namespace base {
 
 inline int strncasecmp(const char* s1, const char* s2, size_t count) {
@@ -51,6 +53,8 @@ inline int vsnprintf(char* buffer, size_t size,
 
 inline int vswprintf(wchar_t* buffer, size_t size,
                      const wchar_t* format, va_list arguments) {
+  DCHECK(IsWprintfFormatPortable(format));
+
   int length = _vsnwprintf_s(buffer, size, size - 1, format, arguments);
   if (length < 0)
     return _vscwprintf(format, arguments);
@@ -59,4 +63,4 @@ inline int vswprintf(wchar_t* buffer, size_t size,
 
 }  // namespace base
 
-#endif  // BASE_STRING_UTIL_WIN_H__
+#endif  // BASE_STRING_UTIL_WIN_H_
diff --git a/webkit/glue/dom_operations.cc b/webkit/glue/dom_operations.cc
index dd89d84..8b1e8bd 100644
--- a/webkit/glue/dom_operations.cc
+++ b/webkit/glue/dom_operations.cc
@@ -49,10 +49,13 @@
 #pragma warning(pop)
 #undef LOG
 
+// Brings in more WebKit headers and #undefs LOG again, so this needs to come
+// first.
+#include "webkit/glue/autocomplete_input_listener.h"
+
 #include "webkit/glue/dom_operations.h"
 
 #include "base/string_util.h"
-#include "webkit/glue/autocomplete_input_listener.h"
 #include "webkit/glue/form_data.h"
 #include "webkit/glue/glue_util.h"
 #include "webkit/glue/password_autocomplete_listener.h"
diff --git a/webkit/glue/editor_client_impl.cc b/webkit/glue/editor_client_impl.cc
index 50545d6..a5ec8de 100644
--- a/webkit/glue/editor_client_impl.cc
+++ b/webkit/glue/editor_client_impl.cc
@@ -42,7 +42,6 @@
 #include "KeyboardEvent.h"
 #include "PlatformKeyboardEvent.h"
 #include "PlatformString.h"
-#include "WebView.h"
 #pragma warning(pop)
 
 #undef LOG
@@ -51,6 +50,7 @@
 #include "webkit/glue/editor_client_impl.h"
 #include "webkit/glue/glue_util.h"
 #include "webkit/glue/webkit_glue.h"
+#include "webkit/glue/webview.h"
 #include "webkit/glue/webview_impl.h"
 
 // The notImplemented() from NotImplemented.h is now being dragged in via
diff --git a/webkit/port/bindings/v8/v8_np_utils.cpp b/webkit/port/bindings/v8/v8_np_utils.cpp
index 3887460..c1d3510 100644
--- a/webkit/port/bindings/v8/v8_np_utils.cpp
+++ b/webkit/port/bindings/v8/v8_np_utils.cpp
@@ -31,10 +31,12 @@
 
 #include "v8_np_utils.h"
 
-#include "base/scoped_ptr.h"
-#include "base/string_util.h"
 #include "DOMWindow.h"
 #include "Frame.h"
+#undef LOG
+
+#include "base/scoped_ptr.h"
+#include "base/string_util.h"
 #include "npruntime_priv.h"
 #include "np_v8object.h"
 #include "v8_npobject.h"