400 Bad Request when using Windows Push Notification Services (WNS) endpoint

[meewash-p]

Hi,

I've encountered an issue when sending a push to the endpoint from WNS (MS Edge browser). Their service is responding with 400 Bad Request with no body.

After quick debugging, there was an error reason in response headers:

{
"Content-Length": "0",
"mise-correlation-id": "927f3b90-c2cf-4649-97a9-94330058c16d",
"X-WNS-ERROR-DESCRIPTION": "Ttl value conflicts with X-WNS-Cache-Policy.",
"X-WNS-STATUS": "dropped",
"X-WNS-NOTIFICATIONSTATUS": "dropped",
"Date": "Mon, 04 Mar 2024 12:24:23 GMT"
}

Turns out, WNS requires X-WNS-Cache-Policy request header set to either cache or no-cache depending on a ttl value.

Adding the header helps. Also, I'd suggest including headers in the exception message. :)

If you have that issue with WNS add x-wns-cache-policy header (if using ttl than x-wns-cache-policy: cache) in your code, e.g.:

webpush(
    subscription_info=push_subscription,
    data=payload_data,
    vapid_private_key=vapid_private_key,
    vapid_claims=vapid_claims,
    headers={'x-wns-cache-policy': 'no-cache'}
)

I'd do sth like this:

From a5442bd95eeb82835d3e7ecf62f11616d1cca8bd Mon Sep 17 00:00:00 2001
From: mp <mp@meewash.com>
Date: Mon, 4 Mar 2024 13:40:55 +0100
Subject: [PATCH] Add headers.update with X-WNS-Cache-Policy and tests

---diff
 pywebpush/__init__.py           | 12 ++++++++++--
 pywebpush/tests/test_webpush.py | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

--- a/pywebpush/__init__.py
+++ b/pywebpush/__init__.py
@@ -326,6 +326,14 @@ class WebPusher:
             headers.update({
                 'content-encoding': content_encoding,
             })
+            if ttl == 0:
+                headers.update({
+                    'x-wns-cache-policy': 'no-cache',
+                })
+            else:
+                headers.update({
+                    'x-wns-cache-policy': 'cache',
+                })
         if gcm_key:
             # guess if it is a legacy GCM project key or actual FCM key
             # gcm keys are all about 40 chars (use 100 for confidence),
@@ -494,7 +502,7 @@ def webpush(subscription_info,
         timeout=timeout,
     )
     if not curl and response.status_code > 202:
-        raise WebPushException("Push failed: {} {}\nResponse body:{}".format(
-            response.status_code, response.reason, response.text),
+        raise WebPushException("Push failed: {} {}\nResponse body:{}\nResponse headers: {}".format(
+            response.status_code, response.reason, response.text, str(response.headers)),
             response=response)
     return response
diff --git a/pywebpush/tests/test_webpush.py b/pywebpush/tests/test_webpush.py
index 93dc51a..938d9d1 100644
--- a/pywebpush/tests/test_webpush.py
+++ b/pywebpush/tests/test_webpush.py
@@ -142,6 +142,23 @@ class WebpushTestCase(unittest.TestCase):
         ckey = pheaders.get('crypto-key')
         assert 'pre-existing' in ckey
         assert pheaders.get('content-encoding') == 'aes128gcm'
+        assert pheaders.get('x-wns-cache-policy') == 'no-cache'
+
+    @patch("requests.post")
+    def test_send_ttl(self, mock_post):
+        subscription_info = self._gen_subscription_info()
+        headers = {"Crypto-Key": "pre-existing",
+                   "Authentication": "bearer vapid"}
+        data = "Mary had a little lamb"
+        WebPusher(subscription_info).send(data, headers, ttl=10)
+        assert subscription_info.get('endpoint') == mock_post.call_args[0][0]
+        pheaders = mock_post.call_args[1].get('headers')
+        assert pheaders.get('ttl') == '10'
+        assert pheaders.get('AUTHENTICATION') == headers.get('Authentication')
+        ckey = pheaders.get('crypto-key')
+        assert 'pre-existing' in ckey
+        assert pheaders.get('content-encoding') == 'aes128gcm'
+        assert pheaders.get('x-wns-cache-policy') == 'cache'
 
     @patch("requests.post")
     def test_send_vapid(self, mock_post):
@@ -173,6 +190,7 @@ class WebpushTestCase(unittest.TestCase):
         ckey = pheaders.get('crypto-key')
         assert 'dh=' in ckey
         assert pheaders.get('content-encoding') == 'aesgcm'
+        assert pheaders.get('x-wns-cache-policy') == 'no-cache'
         assert pheaders.get('test-header') == 'test-value'
 
     @patch.object(WebPusher, "send")
@@ -288,6 +306,7 @@ class WebpushTestCase(unittest.TestCase):
         pheaders = mock_post.call_args[1].get('headers')
         assert pheaders.get('ttl') == '0'
         assert pheaders.get('content-encoding') == 'aes128gcm'
+        assert pheaders.get('x-wns-cache-policy') == 'no-cache'
 
     @patch("pywebpush.open")
     def test_as_curl(self, opener):
@@ -334,6 +353,7 @@ class WebpushTestCase(unittest.TestCase):
         assert pdata["registration_ids"][0] == "regid123"
         assert pheaders.get("authorization") == "key=gcm_key_value"
         assert pheaders.get("content-type") == "application/json"
+        assert pheaders.get('x-wns-cache-policy') == 'no-cache'
 
     @patch("requests.post")
     def test_timeout(self, mock_post):
@@ -360,6 +380,7 @@ class WebpushTestCase(unittest.TestCase):
         ckey = pheaders.get('crypto-key')
         assert 'pre-existing' in ckey
         assert pheaders.get('content-encoding') == 'aes128gcm'
+        assert pheaders.get('x-wns-cache-policy') == 'no-cache'
 
 
 class WebpushExceptionTestCase(unittest.TestCase):
-- 
2.34.1

[jrconlin]

Ah, looks like there are several headers that they want.
I'm half tempted to put these under an option flag just in case they might mess up Apple or Google for some stupid reason.

[jrconlin]

(I'll also note that pywebpush exposes a parameter for headers for things like this. It does mean that the calling code needs to know the TTL is 0, so it applies the correct caching header, but it's a reasonable work-around)

[martinthomas]

I have been trying to make this work (web push to Edge from a Flask app using this library) and was about to give up on Edge until I successfully sent a web push to Edge using the Node web-push library from the command line. After seeing the notes in the issue, I took a look in the Node library code for wns headers and found none. Just FYI

[jrconlin]

Ah, thanks for the bump on this.

I was waiting for a review. Apparently from Godot.
I'm going to update it to include an error to check for the X-WNS-Type header if --wns is specified as a flag.

[martinthomas]

My code now works after adding only a ttl value after seeing the npm module has a large default value.
Looking at the MS doc mentioned earlier many of the headers are optional.

[jrconlin]

Right, X-WNS-Type is marked as a required field (as either tile, toast, badge or raw, with the Content-Type set to text/xml except if raw where it's set to application/octet-stream.)

I can do some simple checks to make sure folk are sending the right format, but this is all fairly non-standard and a lot of extra work. I'm also just tempted to add some content to the README talking about how non-standard this is and what extra stuff you need to do to send notification messages to Windows computers.

[jrconlin]

Ok, so here's the deal.

I'm not going to add any special handling for WNS. In fact, I'm going to take this opportunity to drop the special handling we were doing for GCM/FCM, since that's going away in less than 2 months and has been obsolete for nearly a year now.

If you want to use this library with WNS, you have to add the headers yourself, since they're subject to change anyway. I've added a note in the README for what you'll need to do.
If you were using this to talk to GCM/FCM directly and not using Web Push, this is your wake-up call that you really, really need to stop doing that.

[martinthomas]

Sounds right to me. Anything beyond adding a note for WNS - add TTL and life is good other than that add headers, see this doc - seems unnecessary.

…

On Fri, Apr 19, 2024 at 12:35 PM JR Conlin ***@***.***> wrote: Ok, so here's the deal. I'm not going to add any special handling for WNS. In fact, I'm going to take this opportunity to drop the special handling we were doing for GCM/FCM, since that's going away in less than 2 months and has been obsolete for nearly a year now. If you want to use this library with WNS, you have to add the headers yourself, since they're subject to change anyway. I've added a note in the README for what you'll need to do. If you were using this to talk to GCM/FCM directly and not using Web Push, this is your wake-up call that you really, really need to stop doing that. — Reply to this email directly, view it on GitHub <#162 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACM4WELXNTKXXGU4PNSHJLY6FIUTAVCNFSM6AAAAABEFE2GQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXGAYDKNRYGQ> . You are receiving this because you commented.Message ID: ***@***.***>