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.

Sign up for GitHub

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

Jump to bottom

Fixes #9263 #9266

Merged
merged 2 commits into from
Mar 6, 2024
Merged

Fixes #9263 #9266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
946423e
Fixes #9283
Jarred-Sumner Mar 5, 2024
9c51194
Update URLSearchParams.test.ts
Jarred-Sumner Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions src/bun.js/bindings/URLSearchParams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ String URLSearchParams::get(const String& name) const
return String();
}

bool URLSearchParams::has(const String& name) const
bool URLSearchParams::has(const String& name, const String& value) const
{
for (const auto& pair : m_pairs) {
if (pair.key == name)
if (pair.key == name && (value.isNull() || pair.value == value))
return true;
}
return false;
Expand All @@ -101,6 +101,7 @@ void URLSearchParams::sort()
return WTF::codePointCompareLessThan(a.key, b.key);
});
updateURL();
needsSorting = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does needsSorting exist? it doesn't seem to be used

}

void URLSearchParams::set(const String& name, const String& value)
Expand Down Expand Up @@ -147,13 +148,11 @@ Vector<String> URLSearchParams::getAll(const String& name) const
return values;
}

void URLSearchParams::remove(const String& name)
void URLSearchParams::remove(const String& name, const String& value)
{
if (!m_pairs.removeAllMatching([&](const auto& pair) {
return pair.key == name;
})
&& m_pairs.size() > 0)
return;
m_pairs.removeAllMatching([&](const auto& pair) {
return pair.key == name && (value.isNull() || pair.value == value);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relavent WebKit change: WebKit/WebKit@93a5678

updateURL();
needsSorting = true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/bindings/URLSearchParams.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ class URLSearchParams : public RefCounted<URLSearchParams> {
}

void append(const String& name, const String& value);
void remove(const String& name);
void remove(const String& name, const String& value = {});
String get(const String& name) const;
Vector<String> getAll(const String& name) const;
bool has(const String& name) const;
bool has(const String& name, const String& value = {}) const;
void set(const String& name, const String& value);
String toString() const;
void updateFromAssociatedURL();
Expand Down
21 changes: 19 additions & 2 deletions src/bun.js/bindings/webcore/JSURLSearchParams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include <wtf/Vector.h>
#include <variant>
#include "GCDefferalContext.h"
#include "wtf/StdLibExtras.h"

namespace WebCore {
using namespace JSC;
Expand Down Expand Up @@ -278,7 +279,15 @@ static inline JSC::EncodedJSValue jsURLSearchParamsPrototypeFunction_deleteBody(
EnsureStillAliveScope argument0 = callFrame->uncheckedArgument(0);
auto name = convert<IDLUSVString>(*lexicalGlobalObject, argument0.value());
RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLUndefined>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.remove(WTFMove(name)); })));

String value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do an argsCount() check here before argument(1) since the arg count may be 1 and not 2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argument() does the check internally

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's right, I was looking at the wrong implementation. I thought I remembered something about uncheckedArgument: https://github.com/WebKit/WebKit/blob/de89f2977a314e5ddbe49384e35b953061d17f29/Source/JavaScriptCore/interpreter/CallFrame.h#L275-L285

EnsureStillAliveScope argument1 = callFrame->argument(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very handy!
https://github.com/WebKit/WebKit/blob/f8b8f96af30ce6909fc2f7ece6c5575072463b7f/Source/JavaScriptCore/runtime/JSCJSValue.h#L720-L723

if (!argument1.value().isUndefined()) {
value = convert<IDLUSVString>(*lexicalGlobalObject, argument1.value());
RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
}

RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLUndefined>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.remove(WTFMove(name), WTFMove(value)); })));
}

JSC_DEFINE_HOST_FUNCTION(jsURLSearchParamsPrototypeFunction_delete, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
Expand Down Expand Up @@ -338,7 +347,15 @@ static inline JSC::EncodedJSValue jsURLSearchParamsPrototypeFunction_hasBody(JSC
EnsureStillAliveScope argument0 = callFrame->uncheckedArgument(0);
auto name = convert<IDLUSVString>(*lexicalGlobalObject, argument0.value());
RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLBoolean>(*lexicalGlobalObject, throwScope, impl.has(WTFMove(name)))));

String value;
EnsureStillAliveScope argument1 = callFrame->argument(1);
if (!argument1.value().isUndefined()) {
value = convert<IDLUSVString>(*lexicalGlobalObject, argument1.value());
RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
}

RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLBoolean>(*lexicalGlobalObject, throwScope, impl.has(WTFMove(name), WTFMove(value)))));
}

JSC_DEFINE_HOST_FUNCTION(jsURLSearchParamsPrototypeFunction_has, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
Expand Down
16 changes: 16 additions & 0 deletions test/js/web/html/URLSearchParams.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,19 @@ describe("URLSearchParams", () => {
});
});
});

it(".delete second argument", () => {
const params = new URLSearchParams("a=1&a=2&b=3");
params.delete("a", 1);
params.delete("b", undefined);
expect(params + "").toBe("a=2");
});

it(".has second argument", () => {
const params = new URLSearchParams("a=1&a=2&b=3");
expect(params.has("a", 1)).toBe(true);
expect(params.has("a", 2)).toBe(true);
expect(params.has("a", 3)).toBe(false);
expect(params.has("b", 3)).toBe(true);
expect(params.has("b", 4)).toBe(false);
});
Loading