Bug 1963775 - Expose path/domain/expiry/sameSite properties in CookieStore API, r=edgul,webidl,emilio,valentin,anti-tracking-reviewers,bvandersloot

Differential Revision: https://phabricator.services.mozilla.com/D247421

dom/cookiestore/CookieStore.cpp

@@ -16,6 +16,7 @@
 #include "mozilla/ipc/BackgroundChild.h"
 #include "mozilla/ipc/PBackgroundChild.h"
 #include "mozilla/net/CookieCommons.h"
+#include "mozilla/net/NeckoChannelParams.h"
 #include "mozilla/StorageAccess.h"
 #include "nsICookie.h"
 #include "nsIGlobalObject.h"
@@ -216,16 +217,11 @@ bool ValidateCookieNamePrefix(const nsAString& aName, const nsAString& aValue,
   return true;
 }
 
-void CookieDataToItem(const CookieData& aData, CookieListItem* aItem) {
-  aItem->mName.Construct(aData.name());
-  aItem->mValue.Construct(aData.value());
-}
-
-void CookieDataToList(const nsTArray<CookieData>& aData,
-                      nsTArray<CookieListItem>& aResult) {
-  for (const CookieData& data : aData) {
+void CookieStructToList(const nsTArray<CookieStruct>& aData,
+                        nsTArray<CookieListItem>& aResult) {
+  for (const CookieStruct& data : aData) {
     CookieListItem* item = aResult.AppendElement();
-    CookieDataToItem(data, item);
+    CookieStore::CookieStructToItem(data, item);
   }
 }
 
@@ -783,7 +779,7 @@ already_AddRefed<Promise> CookieStore::GetInternal(
               }
 
               nsTArray<CookieListItem> list;
-              CookieDataToList(aResult.ResolveValue(), list);
+              CookieStructToList(aResult.ResolveValue(), list);
 
               if (!aOnlyTheFirstMatch) {
                 promise->MaybeResolve(list);
@@ -820,4 +816,44 @@ Document* CookieStore::MaybeGetDocument() const {
   return nullptr;
 }
 
+// static
+void CookieStore::CookieStructToItem(const CookieStruct& aData,
+                                     CookieListItem* aItem) {
+  aItem->mName.Construct(aData.name());
+  aItem->mValue.Construct(aData.value());
+  aItem->mPath.Construct(aData.path());
+
+  if (aData.host().IsEmpty() || aData.host()[0] != '.') {
+    aItem->mDomain.Construct(VoidCString());
+  } else {
+    aItem->mDomain.Construct(nsDependentCSubstring(aData.host(), 1));
+  }
+
+  if (!aData.isSession()) {
+    aItem->mExpires.Construct(aData.expiry() * PR_MSEC_PER_SEC);
+  } else {
+    aItem->mExpires.Construct(nullptr);
+  }
+
+  aItem->mSecure.Construct(aData.isSecure());
+
+  CookieSameSite sameSite = CookieSameSite::None;
+  switch (aData.sameSite()) {
+    case nsICookie::SAMESITE_STRICT:
+      sameSite = CookieSameSite::Strict;
+      break;
+
+    case nsICookie::SAMESITE_LAX:
+      sameSite = CookieSameSite::Lax;
+      break;
+
+    default:
+      // FIXME: lax by default?
+      break;
+  }
+
+  aItem->mSameSite.Construct(sameSite);
+  aItem->mPartitioned.Construct(aData.isPartitioned());
+}
+
 }  // namespace mozilla::dom

dom/cookiestore/CookieStore.h

@@ -13,9 +13,12 @@
 
 class nsIGlobalObject;
 
+namespace mozilla::net {
+class CookieStruct;
+}
+
 namespace mozilla::dom {
 
-class CookieData;
 class CookieStoreChild;
 class CookieStoreNotificationWatcherWrapper;
 class CookieStoreNotifier;
@@ -57,6 +60,9 @@ class CookieStore final : public DOMEventTargetHelper {
 
   IMPL_EVENT_HANDLER(change);
 
+  static void CookieStructToItem(const net::CookieStruct& aData,
+                                 CookieListItem* aItem);
+
  private:
   explicit CookieStore(nsIGlobalObject* aGlobal);
   ~CookieStore();

dom/cookiestore/CookieStoreNotifier.cpp

@@ -7,6 +7,7 @@
 #include "CookieStoreNotifier.h"
 #include "CookieStore.h"
 #include "CookieChangeEvent.h"
+#include "mozilla/net/Cookie.h"
 #include "mozilla/net/CookieCommons.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/WorkerPrivate.h"
@@ -140,23 +141,10 @@ CookieStoreNotifier::Observe(nsISupports* aSubject, const char* aTopic,
   }
 
   CookieListItem item;
+  CookieStore::CookieStructToItem(net::Cookie::Cast(cookie)->ToIPC(), &item);
 
-  nsAutoCString name;
-  rv = cookie->GetName(name);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  item.mName.Construct(NS_ConvertUTF8toUTF16(name));
-
-  if (action != nsICookieNotification::COOKIE_DELETED) {
-    nsAutoCString value;
-    rv = cookie->GetValue(value);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-
-    item.mValue.Construct(NS_ConvertUTF8toUTF16(value));
+  if (action == nsICookieNotification::COOKIE_DELETED) {
+    item.mValue.Reset();
   }
 
   bool deletedEvent = action == nsICookieNotification::COOKIE_DELETED;

dom/cookiestore/CookieStoreParent.cpp

@@ -86,7 +86,7 @@ mozilla::ipc::IPCResult CookieStoreParent::RecvGetRequest(
                aPartitionedOriginAttributes, aThirdPartyContext,
                aPartitionForeign, aUsingStorageAccess, aIsOn3PCBExceptionList,
                aMatchName, aName, aPath, aOnlyFirstMatch]() {
-                CopyableTArray<CookieData> results;
+                CopyableTArray<CookieStruct> results;
                 self->GetRequestOnMainThread(
                     uri, aOriginAttributes, aPartitionedOriginAttributes,
                     aThirdPartyContext, aPartitionForeign, aUsingStorageAccess,
@@ -269,7 +269,7 @@ void CookieStoreParent::GetRequestOnMainThread(
     bool aThirdPartyContext, bool aPartitionForeign, bool aUsingStorageAccess,
     bool aIsOn3PCBExceptionList, bool aMatchName, const nsAString& aName,
     const nsACString& aPath, bool aOnlyFirstMatch,
-    nsTArray<CookieData>& aResults) {
+    nsTArray<CookieStruct>& aResults) {
   nsresult rv;
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -303,7 +303,7 @@ void CookieStoreParent::GetRequestOnMainThread(
     attrsList.AppendElement(aPartitionedOriginAttributes.value());
   }
 
-  nsTArray<CookieData> list;
+  nsTArray<CookieStruct> list;
 
   for (const OriginAttributes& attrs : attrsList) {
     nsTArray<RefPtr<Cookie>> cookies;
@@ -332,9 +332,7 @@ void CookieStoreParent::GetRequestOnMainThread(
         continue;
       }
 
-      CookieData* data = list.AppendElement();
-      data->name() = NS_ConvertUTF8toUTF16(cookie->Name());
-      data->value() = NS_ConvertUTF8toUTF16(cookie->Value());
+      list.AppendElement(cookie->ToIPC());
 
       if (aOnlyFirstMatch) {
         break;

dom/cookiestore/CookieStoreParent.h

@@ -20,7 +20,7 @@ class CookieStoreParent final : public PCookieStoreParent {
 
  public:
   using GetRequestPromise =
-      MozPromise<CopyableTArray<CookieData>, nsresult, true>;
+      MozPromise<CopyableTArray<CookieStruct>, nsresult, true>;
   using SetDeleteRequestPromise = MozPromise<bool, nsresult, true>;
   using GetSubscriptionsRequestPromise =
       MozPromise<CopyableTArray<CookieSubscription>, nsresult, true>;
@@ -75,7 +75,7 @@ class CookieStoreParent final : public PCookieStoreParent {
       bool aThirdPartyContext, bool aPartitionForeign, bool aUsingStorageAccess,
       bool aIsOn3PCBExceptionList, bool aMatchName, const nsAString& aName,
       const nsACString& aPath, bool aOnlyFirstMatch,
-      nsTArray<CookieData>& aResults);
+      nsTArray<CookieStruct>& aResults);
 
   // Returns true if a cookie notification has been generated while completing
   // the operation.

dom/cookiestore/CookieStoreSubscriptionService.cpp

@@ -344,8 +344,8 @@ CookieStoreSubscriptionService::Observe(nsISupports* aSubject,
       }
 
       rv = swm->SendCookieChangeEvent(principalInfo.attrs(),
-                                      data.mRegistration.scope(), name, value,
-                                      deleteEvent);
+                                      data.mRegistration.scope(),
+                                      cookie->AsCookie().ToIPC(), deleteEvent);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }

dom/cookiestore/PCookieStore.ipdl

@@ -4,6 +4,7 @@
 
 include protocol PBackground;
 include PBackgroundSharedTypes;
+include NeckoChannelParams;
 
 using mozilla::OriginAttributes from "mozilla/ipc/BackgroundUtils.h";
 using struct mozilla::void_t from "ipc/IPCMessageUtils.h";
@@ -13,15 +14,9 @@ using struct nsID from "nsID.h";
 namespace mozilla {
 namespace dom {
 
-struct CookieData
+union MaybeCookieStruct
 {
-  nsString name;
-  nsString value;
-};
-
-union MaybeCookieData
-{
-  CookieData;
+  CookieStruct;
   void_t;
 };
 
@@ -48,7 +43,7 @@ parent:
                    bool matchName,
                    nsString name,
                    nsCString path,
-                   bool onlyFirstMatch) returns (CookieData[] data);
+                   bool onlyFirstMatch) returns (CookieStruct[] data);
 
   async SetRequest(nullable nsIURI cookieURI,
                    OriginAttributes attrs,

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -1098,8 +1098,7 @@ ServiceWorkerManager::SendPushEvent(const nsACString& aOriginAttributes,
 
 nsresult ServiceWorkerManager::SendCookieChangeEvent(
     const OriginAttributes& aOriginAttributes, const nsACString& aScope,
-    const nsAString& aCookieName, const nsAString& aCookieValue,
-    bool aCookieDeleted) {
+    const net::CookieStruct& aCookie, bool aCookieDeleted) {
   nsCOMPtr<nsIPrincipal> principal;
   MOZ_TRY_VAR(principal, ScopeToPrincipal(aScope, aOriginAttributes));
 
@@ -1117,7 +1116,7 @@ nsresult ServiceWorkerManager::SendCookieChangeEvent(
   }
 
   return serviceWorker->WorkerPrivate()->SendCookieChangeEvent(
-      aCookieName, aCookieValue, aCookieDeleted, registration);
+      aCookie, aCookieDeleted, registration);
 }
 
 nsresult ServiceWorkerManager::SendPushEvent(

dom/serviceworkers/ServiceWorkerManager.h

@@ -43,6 +43,10 @@ namespace ipc {
 class PrincipalInfo;
 }  // namespace ipc
 
+namespace net {
+class CookieStruct;
+}
+
 namespace dom {
 
 class ContentParent;
@@ -261,8 +265,7 @@ class ServiceWorkerManager final : public nsIServiceWorkerManager,
 
   nsresult SendCookieChangeEvent(const OriginAttributes& aOriginAttributes,
                                  const nsACString& aScope,
-                                 const nsAString& aCookieName,
-                                 const nsAString& aCookieValue,
+                                 const net::CookieStruct& aCookie,
                                  bool aCookieDeleted);
 
   nsresult SendPushEvent(const nsACString& aOriginAttributes,

dom/serviceworkers/ServiceWorkerOp.cpp

@@ -12,6 +12,7 @@
 #include "js/Exception.h"  // JS::ExceptionStack, JS::StealPendingExceptionStack
 #include "jsapi.h"
 
+#include "mozilla/dom/CookieStore.h"
 #include "mozilla/dom/PushSubscriptionChangeEvent.h"
 #include "mozilla/dom/PushSubscriptionChangeEventBinding.h"
 #include "nsCOMPtr.h"
@@ -715,8 +716,7 @@ class CookieChangeEventOp final : public ExtendableEventOp {
         mArgs.get_ServiceWorkerCookieChangeEventOpArgs();
 
     CookieListItem item;
-    item.mName.Construct();
-    item.mName.Value() = args.name();
+    CookieStore::CookieStructToItem(args.cookie(), &item);
 
     GlobalObject globalObj(aCx, aWorkerPrivate->GlobalScope()->GetWrapper());
     nsCOMPtr<EventTarget> eventTarget =
@@ -725,14 +725,12 @@ class CookieChangeEventOp final : public ExtendableEventOp {
 
     RefPtr<ExtendableCookieChangeEvent> event;
 
-    if (!args.deleted()) {
-      item.mValue.Construct();
-      item.mValue.Value() = args.value();
-
-      event = ExtendableCookieChangeEvent::CreateForChangedCookie(eventTarget,
+    if (args.deleted()) {
+      item.mValue.Reset();
+      event = ExtendableCookieChangeEvent::CreateForDeletedCookie(eventTarget,
                                                                   item);
     } else {
-      event = ExtendableCookieChangeEvent::CreateForDeletedCookie(eventTarget,
+      event = ExtendableCookieChangeEvent::CreateForChangedCookie(eventTarget,
                                                                   item);
     }
 

dom/serviceworkers/ServiceWorkerOpArgs.ipdlh

@@ -5,6 +5,7 @@
 include ClientIPCTypes;
 include DOMTypes;
 include FetchTypes;
+include NeckoChannelParams;
 
 include "mozilla/dom/ServiceWorkerIPCUtils.h";
 
@@ -32,8 +33,7 @@ struct ServiceWorkerLifeCycleEventOpArgs {
 };
 
 struct ServiceWorkerCookieChangeEventOpArgs {
-  nsString name;
-  nsString value;
+  CookieStruct cookie;
   bool deleted;
 };
 

dom/serviceworkers/ServiceWorkerPrivate.cpp

@@ -937,15 +937,14 @@ nsresult ServiceWorkerPrivate::SendLifeCycleEvent(
 }
 
 nsresult ServiceWorkerPrivate::SendCookieChangeEvent(
-    const nsAString& aCookieName, const nsAString& aCookieValue,
-    bool aCookieDeleted, RefPtr<ServiceWorkerRegistrationInfo> aRegistration) {
+    const net::CookieStruct& aCookie, bool aCookieDeleted,
+    RefPtr<ServiceWorkerRegistrationInfo> aRegistration) {
   AssertIsOnMainThread();
   MOZ_ASSERT(mInfo);
   MOZ_ASSERT(aRegistration);
 
   ServiceWorkerCookieChangeEventOpArgs args;
-  args.name() = aCookieName;
-  args.value() = aCookieValue;
+  args.cookie() = aCookie;
   args.deleted() = aCookieDeleted;
 
   if (mInfo->State() == ServiceWorkerState::Activating) {

dom/serviceworkers/ServiceWorkerPrivate.h

@@ -40,6 +40,10 @@ class Maybe;
 
 class JSObjectHolder;
 
+namespace net {
+class CookieStruct;
+}
+
 namespace dom {
 
 class PostMessageSource;
@@ -106,8 +110,8 @@ class ServiceWorkerPrivate final : public RemoteWorkerObserver {
       const RefPtr<LifeCycleEventCallback>& aCallback);
 
   nsresult SendCookieChangeEvent(
-      const nsAString& aCookieName, const nsAString& aCookieValue,
-      bool aCookieDeleted, RefPtr<ServiceWorkerRegistrationInfo> aRegistration);
+      const net::CookieStruct& aCookie, bool aCookieDeleted,
+      RefPtr<ServiceWorkerRegistrationInfo> aRegistration);
 
   nsresult SendPushEvent(const nsAString& aMessageId,
                          const Maybe<nsTArray<uint8_t>>& aData,

dom/webidl/CookieStore.webidl

@@ -69,28 +69,28 @@ dictionary CookieStoreDeleteOptions {
 };
 
 dictionary CookieListItem {
-  USVString name;
-  USVString value;
+  /* UTF8String semantics match USVString */
 
-  /* Bug 1475599 - We decide to do not implement the entire cookie-store spec.
-   * Instead, we implement only the subset that is compatible with document.cookie */
-  // USVString? domain;
-  // USVString path;
-  // DOMHighResTimeStamp? expires;
-  // boolean secure;
-  // CookieSameSite sameSite;
-  // boolean partitioned;
+  UTF8String name;
+  UTF8String value;
+
+  [Pref="dom.cookieStore.extra.enabled"]
+  UTF8String path;
+
+  [Pref="dom.cookieStore.extra.enabled"]
+  UTF8String? domain;
+
+  [Pref="dom.cookieStore.extra.enabled"]
+  DOMHighResTimeStamp? expires;
+
+  [Pref="dom.cookieStore.extra.enabled"]
+  boolean secure;
+
+  [Pref="dom.cookieStore.extra.enabled"]
+  CookieSameSite sameSite;
+
+  [Pref="dom.cookieStore.extra.enabled"]
+  boolean partitioned;
 };
 
 typedef sequence<CookieListItem> CookieList;
-
-/* Bug 1475599 - We decide to do not implement the entire cookie-store spec.
- * Instead, we implement only the subset that is compatible with document.cookie
-[Exposed=(ServiceWorker,Window),
- SecureContext]
-interface CookieStoreManager {
-  Promise<undefined> subscribe(sequence<CookieStoreGetOptions> subscriptions);
-  Promise<sequence<CookieStoreGetOptions>> getSubscriptions();
-  Promise<undefined> unsubscribe(sequence<CookieStoreGetOptions> subscriptions);
-};
-*/

modules/libpref/init/StaticPrefList.yaml

@@ -2576,6 +2576,12 @@
   value: true
   mirror: always
 
+# Disable extra cookie-store properties
+- name: dom.cookieStore.extra.enabled
+  type: RelaxedAtomicBool
+  value: @IS_NIGHTLY_BUILD@
+  mirror: always
+
 # Is support for CSSPseudoElement enabled?
 - name: dom.css_pseudo_element.enabled
   type: bool

testing/web-platform/meta/cookie-store/__dir__.ini

@@ -1,4 +1,4 @@
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1475599
-prefs: [dom.cookieStore.enabled:true,dom.cookieStore.manager.enabled:true]
+prefs: [dom.cookieStore.enabled:true,dom.cookieStore.manager.enabled:true,dom.cookieStore.extra.enabled:true]
 implementation-status: backlog
 lsan-allowed: [Alloc, Malloc, Then, mozilla::BasePrincipal::CreateContentPrincipal, mozilla::dom::DocGroup::Create, mozilla::dom::ServiceWorkerJobQueue::RunJob, mozilla::dom::ServiceWorkerManager::Unregister, mozilla::dom::ServiceWorkerRegistrationMainThread::Unregister, mozilla::dom::UnregisterCallback::UnregisterCallback, mozilla::net::nsStandardURL::TemplatedMutator, operator]

testing/web-platform/meta/cookie-store/cookieListItem_attributes.https.any.js.ini

@@ -1,70 +0,0 @@
-[cookieListItem_attributes.https.any.serviceworker.html]
-  expected:
-    if os == "win": [OK, TIMEOUT]
-  [CookieListItem - cookieStore.set defaults with positional name and value]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set defaults with name and value in options]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with expires set to a timestamp 10 years in the future]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with expires set to a Date 10 years in the future]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with domain set to the current hostname]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with path set to the current directory]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set adds / to path if it does not end with /]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with sameSite set to strict]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with sameSite set to lax]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with sameSite set to none]
-    expected: FAIL
-
-  [CookieListItem - secure defaults to true]
-    expected: FAIL
-
-
-[cookieListItem_attributes.https.any.html]
-  [CookieListItem - cookieStore.set defaults with positional name and value]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set defaults with name and value in options]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with expires set to a timestamp 10 years in the future]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with expires set to a Date 10 years in the future]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with domain set to the current hostname]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with path set to the current directory]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set adds / to path if it does not end with /]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with sameSite set to strict]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with sameSite set to lax]
-    expected: FAIL
-
-  [CookieListItem - cookieStore.set with sameSite set to none]
-    expected: FAIL
-
-  [CookieListItem - secure defaults to true]
-    expected: FAIL

testing/web-platform/meta/cookie-store/cookieStore_set_arguments.https.any.js.ini

@@ -1,8 +0,0 @@
-[cookieStore_set_arguments.https.any.html]
-  [cookieStore.set adds / to path that does not end with /]
-    expected: FAIL
-
-
-[cookieStore_set_arguments.https.any.serviceworker.html]
-  [cookieStore.set adds / to path that does not end with /]
-    expected: FAIL