From b8f970c1c710c7e43cff6770fa845b96445cdaf8 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 16 Mar 2023 13:48:14 +0100 Subject: [PATCH] fix: properly bubble up cookie creation failure message (#37586) --- shell/browser/api/electron_api_cookies.cc | 15 ++++++++++----- spec/api-net-spec.ts | 10 ++++++++++ spec/api-session-spec.ts | 4 ++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/shell/browser/api/electron_api_cookies.cc b/shell/browser/api/electron_api_cookies.cc index 8b2edc8fa5e7..41115a0db7b8 100644 --- a/shell/browser/api/electron_api_cookies.cc +++ b/shell/browser/api/electron_api_cookies.cc @@ -180,7 +180,7 @@ std::string InclusionStatusToString(net::CookieInclusionStatus status) { return "Failed to parse cookie"; if (status.HasExclusionReason( net::CookieInclusionStatus::EXCLUDE_INVALID_DOMAIN)) - return "Failed to get cookie domain"; + return "Failed to set cookie with an invalid domain attribute"; if (status.HasExclusionReason( net::CookieInclusionStatus::EXCLUDE_INVALID_PREFIX)) return "Failed because the cookie violated prefix rules."; @@ -318,19 +318,24 @@ v8::Local Cookies::Set(v8::Isolate* isolate, return handle; } + net::CookieInclusionStatus status; auto canonical_cookie = net::CanonicalCookie::CreateSanitizedCookie( url, name ? *name : "", value ? *value : "", domain ? *domain : "", path ? *path : "", ParseTimeProperty(details.FindDouble("creationDate")), ParseTimeProperty(details.FindDouble("expirationDate")), ParseTimeProperty(details.FindDouble("lastAccessDate")), secure, http_only, same_site, net::COOKIE_PRIORITY_DEFAULT, same_party, - absl::nullopt); + absl::nullopt, &status); + if (!canonical_cookie || !canonical_cookie->IsCanonical()) { - promise.RejectWithErrorMessage( - InclusionStatusToString(net::CookieInclusionStatus( - net::CookieInclusionStatus::EXCLUDE_FAILURE_TO_STORE))); + promise.RejectWithErrorMessage(InclusionStatusToString( + !status.IsInclude() + ? status + : net::CookieInclusionStatus( + net::CookieInclusionStatus::EXCLUDE_FAILURE_TO_STORE))); return handle; } + net::CookieOptions options; if (http_only) { options.set_include_httponly(); diff --git a/spec/api-net-spec.ts b/spec/api-net-spec.ts index 61c259e7e80c..c4cae25689be 100644 --- a/spec/api-net-spec.ts +++ b/spec/api-net-spec.ts @@ -903,6 +903,16 @@ describe('net module', () => { expect(cookies[0].name).to.equal('cookie2'); }); + it('throws when an invalid domain is passed', async () => { + const sess = session.fromPartition(`cookie-tests-${Math.random()}`); + + await expect(sess.cookies.set({ + url: 'https://electronjs.org', + domain: 'wssss.iamabaddomain.fun', + name: 'cookie1' + })).to.eventually.be.rejectedWith(/Failed to set cookie with an invalid domain attribute/); + }); + it('should be able correctly filter out cookies that are session', async () => { const sess = session.fromPartition(`cookie-tests-${Math.random()}`); diff --git a/spec/api-session-spec.ts b/spec/api-session-spec.ts index 947363e61569..f1c587d4d154 100644 --- a/spec/api-session-spec.ts +++ b/spec/api-session-spec.ts @@ -128,7 +128,7 @@ describe('session module', () => { await expect( cookies.set({ url: '', name, value }) - ).to.eventually.be.rejectedWith('Failed to get cookie domain'); + ).to.eventually.be.rejectedWith('Failed to set cookie with an invalid domain attribute'); }); it('yields an error when setting a cookie with an invalid URL', async () => { @@ -138,7 +138,7 @@ describe('session module', () => { await expect( cookies.set({ url: 'asdf', name, value }) - ).to.eventually.be.rejectedWith('Failed to get cookie domain'); + ).to.eventually.be.rejectedWith('Failed to set cookie with an invalid domain attribute'); }); it('should overwrite previous cookies', async () => {