Changeset 27196 in webkit

Timestamp:

Oct 28, 2007, 6:29:48 PM (17 years ago)

Author:

mjs

Message:

JavaScriptCore:

Reviewed by Mark.

• Added assertions to protect against adding empty or deleted keys to a HashTable

• wtf/HashTable.h: (WTF::HashTable::lookup): (WTF::HashTable::lookupForWriting): (WTF::HashTable::fullLookupForWriting): (WTF::HashTable::add):

WebCore:

Reviewed by Mark.

• fixed REGRESSION(r27176): Reproducible crash while trying to order dinner makes bdash sad ​http://bugs.webkit.org/show_bug.cgi?id=15731

• bindings/js/kjs_window.cpp: (KJS::Window::installTimeout): Avoid putting in or accessing empty or deleted keys. (KJS::Window::clearTimeout): ditto
• manual-tests/bad-clearTimeout-crash.html: Added. Automated test not possible.

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/wtf/HashTable.h (modified) (4 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/kjs_window.cpp (modified) (2 diffs)
• WebCore/manual-tests/bad-clearTimeout-crash.html (added)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2007-10-28  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Mark.
+        
+        - Added assertions to protect against adding empty or deleted keys to a HashTable
+
+        * wtf/HashTable.h:
+        (WTF::HashTable::lookup):
+        (WTF::HashTable::lookupForWriting):
+        (WTF::HashTable::fullLookupForWriting):
+        (WTF::HashTable::add):
+
 2007-10-28  Darin Adler  <darin@apple.com>
 

trunk/JavaScriptCore/wtf/HashTable.h

@@ -403 +403 @@
     {
         ASSERT(m_table);
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
 
         int k = 0;
@@ -447 +453 @@
     {
         ASSERT(m_table);
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
 
         int k = 0;
@@ -498 +510 @@
     {
         ASSERT(m_table);
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
 
         int k = 0;
@@ -548 +566 @@
     inline pair<typename HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::iterator, bool> HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::add(const T& key, const Extra& extra)
     {
+#ifndef ASSERT_DISABLED
+        if (HashFunctions::safeToCompareToEmptyOrDeleted) {
+            ASSERT(!HashTranslator::equal(KeyTraits::emptyValue(), key));
+            ASSERT(!HashTranslator::equal(KeyTraits::deletedValue(), key));
+        }
+#endif
+
         invalidateIterators();
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-10-28  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Mark.
+
+        - fixed REGRESSION(r27176): Reproducible crash while trying to order dinner makes bdash sad
+        http://bugs.webkit.org/show_bug.cgi?id=15731
+
+        * bindings/js/kjs_window.cpp:
+        (KJS::Window::installTimeout): Avoid putting in or accessing empty or deleted keys.
+        (KJS::Window::clearTimeout): ditto
+        * manual-tests/bad-clearTimeout-crash.html: Added. Automated test not possible.
+
 2007-10-28  Kevin Ollivier  <kevino@theolliviers.com>
 

trunk/WebCore/bindings/js/kjs_window.cpp

@@ -1522 +1522 @@
 {
     int timeoutId = ++lastUsedTimeoutId;
+
+    // avoid wraparound going negative on us
+    if (timeoutId <= 0)
+        timeoutId = 1;
+
     int nestLevel = timerNestingLevel + 1;
     DOMWindowTimer* timer = new DOMWindowTimer(timeoutId, nestLevel, this, a);
@@ -1593 +1598 @@
 void Window::clearTimeout(int timeoutId, bool delAction)
 {
+    // timeout IDs have to be positive, and 0 and -1 are unsafe to
+    // even look up since they are the empty and deleted value
+    // respectively
+    if (timeoutId <= 0)
+        return;
+
     WindowPrivate::TimeoutsMap::iterator it = d->m_timeouts.find(timeoutId);
     if (it == d->m_timeouts.end())