Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-sync URL from WebKit + set ERR_MISSING_ARGS #10129

Merged
merged 8 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions bench/snippets/url.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { bench, run } from "./runner.mjs";

bench(`new URL('https://example.com/')`, () => {
const url = new URL("https://example.com/");
});

bench(`new URL('https://example.com')`, () => {
const url = new URL("https://example.com");
});

bench(`new URL('https://www.example.com')`, () => {
const url = new URL("https://www.example.com");
});

bench(`new URL('https://www.example.com/')`, () => {
const url = new URL("https://www.example.com/");
});

await run();
109 changes: 69 additions & 40 deletions src/bun.js/bindings/DOMURL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,42 +23,50 @@
* Boston, MA 02110-1301, USA.
*/

#include "config.h"
#include "DOMURL.h"

// #include "ActiveDOMObject.h"
#include "ActiveDOMObject.h"
// #include "Blob.h"
// #include "BlobURL.h"
// #include "MemoryCache.h"
// #include "PublicURLManager.h"
// #include "ResourceRequest.h"
#include "ScriptExecutionContext.h"
// #include "SecurityOrigin.h"
#include "URLSearchParams.h"
// #include <wtf/MainThread.h>
#include <wtf/MainThread.h>

class URLRegistrable {
public:
};

class Blob {
public:
};

namespace WebCore {

static inline String redact(const String& input)
{
if (input.contains("@"_s))
if (input.contains('@'))
return "<redacted>"_s;

return makeString('"', input, '"');
}

inline DOMURL::DOMURL(URL&& completeURL, const URL& baseURL)
: m_baseURL(baseURL)
, m_url(WTFMove(completeURL))
inline DOMURL::DOMURL(URL&& completeURL)
: m_url(WTFMove(completeURL))
{
ASSERT(m_url.isValid());
}

DOMURL::~DOMURL() = default;

bool DOMURL::canParse(const String& url, const String& base)
ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url)
{
URL baseURL { base };
if (!base.isNull() && !baseURL.isValid())
return false;
URL completeURL { baseURL, url };
return completeURL.isValid();
URL completeURL { url };
if (!completeURL.isValid())
return Exception { TypeError, makeString(redact(url), " cannot be parsed as a URL.") };
return adoptRef(*new DOMURL(WTFMove(completeURL)));
}

ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const URL& base)
Expand All @@ -67,7 +75,7 @@ ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const URL& base)
URL completeURL { base, url };
if (!completeURL.isValid())
return Exception { TypeError, makeString(redact(url), " cannot be parsed as a URL.") };
return adoptRef(*new DOMURL(WTFMove(completeURL), base));
return adoptRef(*new DOMURL(WTFMove(completeURL)));
}

ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const String& base)
Expand All @@ -78,9 +86,27 @@ ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const String& base)
return create(url, baseURL);
}

ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const DOMURL& base)
DOMURL::~DOMURL() = default;

static URL parseInternal(const String& url, const String& base)
{
URL baseURL { base };
if (!base.isNull() && !baseURL.isValid())
return {};
return { baseURL, url };
}

RefPtr<DOMURL> DOMURL::parse(const String& url, const String& base)
{
return create(url, base.href());
auto completeURL = parseInternal(url, base);
if (!completeURL.isValid())
return {};
return adoptRef(*new DOMURL(WTFMove(completeURL)));
}

bool DOMURL::canParse(const String& url, const String& base)
{
return parseInternal(url, base).isValid();
}

ExceptionOr<void> DOMURL::setHref(const String& url)
Expand All @@ -96,26 +122,27 @@ ExceptionOr<void> DOMURL::setHref(const String& url)
return {};
}

void DOMURL::setQuery(const String& query)
String DOMURL::createObjectURL(ScriptExecutionContext& scriptExecutionContext, Blob& blob)
{
m_url.setQuery(query);
UNUSED_PARAM(blob);
UNUSED_PARAM(scriptExecutionContext);
return String();
// return createPublicURL(scriptExecutionContext, blob);
}

// String DOMURL::createObjectURL(Blob& blob)
// {
// return createPublicURL(scriptExecutionContext, blob);
// }

// String DOMURL::createPublicURL(URLRegistrable& registrable)
// {
// URL publicURL = BlobURL::createPublicURL(scriptExecutionContext.securityOrigin());
// if (publicURL.isEmpty())
// return String();
String DOMURL::createPublicURL(ScriptExecutionContext& scriptExecutionContext, URLRegistrable& registrable)
{
// URL publicURL = BlobURL::createPublicURL(scriptExecutionContext.securityOrigin());
// if (publicURL.isEmpty())
// return String();

// scriptExecutionContext.publicURLManager().registerURL(publicURL, registrable);
// scriptExecutionContext.publicURLManager().registerURL(publicURL, registrable);

// return publicURL.string();
// }
// return publicURL.string();
UNUSED_PARAM(scriptExecutionContext);
UNUSED_PARAM(registrable);
return String();
}

URLSearchParams& DOMURL::searchParams()
{
Expand All @@ -124,15 +151,17 @@ URLSearchParams& DOMURL::searchParams()
return *m_searchParams;
}

// void DOMURL::revokeObjectURL(const String& urlString)
// {
// // URL url(URL(), urlString);
// // ResourceRequest request(url);
// // request.setDomainForCachePartition(scriptExecutionContext.domainForCachePartition());
void DOMURL::revokeObjectURL(ScriptExecutionContext& scriptExecutionContext, const String& urlString)
{
// URL url { urlString };
// ResourceRequest request(url);
// request.setDomainForCachePartition(scriptExecutionContext.domainForCachePartition());

// // MemoryCache::removeRequestFromSessionCaches(scriptExecutionContext, request);
// MemoryCache::removeRequestFromSessionCaches(scriptExecutionContext, request);

// // scriptExecutionContext.publicURLManager().revoke(url);
// }
// scriptExecutionContext.publicURLManager().revoke(url);
UNUSED_PARAM(scriptExecutionContext);
UNUSED_PARAM(urlString);
}

} // namespace WebCore
19 changes: 11 additions & 8 deletions src/bun.js/bindings/DOMURL.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,36 +35,39 @@

namespace WebCore {

class Blob;
class ScriptExecutionContext;
class URLRegistrable;
class URLSearchParams;

class DOMURL final : public RefCounted<DOMURL>, public CanMakeWeakPtr<DOMURL>, public URLDecomposition {
public:
static ExceptionOr<Ref<DOMURL>> create(const String& url, const String& base);
static ExceptionOr<Ref<DOMURL>> create(const String& url, const DOMURL& base);
~DOMURL();
static ExceptionOr<Ref<DOMURL>> create(const String& url);
WEBCORE_EXPORT ~DOMURL();

static RefPtr<DOMURL> parse(const String& url, const String& base);
static bool canParse(const String& url, const String& base);

const URL& href() const { return m_url; }
ExceptionOr<void> setHref(const String&);
void setQuery(const String&);

URLSearchParams& searchParams();

const String& toJSON() const { return m_url.string(); }

// static String createObjectURL(ScriptExecutionContext&, Blob&);
// static void revokeObjectURL(ScriptExecutionContext&, const String&);
static String createObjectURL(ScriptExecutionContext&, Blob&);
static void revokeObjectURL(ScriptExecutionContext&, const String&);

// static String createPublicURL(ScriptExecutionContext&, URLRegistrable&);
static String createPublicURL(ScriptExecutionContext&, URLRegistrable&);

private:
static ExceptionOr<Ref<DOMURL>> create(const String& url, const URL& base);
DOMURL(URL&& completeURL, const URL& baseURL);
DOMURL(URL&& completeURL);

URL fullURL() const final { return m_url; }
void setFullURL(const URL& fullURL) final { setHref(fullURL.string()); }

URL m_baseURL;
URL m_url;
RefPtr<URLSearchParams> m_searchParams;
};
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/URLSearchParams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ String URLSearchParams::toString() const
void URLSearchParams::updateURL()
{
if (m_associatedURL)
m_associatedURL->setQuery(WTF::URLParser::serialize(m_pairs));
m_associatedURL->setSearch(WTF::URLParser::serialize(m_pairs));
}

void URLSearchParams::updateFromAssociatedURL()
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2709,7 +2709,7 @@ pub const JSGlobalObject = extern struct {
got: usize,
) JSC.JSValue {
return JSC.toTypeErrorWithCode(
"NOT_ENOUGH_ARGUMENTS",
@tagName(JSC.Node.ErrorCode.ERR_MISSING_ARGS),
"Not enough arguments to '" ++ name_ ++ "'. Expected {d}, got {d}.",
.{ expected, got },
this,
Expand Down
22 changes: 22 additions & 0 deletions src/bun.js/bindings/webcore/JSDOMOperation.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include "root.h"

#include "BunClientData.h"
#include "JSDOMOperation.h"
#include "BunBuiltinNames.h"

#undef createNotEnoughArgumentsError

namespace WebCore {

JSC::JSObject* createNotEnoughArgumentsErrorBun(JSC::JSGlobalObject* globalObject)
{
JSC::JSObject* error = JSC::createNotEnoughArgumentsError(globalObject);
if (LIKELY(error)) {
auto& vm = globalObject->vm();
const auto& names = WebCore::builtinNames(vm);
error->putDirect(vm, names.codePublicName(), JSC::jsString(vm, WTF::String("ERR_MISSING_ARGS"_s)), 0);
}

return error;
}
}
12 changes: 10 additions & 2 deletions src/bun.js/bindings/webcore/JSDOMOperation.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class IDLOperation {
static JSC::EncodedJSValue call(JSC::JSGlobalObject& lexicalGlobalObject, JSC::CallFrame& callFrame, const char* operationName)
{
auto throwScope = DECLARE_THROW_SCOPE(JSC::getVM(&lexicalGlobalObject));

auto* thisObject = cast(lexicalGlobalObject, callFrame);
if constexpr (shouldThrow != CastedThisErrorBehavior::Assert) {
if (UNLIKELY(!thisObject))
Expand All @@ -58,7 +58,7 @@ class IDLOperation {
ASSERT(thisObject);

ASSERT_GC_OBJECT_INHERITS(thisObject, JSClass::info());

// FIXME: We should refactor the binding generated code to use references for lexicalGlobalObject and thisObject.
RELEASE_AND_RETURN(throwScope, (operation(&lexicalGlobalObject, &callFrame, thisObject)));
}
Expand All @@ -71,4 +71,12 @@ class IDLOperation {
}
};

// Rewrite all usages of JSC::createNotEnoughArgumentsError to use our own version.
// Our version adds the "code" property from Node.js.
JSC::JSObject* createNotEnoughArgumentsErrorBun(JSGlobalObject* globalObject);

#ifndef createNotEnoughArgumentsError
#define createNotEnoughArgumentsError WebCore::createNotEnoughArgumentsErrorBun
#endif

} // namespace WebCore