Skip to content

Commit

Permalink
Conditionally exclude crossorigin attribute when saving web page reso…
Browse files Browse the repository at this point in the history 
…urces

https://bugs.webkit.org/show_bug.cgi?id=270874
rdar://124074708

Reviewed by Ryosuke Niwa.

When saving complete web page, we may replace URLs of elements with relative paths that point to saved subresource
files. In this case, we should drop crossorigin attribute on these elements, otherwise the saved page may not have
subresources loaded correctly as browsers can perform CORS checks on the element (e.g. requiring response to contain
Access-Control-Allow-Origin header).

Test: WebArchive.SaveResourcesExcludeCrossOriginAttribute

* Source/WebCore/editing/MarkupAccumulator.cpp:
(WebCore::MarkupAccumulator::resolveURLIfNeeded const):
(WebCore::isURLAttributeForElement):
(WebCore::MarkupAccumulator::appendStartTag):
(WebCore::MarkupAccumulator::appendURLAttributeForReplacementIfNecessary):
(WebCore::MarkupAccumulator::appendAttribute):
(WebCore::MarkupAccumulator::appendURLAttributeIfNecessary): Deleted.
* Source/WebCore/editing/MarkupAccumulator.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/CreateWebArchive.mm:

Canonical link: https://commits.webkit.org/276043@main
  • Loading branch information
@szewai
szewai committed Mar 13, 2024
1 parent 5011b2f commit 5b67039
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 18 deletions.
56 changes: 41 additions & 15 deletions Source/WebCore/editing/MarkupAccumulator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,30 +310,30 @@ void MarkupAccumulator::serializeNodesWithNamespaces(Node& targetNode, Serialize
} while (current != &targetNode);
}

String MarkupAccumulator::resolveURLIfNeeded(const Element& element, const String& urlString) const
std::pair<String, MarkupAccumulator::IsCreatedByURLReplacement> MarkupAccumulator::resolveURLIfNeeded(const Element& element, const String& urlString) const
{
if (RefPtr link = dynamicDowncast<HTMLLinkElement>(element); link && !m_replacementURLStringsForCSSStyleSheet.isEmpty()) {
if (RefPtr cssStyleSheet = link->sheet()) {
auto replacementURLString = m_replacementURLStringsForCSSStyleSheet.get(cssStyleSheet);
if (!replacementURLString.isEmpty())
return replacementURLString;
return { replacementURLString, IsCreatedByURLReplacement::Yes };
}
}

if (!m_replacementURLStrings.isEmpty()) {
if (auto frame = frameForAttributeReplacement(element)) {
auto replacementURLString = m_replacementURLStrings.get(frame->frameID().toString());
if (!replacementURLString.isEmpty())
return replacementURLString;
return { replacementURLString, IsCreatedByURLReplacement::Yes };
}

auto resolvedURLString = element.resolveURLStringIfNeeded(urlString);
auto replacementURLString = m_replacementURLStrings.get(resolvedURLString);
if (!replacementURLString.isEmpty())
return replacementURLString;
return { replacementURLString, IsCreatedByURLReplacement::Yes };
}

return element.resolveURLStringIfNeeded(urlString, m_resolveURLs);
return { element.resolveURLStringIfNeeded(urlString, m_resolveURLs), IsCreatedByURLReplacement::No };
}

RefPtr<Element> MarkupAccumulator::replacementElement(const Node& node)
Expand Down Expand Up @@ -525,23 +525,41 @@ static void appendDocumentType(StringBuilder& result, const DocumentType& docume
);
}

static bool isURLAttributeForElement(const Element& element, const Attribute& attribute)
{
return element.isURLAttribute(attribute) || element.isHTMLContentAttribute(attribute);
}

void MarkupAccumulator::appendStartTag(StringBuilder& result, const Element& element, Namespaces* namespaces)
{
appendOpenTag(result, element, namespaces);

bool hasURLAttribute = false;
bool isURLReplaced = false;
Vector<Attribute> attributesToAppendIfURLNotReplaced;

if (element.hasAttributes()) {
for (const Attribute& attribute : element.attributesIterator()) {
if (!hasURLAttribute && (element.isURLAttribute(attribute) || element.isHTMLContentAttribute(attribute)))
if (attribute.name() == crossoriginAttr || attribute.name() == integrityAttr) {
attributesToAppendIfURLNotReplaced.append(attribute);
continue;
}

if (!hasURLAttribute && isURLAttributeForElement(element, attribute))
hasURLAttribute = true;
auto updatedAttribute = replaceAttributeIfNecessary(element, attribute);
appendAttribute(result, element, updatedAttribute, namespaces);
if (appendAttribute(result, element, updatedAttribute, namespaces))
isURLReplaced = true;
}
}

if (!hasURLAttribute)
appendURLAttributeIfNecessary(result, element, namespaces);
if (!hasURLAttribute && appendURLAttributeForReplacementIfNecessary(result, element, namespaces))
isURLReplaced = true;

if (!isURLReplaced) {
for (auto& attribute : attributesToAppendIfURLNotReplaced)
appendAttribute(result, element, attribute, namespaces);
}
// Give an opportunity to subclasses to add their own attributes.
appendCustomAttributes(result, element, namespaces);

Expand Down Expand Up @@ -660,18 +678,21 @@ Attribute MarkupAccumulator::replaceAttributeIfNecessary(const Element& element,
return element.replaceURLsInAttributeValue(attribute, m_replacementURLStrings);
}

void MarkupAccumulator::appendURLAttributeIfNecessary(StringBuilder& result, const Element& element, Namespaces* namespaces)
bool MarkupAccumulator::appendURLAttributeForReplacementIfNecessary(StringBuilder& result, const Element& element, Namespaces* namespaces)
{
auto frame = frameForAttributeReplacement(element);
if (!frame)
return;
return false;

auto replacementURLString = m_replacementURLStrings.get(frame->frameID().toString());
if (!replacementURLString.isNull())
appendAttribute(result, element, Attribute { srcAttr, AtomString { replacementURLString } }, namespaces);
if (!replacementURLString)
return false;

appendAttribute(result, element, Attribute { srcAttr, AtomString { replacementURLString } }, namespaces);
return true;
}

void MarkupAccumulator::appendAttribute(StringBuilder& result, const Element& element, const Attribute& attribute, Namespaces* namespaces)
bool MarkupAccumulator::appendAttribute(StringBuilder& result, const Element& element, const Attribute& attribute, Namespaces* namespaces)
{
bool isSerializingHTML = !inXMLFragmentSerialization();

Expand All @@ -694,13 +715,18 @@ void MarkupAccumulator::appendAttribute(StringBuilder& result, const Element& el
result.append('=');

result.append('"');
bool isURLAttributeValueReplaced = false;
if (element.isURLAttribute(attribute)) {
// FIXME: This does not fully match other browsers. Firefox percent-escapes
// non-ASCII characters for innerHTML.
appendAttributeValue(result, resolveURLIfNeeded(element, attribute.value()), isSerializingHTML);
auto [resolvedURL, isCreatedByURLReplacement] = resolveURLIfNeeded(element, attribute.value());
appendAttributeValue(result, resolvedURL, isSerializingHTML);
isURLAttributeValueReplaced = isCreatedByURLReplacement == IsCreatedByURLReplacement::Yes;
} else
appendAttributeValue(result, attribute.value(), isSerializingHTML);
result.append('"');

return isURLAttributeValueReplaced;
}

void MarkupAccumulator::appendNonElementNode(StringBuilder& result, const Node& node, Namespaces* namespaces)
Expand Down
7 changes: 4 additions & 3 deletions Source/WebCore/editing/MarkupAccumulator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,23 @@ class MarkupAccumulator {
void appendNonElementNode(StringBuilder&, const Node&, Namespaces*);

static void appendAttributeValue(StringBuilder&, const String&, bool isSerializingHTML);
void appendAttribute(StringBuilder&, const Element&, const Attribute&, Namespaces*);
bool appendAttribute(StringBuilder&, const Element&, const Attribute&, Namespaces*);

OptionSet<EntityMask> entityMaskForText(const Text&) const;

Vector<Ref<Node>>* const m_nodes;

private:
void appendNamespace(StringBuilder&, const AtomString& prefix, const AtomString& namespaceURI, Namespaces&, bool allowEmptyDefaultNS = false);
String resolveURLIfNeeded(const Element&, const String&) const;
enum class IsCreatedByURLReplacement : bool { No, Yes };
std::pair<String, IsCreatedByURLReplacement> resolveURLIfNeeded(const Element&, const String&) const;
void serializeNodesWithNamespaces(Node& targetNode, SerializedNodes, const Namespaces*);
bool inXMLFragmentSerialization() const { return m_serializationSyntax == SerializationSyntax::XML; }
void generateUniquePrefix(QualifiedName&, const Namespaces&);
QualifiedName xmlAttributeSerialization(const Attribute&, Namespaces*);
LocalFrame* frameForAttributeReplacement(const Element&) const;
Attribute replaceAttributeIfNecessary(const Element&, const Attribute&);
void appendURLAttributeIfNecessary(StringBuilder&, const Element&, Namespaces*);
bool appendURLAttributeForReplacementIfNecessary(StringBuilder&, const Element&, Namespaces*);
RefPtr<Element> replacementElement(const Node&);
bool shouldExcludeElement(const Element&);

Expand Down
97 changes: 97 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/CreateWebArchive.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1918,6 +1918,103 @@ function onImageLoad() {
Util::run(&saved);
}

static const char* htmlDataBytesForExcludeCrossOriginAttribute = R"TESTRESOURCE(
<head>
<link href="webarchivetest://resource.com/style.css" rel="stylesheet" crossorigin="anonymous">
</head>
<div id="console"></div>
<script src="webarchivetest://resource.com/script.js" integrity="sha256-whXcIuErT+KLyiIBuDxrli97oBljDS1fLlyogfljnnM=" crossorigin="anonymous"></script>
<script>
div = document.getElementById("console");
div.innerHTML += getComputedStyle(div).width;
window.webkit.messageHandlers.testHandler.postMessage("done");
</script>
)TESTRESOURCE";

static const char* scriptDataBytesForExcludeCrossOriginAttribute = R"TESTRESOURCE(document.getElementById("console").innerHTML = "ScriptRuns";)TESTRESOURCE";
static const char* cssDataBytesForExcludeCrossOriginAttribute = R"TESTRESOURCE(div { width: 10px; })TESTRESOURCE";

TEST(WebArchive, SaveResourcesExcludeCrossOriginAttribute)
{
RetainPtr<NSURL> directoryURL = [NSURL fileURLWithPath:[NSTemporaryDirectory() stringByAppendingPathComponent:@"SaveResourcesTest"] isDirectory:YES];
NSFileManager *fileManager = [NSFileManager defaultManager];
[fileManager removeItemAtURL:directoryURL.get() error:nil];

auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
auto schemeHandler = adoptNS([[TestURLSchemeHandler alloc] init]);
[configuration setURLSchemeHandler:schemeHandler.get() forURLScheme:@"webarchivetest"];
NSData *htmlData = [NSData dataWithBytes:htmlDataBytesForExcludeCrossOriginAttribute length:strlen(htmlDataBytesForExcludeCrossOriginAttribute)];
NSData *scriptData = [NSData dataWithBytes:scriptDataBytesForExcludeCrossOriginAttribute length:strlen(scriptDataBytesForExcludeCrossOriginAttribute)];
NSData *cssData = [NSData dataWithBytes:cssDataBytesForExcludeCrossOriginAttribute length:strlen(cssDataBytesForExcludeCrossOriginAttribute)];
[schemeHandler setStartURLSchemeTaskHandler:^(WKWebView *, id<WKURLSchemeTask> task) {
NSData *data = nil;
NSString *mimeType = nil;
bool shouldAddAccessControlHeader = false;
if ([task.request.URL.absoluteString isEqualToString:@"webarchivetest://host/main.html"]) {
mimeType = @"text/html";
data = htmlData;
} else if ([task.request.URL.absoluteString isEqualToString:@"webarchivetest://resource.com/script.js"]) {
mimeType = @"application/javascript";
data = scriptData;
shouldAddAccessControlHeader = true;
} else if ([task.request.URL.absoluteString isEqualToString:@"webarchivetest://resource.com/style.css"]) {
mimeType = @"text/css";
data = cssData;
shouldAddAccessControlHeader = true;
}
EXPECT_TRUE(data);

RetainPtr<NSMutableDictionary> headerFields = adoptNS(@{
@"Content-Length": [NSString stringWithFormat:@"%zu", (size_t)data.length],
@"Content-Type": mimeType,
}.mutableCopy);
if (shouldAddAccessControlHeader)
[headerFields setObject:@"*" forKey:@"Access-Control-Allow-Origin"];

auto response = adoptNS([[NSHTTPURLResponse alloc] initWithURL:task.request.URL statusCode:200 HTTPVersion:nil headerFields:headerFields.get()]);
[task didReceiveResponse:response.get()];
[task didReceiveData:data];
[task didFinish];
}];

auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
static bool messageReceived = false;
[webView performAfterReceivingMessage:@"done" action:[&] {
messageReceived = true;
}];
[webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"webarchivetest://host/main.html"]]];
Util::run(&messageReceived);

static bool saved = false;
[webView _saveResources:directoryURL.get() suggestedFileName:@"host" completionHandler:^(NSError *error) {
EXPECT_NULL(error);
NSString *mainResourcePath = [directoryURL URLByAppendingPathComponent:@"host.html"].path;
EXPECT_TRUE([fileManager fileExistsAtPath:mainResourcePath]);

NSString *savedMainResource = [[NSString alloc] initWithData:[NSData dataWithContentsOfFile:mainResourcePath] encoding:NSUTF8StringEncoding];
EXPECT_TRUE([savedMainResource containsString:@"ScriptRuns"]);
EXPECT_TRUE([savedMainResource containsString:@"10px"]);
EXPECT_FALSE([savedMainResource containsString:@"integrity"]);
EXPECT_FALSE([savedMainResource containsString:@"crossorigin"]);

NSString *resourceDirectoryName = @"host_files";
NSString *scriptFile = @"script.js";
NSString *scriptResourceRelativePath = [resourceDirectoryName stringByAppendingPathComponent:scriptFile];
EXPECT_TRUE([savedMainResource containsString:scriptResourceRelativePath]);
NSString *scriptResourcePath = [directoryURL URLByAppendingPathComponent:scriptResourceRelativePath].path;
EXPECT_TRUE([fileManager fileExistsAtPath:scriptResourcePath]);

NSString *cssFile = @"style.css";
NSString *cssResourceRelativePath = [resourceDirectoryName stringByAppendingPathComponent:cssFile];
EXPECT_TRUE([savedMainResource containsString:cssResourceRelativePath]);
NSString *cssResourcePath = [directoryURL URLByAppendingPathComponent:scriptResourceRelativePath].path;
EXPECT_TRUE([fileManager fileExistsAtPath:cssResourcePath]);

saved = true;
}];
Util::run(&saved);
}

} // namespace TestWebKitAPI

#endif // PLATFORM(MAC) || PLATFORM(IOS_FAMILY)

0 comments on commit 5b67039

Please sign in to comment.