microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
66.85k stars 3.66k forks source link

[BUG] extra http headers are not sent with Websocket requests #28948

Open vEpiphyte opened 10 months ago

vEpiphyte commented 10 months ago

System info

Scenario: I am testing an application that is to be deployed behind an AWS Application Load Balancer. The ABLB is configured to inject additional headers into all HTTP traffic that it services. I am attempting to use playwright to mimic that behavior by setting those additional headers on the Playwright. This works for regular HTTP requests, but does not inject the additional HTTP headers when the application makes a WebSocket connection to the server.

Source code

I've modified the playwright tests to demonstrate the lack of headers being sent:

$ git diff tests/*
diff --git a/tests/async/test_websocket.py b/tests/async/test_websocket.py
index eb90f95..f2f670b 100644
--- a/tests/async/test_websocket.py
+++ b/tests/async/test_websocket.py
@@ -24,6 +24,7 @@ from tests.server import Server

 async def test_should_work(page: Page, ws_server: WebSocketServerServer) -> None:
+    await page.set_extra_http_headers({'HeHe-Sock': 'HaHa'})
     value = await page.evaluate(
         """port => {
         let cb;
diff --git a/tests/server.py b/tests/server.py
index 06e3446..c792d2a 100644
--- a/tests/server.py
+++ b/tests/server.py
@@ -273,6 +273,7 @@ class WebSocketProtocol(WebSocketServerProtocol):
         pass

     def onOpen(self) -> None:
+        print(self.http_request_data)
         self.sendMessage(b"incoming")

     def onMessage(self, payload: bytes, isBinary: bool) -> None:

Produces the following output:

$ python -m pytest tests/async/test_websocket.py  -k work\[chromium
=========================================== test session starts =============================================================
platform linux -- Python 3.11.2, pytest-7.4.2, pluggy-1.0.0 -- /some/python3.12.2/directory/bin/python
cachedir: .pytest_cache
rootdir: /some/path/playwright-python
configfile: pyproject.toml
plugins: repeat-0.9.1, asyncio-0.21.1, flaky-3.7.0, xdist-3.3.1, rerunfailures-12.0, timeout-2.1.0, cov-4.1.0
asyncio: mode=Mode.AUTO
collected 18 items / 17 deselected / 1 selected                                                                                                                                                                                                                                                                                                                                                                                    

tests/async/test_websocket.py::test_should_work[chromium] b'GET /ws HTTP/1.1\r\nHost: localhost:50479\r\nConnection: Upgrade\r\nPragma: no-cache\r\nCache-Control: no-cache\r\nUser-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/120.0.6099.28 Safari/537.36\r\nUpgrade: websocket\r\nOrigin: null\r\nSec-WebSocket-Version: 13\r\nAccept-Encoding: gzip, deflate, br\r\nSec-WebSocket-Key: U77GHD/H1pmVcmytWUjsXg==\r\nSec-WebSocket-Extensions: permessage-deflate; client_max_window_bits\r\n\r\n'
PASSED

=========================================== 1 passed, 17 deselected, 1 warning in 1.02s =====================================

Expected

I expect the HTTP 101 requests ( made to upgrade a connection to a websocket ) to include the headers added to the page.

I have tried setting additional headers with the following APIs:

https://playwright.dev/python/docs/api/class-browser#browser-new-context https://playwright.dev/python/docs/api/class-browsercontext#browser-context-set-extra-http-headers https://playwright.dev/python/docs/api/class-page#page-set-extra-http-headers

These API docs imply that all connections will have these additional headers added to them.

Actual

In all my test cases, the extra headers are present on non-websocket requests. The HTTP 101 requests made to upgrade a connection do not have the additional headers available on them.

mxschmitt commented 10 months ago

I can repro with WebKit and Chromium. Passes in Firefox:

diff --git a/tests/page/page-set-extra-http-headers.spec.ts b/tests/page/page-set-extra-http-headers.spec.ts
index 284621edd..0a6a6c74f 100644
--- a/tests/page/page-set-extra-http-headers.spec.ts
+++ b/tests/page/page-set-extra-http-headers.spec.ts
@@ -29,6 +29,22 @@ it('should work @smoke', async ({ page, server }) => {
   expect(request.headers['baz']).toBe(undefined);
 });

+it('should set it on WebSocket connections', async ({ page, server }) => {
+  await page.setExtraHTTPHeaders({
+    foo: 'bar',
+  });
+  const webSocketRequestPromise = server.waitForWebSocketConnectionRequest();
+  await page.evaluate(port => {
+    let cb;
+    const result = new Promise(f => cb = f);
+    const ws = new WebSocket('ws://localhost:' + port + '/ws');
+    ws.addEventListener('open', () => { ws.close(); cb(); });
+    return result;
+  }, server.PORT);
+  const webSocketRequest = await webSocketRequestPromise;
+  expect(webSocketRequest.headers['foo']).toBe('bar');
+});
+
 it('should work with redirects', async ({ page, server }) => {
   server.setRedirect('/foo.html', '/empty.html');
   await page.setExtraHTTPHeaders({
pavelfeldman commented 6 months ago

Why was this issue closed?

Thank you for your contribution to our project. This issue has been closed due to its limited upvotes and recent activity, and insufficient feedback for us to effectively act upon. Our priority is to focus on bugs that reflect higher user engagement and have actionable feedback, to ensure our bug database stays manageable.

Should you feel this closure was in error, please create a new issue and reference this one. We're open to revisiting it given increased support or additional clarity. Your understanding and cooperation are greatly appreciated.

pavelfeldman commented 2 months ago

Why was this issue closed?

Thank you for your contribution to our project. This issue has been closed due to its limited upvotes and recent activity, and insufficient feedback for us to effectively act upon. Our priority is to focus on bugs that reflect higher user engagement and have actionable feedback, to ensure our bug database stays manageable.

Should you feel this closure was in error, please create a new issue and reference this one. We're open to revisiting it given increased support or additional clarity. Your understanding and cooperation are greatly appreciated.

aletzal commented 2 months ago

Hi! It's important bug, blocks writing tests for projects with ws, any ideas how to fix it?

vEpiphyte commented 2 months ago

@aletzal I've refiled this per @pavelfeldman 's note above as https://github.com/microsoft/playwright/issues/32247