Skip to content

Commit

Permalink
Cherry-pick 0ba9e6c. rdar://problem/111161160
Browse files Browse the repository at this point in the history
    WebPage_LoadRequest IPC fails decoding in PingDuoDuo app
    https://bugs.webkit.org/show_bug.cgi?id=258486
    rdar://111161160

    Reviewed by Tim Horton.

    WebPage_LoadRequest IPC was failing decoding in PingDuoDuo app.
    The issue was due to getting a WKSecureCodingURLWrapper instead
    of a NSURL when decoding the baseURL of a NSURL.

    I am not sure how we ended up in this situation but I made the
    bug go away by simplifying the code. The coder used to encode
    the URL in two parts:
    1. The baseURL
    2. The bytes from the URL's relative string

    Then it would decode the URL in 2 parts:
    1. The baseURL
    2. The bytes from the URL's relative string

    It would then call CFURLCreateAbsoluteURLWithBytes() with those
    2 parts, which would result in an *absolute* URL. The information
    about baseURL / relative string would be lost.

    As a result, I have decided to simply encode the URL in one part,
    the absolute URL bytes. The decoding results ends up being the
    same (an absolute URL). It simplifies both coding and decoding
    and makes the bug go away since it was about decoding baseURLs.

    * Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm:
    (-[WKSecureCodingURLWrapper encodeWithCoder:]):
    (-[WKSecureCodingURLWrapper initWithCoder:]):
    * Tools/TestWebKitAPI/Tests/WebKitCocoa/IPCTestingAPI.mm:

    Canonical link: https://commits.webkit.org/265529@main

Canonical link: https://commits.webkit.org/265500.2@safari-7616.1.21-branch
  • Loading branch information
cdumez authored and rjepstein committed Jun 27, 2023
1 parent 5313333 commit 055a741
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 16 deletions.
18 changes: 2 additions & 16 deletions Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,7 @@ + (BOOL)supportsSecureCoding
- (void)encodeWithCoder:(NSCoder *)coder
{
RELEASE_ASSERT(m_wrappedURL);
auto baseURL = m_wrappedURL.get().baseURL;
BOOL hasBaseURL = !!baseURL;

[coder encodeValueOfObjCType:"c" at:&hasBaseURL];
if (hasBaseURL)
[coder encodeObject:baseURL forKey:baseURLKey];

auto bytes = bytesAsVector(bridge_cast(m_wrappedURL.get()));
auto bytes = bytesAsVector(bridge_cast([m_wrappedURL.get() absoluteURL]));
[coder encodeBytes:bytes.data() length:bytes.size()];
}

Expand All @@ -191,16 +184,9 @@ - (instancetype)initWithCoder:(NSCoder *)coder
if (!selfPtr)
return nil;

BOOL hasBaseURL;
[coder decodeValueOfObjCType:"c" at:&hasBaseURL size:sizeof(hasBaseURL)];

RetainPtr<NSURL> baseURL;
if (hasBaseURL)
baseURL = (NSURL *)[coder decodeObjectOfClass:NSURL.class forKey:baseURLKey];

NSUInteger length;
if (auto bytes = (UInt8 *)[coder decodeBytesWithReturnedLength:&length]) {
m_wrappedURL = bridge_cast(adoptCF(CFURLCreateAbsoluteURLWithBytes(nullptr, bytes, length, kCFStringEncodingUTF8, bridge_cast(baseURL.get()), true)));
m_wrappedURL = bridge_cast(adoptCF(CFURLCreateAbsoluteURLWithBytes(nullptr, bytes, length, kCFStringEncodingUTF8, nullptr, true)));
if (!m_wrappedURL)
LOG_ERROR("Failed to decode NSURL due to invalid encoding of length %d. Substituting a blank URL", length);
}
Expand Down
45 changes: 45 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/IPCTestingAPI.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#import "config.h"

#import "DeprecatedGlobalValues.h"
#import "PlatformUtilities.h"
#import "RemoteObjectRegistry.h"
#import "TestWKWebView.h"
#import "Utilities.h"
Expand Down Expand Up @@ -649,3 +650,47 @@ - (BOOL)sayHelloWasCalled
[unarchiver finishDecoding];
unarchiver.get().delegate = nil;
}

TEST(IPCTestingAPI, NSURLWithBaseURLInNSSecureCoding)
{
auto archiver = adoptNS([[NSKeyedArchiver alloc] initRequiringSecureCoding:YES]);

RetainPtr<id<NSKeyedArchiverDelegate, NSKeyedUnarchiverDelegate>> delegate = adoptNS([[NSClassFromString(@"WKSecureCodingArchivingDelegate") alloc] init]);
archiver.get().delegate = delegate.get();

NSString *key = @"SomeString";
NSURL *value = [NSURL URLWithString:@"/garden_home.html" relativeToURL:[NSURL URLWithString:@"amcomponent://com.xunmeng.pinduoduo/"]];
EXPECT_WK_STREQ(value.baseURL.absoluteString, @"amcomponent://com.xunmeng.pinduoduo/");
EXPECT_WK_STREQ(value.relativeString, @"/garden_home.html");
EXPECT_WK_STREQ(value.absoluteString, @"amcomponent://com.xunmeng.pinduoduo/garden_home.html");

auto payload = @{ key : static_cast<id>(value) };
[archiver encodeObject:payload forKey:NSKeyedArchiveRootObjectKey];
[archiver finishEncoding];
[archiver setDelegate:nil];

auto data = [archiver encodedData];

auto unarchiver = adoptNS([[NSKeyedUnarchiver alloc] initForReadingFromData:data error:nullptr]);
unarchiver.get().decodingFailurePolicy = NSDecodingFailurePolicyRaiseException;
unarchiver.get().delegate = delegate.get();

auto allowedClassSet = adoptNS([NSMutableSet new]);
[allowedClassSet addObject:NSDictionary.class];
[allowedClassSet addObject:NSString.class];
[allowedClassSet addObject:NSClassFromString(@"WKSecureCodingURLWrapper")];

NSDictionary *result = [unarchiver decodeObjectOfClasses:allowedClassSet.get() forKey:NSKeyedArchiveRootObjectKey];

EXPECT_EQ(result.count, static_cast<NSUInteger>(1));
NSString *resultKey = result.allKeys[0];
EXPECT_TRUE([key isEqual:resultKey]);
NSURL *resultValue = (NSURL *)(result.allValues[0]);

// Our coder resolves the URL so we end up with an absolute URL instead of base URL + relative string.
EXPECT_WK_STREQ(resultValue.baseURL.absoluteString, @"");
EXPECT_WK_STREQ(resultValue.baseURL.relativeString, @"");
EXPECT_WK_STREQ(resultValue.absoluteString, @"amcomponent://com.xunmeng.pinduoduo/garden_home.html");
[unarchiver finishDecoding];
unarchiver.get().delegate = nil;
}

0 comments on commit 055a741

Please sign in to comment.