Bug 818861 Shouldn't allow promising a flat string from a flat string r=dbaron
authorNeil Rashbrook <neil@parkwaycc.co.uk>
Sat, 22 Dec 2012 20:40:37 +0000
changeset 116893 5abe31fa4a5c6847d1c5c6a5a6b6957be0821ea0
parent 116892 178bc511dec97ec9d828f02fc15aa38ab6898961
child 116894 6bc692ff1c10fa3d2f045d3764e701c101c298db
push id20200
push userneil@parkwaycc.co.uk
push dateSat, 22 Dec 2012 20:40:42 +0000
treeherdermozilla-inbound@5abe31fa4a5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs818861
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 818861 Shouldn't allow promising a flat string from a flat string r=dbaron
content/base/src/nsFrameMessageManager.cpp
content/canvas/src/WebGLContextGL.cpp
dom/plugins/base/nsPluginInstanceOwner.cpp
dom/plugins/ipc/PluginProcessChild.cpp
gfx/thebes/gfxGDIFontList.cpp
image/decoders/icon/mac/nsIconChannelCocoa.mm
profile/dirserviceprovider/src/nsProfileLock.cpp
toolkit/components/alerts/nsAlertsService.cpp
toolkit/components/places/History.cpp
toolkit/xre/nsAppRunner.cpp
xpcom/base/nsCycleCollector.cpp
xpcom/base/nsMemoryInfoDumper.cpp
xpcom/string/public/nsTPromiseFlatString.h
--- a/content/base/src/nsFrameMessageManager.cpp
+++ b/content/base/src/nsFrameMessageManager.cpp
@@ -226,17 +226,17 @@ GetParamsForMessage(JSContext* aCx,
   //    as a dictionary.
   nsAutoString json;
   JSAutoRequest ar(aCx);
   jsval v = aObject;
   NS_ENSURE_TRUE(JS_Stringify(aCx, &v, nullptr, JSVAL_NULL, JSONCreator, &json), false);
   NS_ENSURE_TRUE(!json.IsEmpty(), false);
 
   jsval val = JSVAL_NULL;
-  NS_ENSURE_TRUE(JS_ParseJSON(aCx, static_cast<const jschar*>(PromiseFlatString(json).get()),
+  NS_ENSURE_TRUE(JS_ParseJSON(aCx, static_cast<const jschar*>(json.get()),
                               json.Length(), &val), false);
 
   return WriteStructuredClone(aCx, val, aBuffer, aClosure);
 }
 
 
 // nsISyncMessageSender
 
@@ -530,17 +530,17 @@ nsFrameMessageManager::ReceiveMessage(ns
         jsval json = JSVAL_NULL;
         if (aCloneData && aCloneData->mDataLength &&
             !ReadStructuredClone(ctx, *aCloneData, &json)) {
           JS_ClearPendingException(ctx);
           return NS_OK;
         }
         JSString* jsMessage =
           JS_NewUCStringCopyN(ctx,
-                              static_cast<const jschar*>(PromiseFlatString(aMessage).get()),
+                              static_cast<const jschar*>(aMessage.BeginReading()),
                               aMessage.Length());
         NS_ENSURE_TRUE(jsMessage, NS_ERROR_OUT_OF_MEMORY);
         JS_DefineProperty(ctx, param, "target", targetv, NULL, NULL, JSPROP_ENUMERATE);
         JS_DefineProperty(ctx, param, "name",
                           STRING_TO_JSVAL(jsMessage), NULL, NULL, JSPROP_ENUMERATE);
         JS_DefineProperty(ctx, param, "sync",
                           BOOLEAN_TO_JSVAL(aSync), NULL, NULL, JSPROP_ENUMERATE);
         JS_DefineProperty(ctx, param, "json", json, NULL, NULL, JSPROP_ENUMERATE); // deprecated
--- a/content/canvas/src/WebGLContextGL.cpp
+++ b/content/canvas/src/WebGLContextGL.cpp
@@ -4164,25 +4164,23 @@ WebGLContext::CompileShader(WebGLShader 
         resources.MaxDrawBuffers = 1;
         if (IsExtensionEnabled(OES_standard_derivatives))
             resources.OES_standard_derivatives = 1;
 
         // We're storing an actual instance of StripComments because, if we don't, the 
         // cleanSource nsAString instance will be destroyed before the reference is
         // actually used.
         StripComments stripComments(shader->Source());
-        const nsAString& cleanSource = nsString(stripComments.result().Elements(), stripComments.length());
+        const nsAString& cleanSource = Substring(stripComments.result().Elements(), stripComments.length());
         if (!ValidateGLSLString(cleanSource, "compileShader"))
             return;
 
-        const nsPromiseFlatString& flatSource = PromiseFlatString(cleanSource);
-
         // shaderSource() already checks that the source stripped of comments is in the
         // 7-bit ASCII range, so we can skip the NS_IsAscii() check.
-        const nsCString& sourceCString = NS_LossyConvertUTF16toASCII(flatSource);
+        NS_LossyConvertUTF16toASCII sourceCString(cleanSource);
 
         if (gl->WorkAroundDriverBugs()) {
             const uint32_t maxSourceLength = 0x3ffff;
             if (sourceCString.Length() > maxSourceLength)
                 return ErrorInvalidValue("compileShader: source has more than %d characters", 
                                          maxSourceLength);
         }
 
@@ -4293,18 +4291,17 @@ WebGLContext::CompileShader(WebGLShader 
 
             int len = 0;
             ShGetInfo(compiler, SH_OBJECT_CODE_LENGTH, &len);
 
             nsAutoCString translatedSrc;
             translatedSrc.SetLength(len);
             ShGetObjectCode(compiler, translatedSrc.BeginWriting());
 
-            nsPromiseFlatCString translatedSrc2(translatedSrc);
-            const char *ts = translatedSrc2.get();
+            const char *ts = translatedSrc.get();
 
             gl->fShaderSource(shadername, 1, &ts, NULL);
         } else { // not useShaderSourceTranslation
             // we just pass the raw untranslated shader source. We then can't use ANGLE idenfier mapping.
             // that's really bad, as that means we can't be 100% conformant. We should work towards always
             // using ANGLE identifier mapping.
             gl->fShaderSource(shadername, 1, &s, NULL);
         }
@@ -4608,17 +4605,17 @@ WebGLContext::ShaderSource(WebGLShader *
 
     if (!ValidateObject("shaderSource: shader", shader))
         return;
 
     // We're storing an actual instance of StripComments because, if we don't, the 
     // cleanSource nsAString instance will be destroyed before the reference is
     // actually used.
     StripComments stripComments(source);
-    const nsAString& cleanSource = nsString(stripComments.result().Elements(), stripComments.length());
+    const nsAString& cleanSource = Substring(stripComments.result().Elements(), stripComments.length());
     if (!ValidateGLSLString(cleanSource, "compileShader"))
         return;
 
     shader->SetSource(source);
 
     shader->SetNeedsTranslation();
 }
 
--- a/dom/plugins/base/nsPluginInstanceOwner.cpp
+++ b/dom/plugins/base/nsPluginInstanceOwner.cpp
@@ -1013,17 +1013,17 @@ NS_IMETHODIMP nsPluginInstanceOwner::Get
 
   if (charset.IsEmpty())
     return NS_OK;
 
   // common charsets and those not requiring conversion first
   if (charset.EqualsLiteral("us-ascii")) {
     *result = PL_strdup("US_ASCII");
   } else if (charset.EqualsLiteral("ISO-8859-1") ||
-      !nsCRT::strncmp(PromiseFlatCString(charset).get(), "UTF", 3)) {
+      !nsCRT::strncmp(charset.get(), "UTF", 3)) {
     *result = ToNewCString(charset);
   } else {
     if (!gCharsetMap) {
       const int NUM_CHARSETS = sizeof(charsets) / sizeof(moz2javaCharset);
       gCharsetMap = new nsDataHashtable<nsDepCharHashKey, const char*>();
       if (!gCharsetMap)
         return NS_ERROR_OUT_OF_MEMORY;
       gCharsetMap->Init(NUM_CHARSETS);
--- a/dom/plugins/ipc/PluginProcessChild.cpp
+++ b/dom/plugins/ipc/PluginProcessChild.cpp
@@ -65,17 +65,17 @@ PluginProcessChild::Init()
             }
         }
         if (needsReset) {
             nsCString setInterpose("DYLD_INSERT_LIBRARIES=");
             if (!interpose.IsEmpty()) {
                 setInterpose.Append(interpose);
             }
             // Values passed to PR_SetEnv() must be seperately allocated.
-            char* setInterposePtr = strdup(PromiseFlatCString(setInterpose).get());
+            char* setInterposePtr = strdup(setInterpose.get());
             PR_SetEnv(setInterposePtr);
         }
     }
 #endif
 
 #ifdef XP_WIN
     // Drag-and-drop needs OleInitialize to be called, and Silverlight depends
     // on the host calling CoInitialize (which is called by OleInitialize).
--- a/gfx/thebes/gfxGDIFontList.cpp
+++ b/gfx/thebes/gfxGDIFontList.cpp
@@ -422,17 +422,17 @@ GDIFontEntry::InitLogFont(const nsAStrin
     // always force lfItalic if we want it.  Font selection code will
     // do its best to give us an italic font entry, but if no face exists
     // it may give us a regular one based on weight.  Windows should
     // do fake italic for us in that case.
     mLogFont.lfItalic         = mItalic;
     mLogFont.lfWeight         = mWeight;
 
     int len = NS_MIN<int>(aName.Length(), LF_FACESIZE - 1);
-    memcpy(&mLogFont.lfFaceName, nsPromiseFlatString(aName).get(), len * 2);
+    memcpy(&mLogFont.lfFaceName, aName.BeginReading(), len * sizeof(PRUnichar));
     mLogFont.lfFaceName[len] = '\0';
 }
 
 GDIFontEntry* 
 GDIFontEntry::CreateFontEntry(const nsAString& aName,
                               gfxWindowsFontType aFontType, bool aItalic,
                               uint16_t aWeight, int16_t aStretch,
                               gfxUserFontData* aUserFontData,
@@ -555,20 +555,17 @@ GDIFontFamily::FindStyleVariations()
     HDC hdc = GetDC(nullptr);
     SetGraphicsMode(hdc, GM_ADVANCED);
 
     LOGFONTW logFont;
     memset(&logFont, 0, sizeof(LOGFONTW));
     logFont.lfCharSet = DEFAULT_CHARSET;
     logFont.lfPitchAndFamily = 0;
     uint32_t l = NS_MIN<uint32_t>(mName.Length(), LF_FACESIZE - 1);
-    memcpy(logFont.lfFaceName,
-           nsPromiseFlatString(mName).get(),
-           l * sizeof(PRUnichar));
-    logFont.lfFaceName[l] = 0;
+    memcpy(logFont.lfFaceName, mName.get(), l * sizeof(PRUnichar));
 
     EnumFontFamiliesExW(hdc, &logFont,
                         (FONTENUMPROCW)GDIFontFamily::FamilyAddStylesProc,
                         (LPARAM)this, 0);
 #ifdef PR_LOGGING
     if (LOG_FONTLIST_ENABLED() && mAvailableFonts.Length() == 0) {
         LOG_FONTLIST(("(fontlist) no styles available in family \"%s\"",
                       NS_ConvertUTF16toUTF8(mName).get()));
--- a/image/decoders/icon/mac/nsIconChannelCocoa.mm
+++ b/image/decoders/icon/mac/nsIconChannelCocoa.mm
@@ -228,17 +228,17 @@ nsresult nsIconChannel::MakeInputStream(
     if (NS_SUCCEEDED(localFileMac->GetCFURL(&macURL))) {
       iconImage = [[NSWorkspace sharedWorkspace] iconForFile:[(NSURL*)macURL path]];
       ::CFRelease(macURL);
     }
   }
 
   // if we don't have an icon yet try to get one by extension
   if (!iconImage && !fileExt.IsEmpty()) {
-    NSString* fileExtension = [NSString stringWithUTF8String:PromiseFlatCString(fileExt).get()];
+    NSString* fileExtension = [NSString stringWithUTF8String:fileExt.get()];
     iconImage = [[NSWorkspace sharedWorkspace] iconForFileType:fileExtension];
   }
 
   // If we still don't have an icon, get the generic document icon.
   if (!iconImage)
     iconImage = [[NSWorkspace sharedWorkspace] iconForFileType:NSFileTypeUnknown];
 
   if (!iconImage)
--- a/profile/dirserviceprovider/src/nsProfileLock.cpp
+++ b/profile/dirserviceprovider/src/nsProfileLock.cpp
@@ -203,18 +203,17 @@ nsresult nsProfileLock::LockWithFcntl(ns
     rv = aLockFile->GetNativePath(lockFilePath);
     if (NS_FAILED(rv)) {
         NS_ERROR("Could not get native path");
         return rv;
     }
 
     aLockFile->GetLastModifiedTime(&mReplacedLockTime);
 
-    mLockFileDesc = open(PromiseFlatCString(lockFilePath).get(),
-                          O_WRONLY | O_CREAT | O_TRUNC, 0666);
+    mLockFileDesc = open(lockFilePath.get(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
     if (mLockFileDesc != -1)
     {
         struct flock lock;
         lock.l_start = 0;
         lock.l_len = 0; // len = 0 means entire file
         lock.l_type = F_WRLCK;
         lock.l_whence = SEEK_SET;
 
@@ -331,18 +330,17 @@ nsresult nsProfileLock::LockWithSymlink(
         status = PR_GetHostByName(hostname, netdbbuf, sizeof netdbbuf, &hostent);
         if (status == PR_SUCCESS)
             memcpy(&inaddr, hostent.h_addr, sizeof inaddr);
     }
 
     char *signature =
         PR_smprintf("%s:%s%lu", inet_ntoa(inaddr), aHaveFcntlLock ? "+" : "",
                     (unsigned long)getpid());
-    const nsPromiseFlatCString& flat = PromiseFlatCString(lockFilePath);
-    const char *fileName = flat.get();
+    const char *fileName = lockFilePath.get();
     int symlink_rv, symlink_errno = 0, tries = 0;
 
     // use ns4.x-compatible symlinks if the FS supports them
     while ((symlink_rv = symlink(signature, fileName)) < 0)
     {
         symlink_errno = errno;
         if (symlink_errno != EEXIST)
             break;
--- a/toolkit/components/alerts/nsAlertsService.cpp
+++ b/toolkit/components/alerts/nsAlertsService.cpp
@@ -73,22 +73,22 @@ NS_IMETHODIMP nsAlertsService::ShowAlert
                                                      const nsAString & aAlertName)
 {
   if (XRE_GetProcessType() == GeckoProcessType_Content) {
     ContentChild* cpc = ContentChild::GetSingleton();
 
     if (aAlertListener)
       cpc->AddRemoteAlertObserver(PromiseFlatString(aAlertCookie), aAlertListener);
 
-    cpc->SendShowAlertNotification(nsAutoString(aImageUrl),
-                                   nsAutoString(aAlertTitle),
-                                   nsAutoString(aAlertText),
+    cpc->SendShowAlertNotification(PromiseFlatString(aImageUrl),
+                                   PromiseFlatString(aAlertTitle),
+                                   PromiseFlatString(aAlertText),
                                    aAlertTextClickable,
-                                   nsAutoString(aAlertCookie),
-                                   nsAutoString(aAlertName));
+                                   PromiseFlatString(aAlertCookie),
+                                   PromiseFlatString(aAlertName));
     return NS_OK;
   }
 
 #ifdef MOZ_WIDGET_ANDROID
   mozilla::AndroidBridge::Bridge()->ShowAlertNotification(aImageUrl, aAlertTitle, aAlertText, aAlertCookie,
                                                           aAlertListener, aAlertName);
   return NS_OK;
 #else
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -2015,17 +2015,17 @@ History::SetURITitle(nsIURI* aURI, const
 
   if (XRE_GetProcessType() == GeckoProcessType_Content) {
     URIParams uri;
     SerializeURI(aURI, uri);
 
     mozilla::dom::ContentChild * cpc = 
       mozilla::dom::ContentChild::GetSingleton();
     NS_ASSERTION(cpc, "Content Protocol is NULL!");
-    (void)cpc->SendSetURITitle(uri, nsString(aTitle));
+    (void)cpc->SendSetURITitle(uri, PromiseFlatString(aTitle));
     return NS_OK;
   } 
 
   nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
 
   // At first, it seems like nav history should always be available here, no
   // matter what.
   //
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -3802,18 +3802,17 @@ XREMain::XRE_mainRun()
 
   if (!mShuttingDown) {
 #ifdef MOZ_ENABLE_XREMOTE
     // if we have X remote support, start listening for requests on the
     // proxy window.
     if (!mDisableRemote)
       mRemoteService = do_GetService("@mozilla.org/toolkit/remote-service;1");
     if (mRemoteService)
-      mRemoteService->Startup(mAppData->name,
-                              PromiseFlatCString(mProfileName).get());
+      mRemoteService->Startup(mAppData->name, mProfileName.get());
 #endif /* MOZ_ENABLE_XREMOTE */
 
     mNativeApp->Enable();
   }
 
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP
   if (PR_GetEnv("MOZ_INSTRUMENT_EVENT_LOOP") || SAMPLER_IS_ACTIVE()) {
     mozilla::InitEventTracing();
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -1610,20 +1610,21 @@ private:
 
         rv = logFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0644);
         NS_ENSURE_SUCCESS(rv, nullptr);
 #ifdef ANDROID
         {
             // On android the default system umask is 0077 which makes these files
             // unreadable to the shell user. In order to pull the dumps off a non-rooted
             // device we need to chmod them to something world-readable.
+            // XXX why not logFile->SetPermissions(0644);
             nsAutoCString path;
             rv = logFile->GetNativePath(path);
             if (NS_SUCCEEDED(rv)) {
-                chmod(PromiseFlatCString(path).get(), 0644);
+                chmod(path.get(), 0644);
             }
         }
 #endif
 
         return logFile.forget();
     }
 
     FILE *mStream;
--- a/xpcom/base/nsMemoryInfoDumper.cpp
+++ b/xpcom/base/nsMemoryInfoDumper.cpp
@@ -493,20 +493,21 @@ OpenTempFile(const nsACString &aFilename
 
   rv = file->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0644);
   NS_ENSURE_SUCCESS(rv, rv);
 #ifdef ANDROID
   {
     // On android the default system umask is 0077 which makes these files
     // unreadable to the shell user. In order to pull the dumps off a non-rooted
     // device we need to chmod them to something world-readable.
+    // XXX why not logFile->SetPermissions(0644);
     nsAutoCString path;
     rv = file->GetNativePath(path);
     if (NS_SUCCEEDED(rv)) {
-      chmod(PromiseFlatCString(path).get(), 0644);
+      chmod(path.get(), 0644);
     }
   }
 #endif
   return NS_OK;
 }
 
 #ifdef MOZ_DMD
 struct DMDWriteState
--- a/xpcom/string/public/nsTPromiseFlatString.h
+++ b/xpcom/string/public/nsTPromiseFlatString.h
@@ -23,31 +23,31 @@
    *
    *
    * How to use it:
    *
    * A |nsPromiseFlat[C]String| doesn't necessarily own the characters it
    * promises.  You must never use it to promise characters out of a string
    * with a shorter lifespan.  The typical use will be something like this:
    *
-   *   SomeOSFunction( PromiseFlatCString(aCString).get() ); // GOOD
+   *   SomeOSFunction( PromiseFlatCString(aCSubstring).get() ); // GOOD
    *
    * Here's a BAD use:
    *
-   *  const char* buffer = PromiseFlatCString(aCString).get();
+   *  const char* buffer = PromiseFlatCString(aCSubstring).get();
    *  SomeOSFunction(buffer); // BAD!! |buffer| is a dangling pointer
    *
    * The only way to make one is with the function |PromiseFlat[C]String|,
    * which produce a |const| instance.  ``What if I need to keep a promise
    * around for a little while?'' you might ask.  In that case, you can keep a
    * reference, like so
    *
-   *   const nsPromiseFlatString& flat = PromiseFlatString(aString);
+   *   const nsCString& flat = PromiseFlatString(aCSubstring);
    *     // this reference holds the anonymous temporary alive, but remember,
-   *     // it must _still_ have a lifetime shorter than that of |aString|
+   *     // it must _still_ have a lifetime shorter than that of |aCSubstring|
    *
    *  SomeOSFunction(flat.get());
    *  SomeOtherOSFunction(flat.get());
    *
    *
    * How does it work?
    *
    * A |nsPromiseFlat[C]String| is just a wrapper for another string.  If you
@@ -64,20 +64,23 @@ class nsTPromiseFlatString_CharT : publi
 
       typedef nsTPromiseFlatString_CharT    self_type;
 
     private:
 
       void Init( const substring_type& );
 
         // NOT TO BE IMPLEMENTED
-      void operator=( const self_type& );
+      void operator=( const self_type& ) MOZ_DELETE;
 
         // NOT TO BE IMPLEMENTED
-      nsTPromiseFlatString_CharT();
+      nsTPromiseFlatString_CharT() MOZ_DELETE;
+
+        // NOT TO BE IMPLEMENTED
+      nsTPromiseFlatString_CharT( const string_type& str ) MOZ_DELETE;
 
     public:
 
       explicit
       nsTPromiseFlatString_CharT( const substring_type& str )
         : string_type()
         {
           Init(str);
@@ -88,23 +91,16 @@ class nsTPromiseFlatString_CharT : publi
         : string_type()
         {
           // nothing else to do here except assign the value of the tuple
           // into ourselves.
           Assign(tuple);
         }
   };
 
-  // e.g., PromiseFlatCString(Substring(s))
-inline
+// We template this so that the constructor is chosen based on the type of the
+// parameter. This allows us to reject attempts to promise a flat flat string.
+template<class T>
 const nsTPromiseFlatString_CharT
-TPromiseFlatString_CharT( const nsTSubstring_CharT& frag )
+TPromiseFlatString_CharT( const T& string )
   {
-    return nsTPromiseFlatString_CharT(frag);
+    return nsTPromiseFlatString_CharT(string);
   }
-
-  // e.g., PromiseFlatCString(a + b)
-inline
-const nsTPromiseFlatString_CharT
-TPromiseFlatString_CharT( const nsTSubstringTuple_CharT& tuple )
-  {
-    return nsTPromiseFlatString_CharT(tuple);
-  }