Changeset 150853 in webkit


Ignore:
Timestamp:
May 28, 2013 4:26:48 PM (11 years ago)
Author:
ap@apple.com
Message:

Freeze when loading a particular page on washingtonpost.com with NetworkProcess enabled
​https://bugs.webkit.org/show_bug.cgi?id=116887
<rdar://problem/12965959>

Reviewed by Darin Adler.

  • Shared/cf/ArgumentCodersCF.cpp: (CoreIPC::encode): (CoreIPC::decode): Fix for the bug: use good shared WebCore code to manipulate CFURLs. Also, added a FIXME about empty URLs.
  • Shared/API/c/cf/WKURLCF.cpp: (WKURLCreateWithCFURL): Fixed to not use CFURLGetString, as that could prevent loading certain resources, like ones with curly braces in resource specifiers. We want KURL normalization, not CFURL one.
Location:
trunk
Files:
5 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r150849 r150853  
     12013-05-28  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Freeze when loading a particular page on washingtonpost.com with NetworkProcess enabled
     4        https://bugs.webkit.org/show_bug.cgi?id=116887
     5        <rdar://problem/12965959>
     6
     7        Reviewed by Darin Adler.
     8
     9        * http/tests/uri/curly-braces-escaping-expected.txt: Added.
     10        * http/tests/uri/curly-braces-escaping.html: Added.
     11        * http/tests/uri/resources/echo-uri.php: Added.
     12
    1132013-05-28  Brent Fulgham  <bfulgham@apple.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r150837 r150853  
     12013-05-28  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Freeze when loading a particular page on washingtonpost.com with NetworkProcess enabled
     4        https://bugs.webkit.org/show_bug.cgi?id=116887
     5        <rdar://problem/12965959>
     6
     7        Reviewed by Darin Adler.
     8
     9        Test: http/tests/uri/curly-braces-escaping.html
     10
     11        The issue was that WebKit used an incorrect accessor to get a string out of CFURL,
     12        unlike WebCore.
     13
     14        Centralized code for converting between CFURLs and strings in a place that WebKit
     15        can use directly.
     16
     17        * WebCore.exp.in:
     18        * WebCore.vcproj/WebCore.vcproj:
     19        * WebCore.vcxproj/WebCore.vcxproj:
     20        * WebCore.vcxproj/WebCore.vcxproj.filters:
     21        * WebCore.xcodeproj/project.pbxproj:
     22        Keeping things building.
     23
     24        * platform/KURL.cpp: (WebCore::KURL::copyToBuffer):
     25        * platform/KURL.h:
     26        Updated for a new typedef name. The code was using Vector<char, 512> and CharBuffer
     27        inconsistently, and now URLs use URLCharBuffer.
     28
     29        * platform/cf/CFURLExtras.cpp: Added.
     30        (WebCore::createCFURLFromBuffer):
     31        (WebCore::getURLBytes):
     32        * platform/cf/CFURLExtras.h: Added.
     33        * platform/cf/KURLCFNet.cpp:
     34        (WebCore::KURL::KURL):
     35        (WebCore::KURL::createCFURL):
     36        Extracted code for CFURL string manipulation that is not logcally part of KURL.
     37
     38        * platform/mac/KURLMac.mm:
     39        (WebCore::KURL::KURL): Use the new getURLBytes function.
     40        (WebCore::KURL::operator NSURL *): Added a comment.
     41        (WebCore::KURL::createCFURL): Added a comment, and updated for new names and
     42        signatures.
     43
     44        * platform/network/cf/ResourceErrorCF.cpp: (WebCore::ResourceError::platformLazyInit):
     45        Added a FIXME. We are still using CFURLGetString here. The difficulty is getting
     46        encodings right.
     47
    1482013-05-28  Anders Carlsson  <andersca@apple.com>
    249

  • trunk/Source/WebCore/WebCore.exp.in

    r150796 r150853  
    168168__ZN7WebCore11SQLResultOkE
    169169__ZN7WebCore11URLWithDataEP6NSDataP5NSURL
     170__ZN7WebCore11getURLBytesEPK7__CFURLRN3WTF7CStringE
     171__ZN7WebCore11getURLBytesEPK7__CFURLRN3WTF6VectorIcLm512ENS3_15CrashOnOverflowEEE
    170172__ZN7WebCore11iBeamCursorEv
    171173__ZN7WebCore11memoryCacheEv
    … …  
    718720__ZN7WebCore21UserContentURLPattern5parseERKN3WTF6StringE
    719721__ZN7WebCore21WindowsLatin1EncodingEv
     722__ZN7WebCore21createCFURLFromBufferEPKcmPK7__CFURL
    720723__ZN7WebCore21findEventWithKeyStateEPNS_5EventE
    721724__ZN7WebCore21getCachedDOMStructureEPNS_17JSDOMGlobalObjectEPKN3JSC9ClassInfoE

  • trunk/Source/WebCore/WebCore.vcproj/WebCore.vcproj

    r150837 r150853  
    2972329723                                >
    2972429724                                <File
     29725                                        RelativePath="..\platform\cf\CFURLExtras.cpp"
     29726                                        >
     29727                                </File>
     29728                                <File
     29729                                        RelativePath="..\platform\cf\CFURLExtras.h"
     29730                                        >
     29731                                </File>
     29732                                <File
    2972529733                                        RelativePath="..\platform\cf\FileSystemCF.cpp"
    2972629734                                        >

  • trunk/Source/WebCore/WebCore.vcxproj/WebCore.vcxproj

    r150837 r150853  
    42224222    <ClCompile Include="..\platform\win\WidgetWin.cpp" />
    42234223    <ClCompile Include="..\platform\win\WindowMessageBroadcaster.cpp" />
     4224    <ClCompile Include="..\platform\cf\CFURLExtras.cpp" />
    42244225    <ClCompile Include="..\platform\cf\FileSystemCF.cpp" />
    42254226    <ClCompile Include="..\platform\cf\KURLCFNet.cpp" />
    … …  
    1160911610    <ClInclude Include="..\platform\win\WindowMessageListener.h" />
    1161011611    <ClInclude Include="..\platform\win\WindowsTouch.h" />
     11612    <ClInclude Include="..\platform\cf\CFURLExtras.h" />
    1161111613    <ClInclude Include="..\platform\cf\win\CertificateCFWin.h" />
    1161211614    <ClInclude Include="..\platform\graphics\BitmapImage.h" />

  • trunk/Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters

    r150837 r150853  
    51795179      <Filter>page</Filter>
    51805180    </ClCompile>
     5181    <ClCompile Include="..\platform\cf\CFURLExtras.cpp">
     5182      <Filter>platform\cf</Filter>
     5183    </ClCompile>
    51815184    <ClCompile Include="..\platform\cf\SharedBufferCF.cpp">
    51825185      <Filter>platform\cf</Filter>
    … …  
    1268212685    <ClInclude Include="..\svg\SVGAnimatedTypeAnimator.h">
    1268312686      <Filter>rendering\svg</Filter>
     12687    </ClInclude>
     12688    <ClInclude Include="..\platform\cf\CFURLExtras.h">
     12689      <Filter>platform\cf</Filter>
    1268412690    </ClInclude>
    1268512691    <ClInclude Include="..\platform\cf\win\CertificateCFWin.h">

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r150837 r150853  
    54205420                E1A643F20EC0972500779668 /* WorkerScriptController.h in Headers */ = {isa = PBXBuildFile; fileRef = E1A643F10EC0972500779668 /* WorkerScriptController.h */; };
    54215421                E1A643FD0EC097A000779668 /* WorkerScriptController.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1A643FC0EC097A000779668 /* WorkerScriptController.cpp */; };
     5422                E1A8E56617552B2A007488E7 /* CFURLExtras.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1A8E56417552B2A007488E7 /* CFURLExtras.cpp */; };
     5423                E1A8E56717552B2A007488E7 /* CFURLExtras.h in Headers */ = {isa = PBXBuildFile; fileRef = E1A8E56517552B2A007488E7 /* CFURLExtras.h */; settings = {ATTRIBUTES = (Private, ); }; };
    54225424                E1ACAF4C0E791AAF0087D12B /* DOMMessagePort.mm in Sources */ = {isa = PBXBuildFile; fileRef = E1ACAF4A0E791AAF0087D12B /* DOMMessagePort.mm */; };
    54235425                E1ACAF4D0E791AAF0087D12B /* DOMMessagePort.h in Headers */ = {isa = PBXBuildFile; fileRef = E1ACAF4B0E791AAF0087D12B /* DOMMessagePort.h */; };
    … …  
    1220212204                E1A643F10EC0972500779668 /* WorkerScriptController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WorkerScriptController.h; sourceTree = "<group>"; };
    1220312205                E1A643FC0EC097A000779668 /* WorkerScriptController.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkerScriptController.cpp; sourceTree = "<group>"; };
     12206                E1A8E56417552B2A007488E7 /* CFURLExtras.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CFURLExtras.cpp; sourceTree = "<group>"; };
     12207                E1A8E56517552B2A007488E7 /* CFURLExtras.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CFURLExtras.h; sourceTree = "<group>"; };
    1220412208                E1AB1EA814E9E27D00449E13 /* FileList.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = FileList.idl; path = fileapi/FileList.idl; sourceTree = "<group>"; };
    1220512209                E1AB1EAB14E9E2EF00449E13 /* DirectoryEntry.idl */ = {isa = PBXFileReference; lastKnownFileType = text; name = DirectoryEntry.idl; path = Modules/filesystem/DirectoryEntry.idl; sourceTree = "<group>"; };
    … …  
    1340513409                        isa = PBXGroup;
    1340613410                        children = (
     13411                                E1A8E56417552B2A007488E7 /* CFURLExtras.cpp */,
     13412                                E1A8E56517552B2A007488E7 /* CFURLExtras.h */,
    1340713413                                5160306B0CC4362300C8AC25 /* FileSystemCF.cpp */,
    1340813414                                1A98956A0AA78F80005EF5EF /* KURLCFNet.cpp */,
    … …  
    2091720923                                29D7BCFA1444AF7D0070619C /* AccessibilitySpinButton.h in Headers */,
    2091820924                                AAC08CF315F941FD00F1E188 /* AccessibilitySVGRoot.h in Headers */,
     20925                                E1A8E56717552B2A007488E7 /* CFURLExtras.h in Headers */,
    2091920926                                29A8122B0FBB9C1D00510293 /* AccessibilityTable.h in Headers */,
    2092020927                                29A812320FBB9C1D00510293 /* AccessibilityTableCell.h in Headers */,
    … …  
    2620026207                                1AF8E1C3125673E000230FF7 /* ProxyServerCFNet.cpp in Sources */,
    2620126208                                FF945ECB161F7F3600971BC8 /* PseudoElement.cpp in Sources */,
     26209                                E1A8E56617552B2A007488E7 /* CFURLExtras.cpp in Sources */,
    2620226210                                0081FEFF16B0A2B6008AAA7A /* PublicSuffixMac.mm in Sources */,
    2620326211                                E4D687770ED7AE3D006EA978 /* PurgeableBufferMac.cpp in Sources */,

  • trunk/Source/WebCore/platform/KURL.cpp

    r149925 r150853  
    16961696}
    16971697
    1698 void KURL::copyToBuffer(CharBuffer& buffer) const
     1698void KURL::copyToBuffer(Vector<char, 512>& buffer) const
    16991699{
    17001700    // FIXME: This throws away the high bytes of all the characters in the string!

  • trunk/Source/WebCore/platform/cf/KURLCFNet.cpp

    r150606 r150853  
    2727#include "KURL.h"
    2828
    29 #include <wtf/RetainPtr.h>
     29#include "CFURLExtras.h"
    3030#include <CoreFoundation/CFURL.h>
     31#include <wtf/text/CString.h>
    3132
    3233using namespace std;
    3334
    3435namespace WebCore {
    35 
    36 typedef Vector<char, 512> CharBuffer;
    37 
    38 RetainPtr<CFURLRef> createCFURLFromBuffer(const CharBuffer&);
    3936
    4037KURL::KURL(CFURLRef url)
    … …  
    4542    }
    4643
    47     CFIndex bytesLength = CFURLGetBytes(url, 0, 0);
    48     Vector<char, 512> buffer(bytesLength + 1);
    49     char* bytes = &buffer[0];
    50     CFURLGetBytes(url, reinterpret_cast<UInt8*>(bytes), bytesLength);
    51     bytes[bytesLength] = '\0';
    52     parse(bytes);
    53 }
    54 
    55 RetainPtr<CFURLRef> createCFURLFromBuffer(const CharBuffer& buffer)
    56 {
    57     // NOTE: We use UTF-8 here since this encoding is used when computing strings when returning URL components
    58     // (e.g calls to NSURL -path). However, this function is not tolerant of illegal UTF-8 sequences, which
    59     // could either be a malformed string or bytes in a different encoding, like Shift-JIS, so we fall back
    60     // onto using ISO Latin-1 in those cases.
    61     RetainPtr<CFURLRef> result = adoptCF(CFURLCreateAbsoluteURLWithBytes(0, reinterpret_cast<const UInt8*>(buffer.data()), buffer.size(), kCFStringEncodingUTF8, 0, true));
    62     if (!result)
    63         result = adoptCF(CFURLCreateAbsoluteURLWithBytes(0, reinterpret_cast<const UInt8*>(buffer.data()), buffer.size(), kCFStringEncodingISOLatin1, 0, true));
    64     return result;
     44    // FIXME: Why is it OK to ignore base URL here?
     45    CString urlBytes;
     46    getURLBytes(url, urlBytes);
     47    parse(urlBytes.data());
    6548}
    6649
    … …  
    7154    // Currently it throws away the high bytes of the characters in the string in that case,
    7255    // which is clearly wrong.
    73     CharBuffer buffer;
     56    URLCharBuffer buffer;
    7457    copyToBuffer(buffer);
    7558    return createCFURLFromBuffer(buffer);

  • trunk/Source/WebCore/platform/mac/KURLMac.mm

    r150606 r150853  
    2727#import "KURL.h"
    2828
     29#import "CFURLExtras.h"
    2930#import "FoundationExtras.h"
    30 #import <CoreFoundation/CFURL.h>
    31 
    32 using namespace WTF;
     31#import <wtf/text/CString.h>
    3332
    3433namespace WebCore {
    35 
    36 typedef Vector<char, 512> CharBuffer;
    37 extern RetainPtr<CFURLRef> createCFURLFromBuffer(const CharBuffer& buffer);
    3834
    3935KURL::KURL(NSURL *url)
    … …  
    4440    }
    4541
    46     CFIndex bytesLength = CFURLGetBytes(reinterpret_cast<CFURLRef>(url), 0, 0);
    47     Vector<char, 512> buffer(bytesLength + 1);
    48     char* bytes = &buffer[0];
    49     CFURLGetBytes(reinterpret_cast<CFURLRef>(url), reinterpret_cast<UInt8*>(bytes), bytesLength);
    50     bytes[bytesLength] = '\0';
    51     parse(bytes);
     42    // FIXME: Why is it OK to ignore base URL here?
     43    CString urlBytes;
     44    getURLBytes(reinterpret_cast<CFURLRef>(url), urlBytes);
     45    parse(urlBytes.data());
    5246}
    5347
    5448KURL::operator NSURL *() const
    5549{
     50    // Creating a toll-free bridged CFURL, because a real NSURL would not preserve the original string.
     51    // We'll need fidelity when round-tripping via CFURLGetBytes().
    5652    return HardAutorelease(createCFURL().leakRef());
    5753}
    5854
    59 // We use the toll-free bridge between NSURL and CFURL to
    60 // create a CFURLRef supporting both empty and null values.
    6155RetainPtr<CFURLRef> KURL::createCFURL() const
    6256{
    … …  
    6559
    6660    if (isEmpty()) {
     61        // We use the toll-free bridge between NSURL and CFURL to
     62        // create a CFURLRef supporting both empty and null values.
    6763        RetainPtr<NSURL> emptyNSURL = adoptNS([[NSURL alloc] initWithString:@""]);
    6864        return reinterpret_cast<CFURLRef>(emptyNSURL.get());
    6965    }
    7066
    71     CharBuffer buffer;
     67    URLCharBuffer buffer;
    7268    copyToBuffer(buffer);
    73     return createCFURLFromBuffer(buffer);
     69    return createCFURLFromBuffer(buffer.data(), buffer.size());
    7470}
    7571

  • trunk/Source/WebCore/platform/network/cf/ResourceErrorCF.cpp

    r149980 r150853  
    108108                RetainPtr<CFURLRef> absoluteURLRef = adoptCF(CFURLCopyAbsoluteURL(failingURL));
    109109                if (absoluteURLRef.get()) {
     110                    // FIXME: CFURLGetString returns a normalized URL which is different from what is actually used by CFNetwork.
     111                    // We should use CFURLGetBytes instead.
    110112                    failingURLString = CFURLGetString(absoluteURLRef.get());
    111113                    m_failingURL = String(failingURLString);

  • trunk/Source/WebKit2/ChangeLog

    r150838 r150853  
     12013-05-28  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Freeze when loading a particular page on washingtonpost.com with NetworkProcess enabled
     4        https://bugs.webkit.org/show_bug.cgi?id=116887
     5        <rdar://problem/12965959>
     6
     7        Reviewed by Darin Adler.
     8
     9        * Shared/cf/ArgumentCodersCF.cpp:
     10        (CoreIPC::encode):
     11        (CoreIPC::decode):
     12        Fix for the bug: use good shared WebCore code to manipulate CFURLs. Also, added
     13        a FIXME about empty URLs.
     14
     15        * Shared/API/c/cf/WKURLCF.cpp: (WKURLCreateWithCFURL): Fixed to not use
     16        CFURLGetString, as that could prevent loading certain resources, like ones with
     17        curly braces in resource specifiers. We want KURL normalization, not CFURL one.
     18
    1192013-05-28  Tim Horton  <timothy_horton@apple.com>
    220

  • trunk/Source/WebKit2/Shared/API/c/cf/WKURLCF.cpp

    r130948 r150853  
    2828
    2929#include "WKAPICast.h"
     30#include <WebCore/CFURLExtras.h>
    3031#include <wtf/PassRefPtr.h>
    3132#include <wtf/RefPtr.h>
    32 #include <wtf/RetainPtr.h>
    3333#include <wtf/text/CString.h>
    3434#include <wtf/text/WTFString.h>
    … …  
    4242        return 0;
    4343
    44     String urlString(CFURLGetString(cfURL));
    45     return toCopiedURLAPI(urlString);
     44    CString urlBytes;
     45    getURLBytes(cfURL, urlBytes);
     46
     47    return toCopiedURLAPI(urlBytes.data());
    4648}
    4749

  • trunk/Source/WebKit2/Shared/cf/ArgumentCodersCF.cpp

    r149255 r150853  
    3030#include "ArgumentEncoder.h"
    3131#include "DataReference.h"
     32#include <WebCore/CFURLExtras.h>
    3233#include <wtf/Vector.h>
    3334
    … …  
    3536#import <Foundation/Foundation.h>
    3637#endif
     38
     39using namespace WebCore;
    3740
    3841namespace CoreIPC {
    … …  
    501504        encode(encoder, baseURL);
    502505
    503     encode(encoder, CFURLGetString(url));
     506    URLCharBuffer urlBytes;
     507    getURLBytes(url, urlBytes);
     508    CoreIPC::DataReference dataReference(reinterpret_cast<const uint8_t*>(urlBytes.data()), urlBytes.size());
     509    encoder << dataReference;
    504510}
    505511
    … …  
    515521    }
    516522
    517     RetainPtr<CFStringRef> string;
    518     if (!decode(decoder, string))
     523    CoreIPC::DataReference urlBytes;
     524    if (!decoder.decode(urlBytes))
    519525        return false;
    520526
    … …  
    522528    // FIXME: Move this to ArgumentCodersCFMac.mm and change this file back to be C++
    523529    // instead of Objective-C++.
    524     if (!CFStringGetLength(string.get())) {
     530    if (urlBytes.isEmpty()) {
    525531        // CFURL can't hold an empty URL, unlike NSURL.
     532        // FIXME: This discards base URL, which seems incorrect.
    526533        result = reinterpret_cast<CFURLRef>([NSURL URLWithString:@""]);
    527534        return true;
    528535    }
    529536#endif
    530                    
    531     CFURLRef url = CFURLCreateWithString(0, string.get(), baseURL.get());
    532     if (!url)
    533         return false;
    534 
    535     result = adoptCF(url);
    536     return true;
     537
     538    result = createCFURLFromBuffer(reinterpret_cast<const char*>(urlBytes.data()), urlBytes.size(), baseURL.get());
     539    return result;
    537540}
    538541
Note: See TracChangeset for help on using the changeset viewer.