Skip to content

Commit

Permalink
chore: remove Chromium Windows proxy hacks (#31724)
Browse files Browse the repository at this point in the history
Fixes #17252
  • Loading branch information
mxschmitt authored Aug 23, 2024
1 parent 9a5b72d commit 1b220c5
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 89 deletions.
6 changes: 0 additions & 6 deletions docs/src/api/params.md
Original file line number Diff line number Diff line change
Expand Up @@ -769,12 +769,6 @@ Actual picture of each page will be scaled down if necessary to fit the specifie

Network proxy settings to use with this context. Defaults to none.

:::note
For Chromium on Windows the browser needs to be launched with the global proxy for this option to work. If all
contexts override the proxy, global proxy will be never used and can be any string, for example
`launch({ proxy: { server: 'http://per-context' } })`.
:::

## context-option-strict
- `strictSelectors` <[boolean]>

Expand Down
48 changes: 18 additions & 30 deletions docs/src/network.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ const browser = await chromium.launch({
Browser browser = chromium.launch(new BrowserType.LaunchOptions()
.setProxy(new Proxy("http://myproxy.com:3128")
.setUsername('usr')
.setPassword('pwd'));
.setPassword('pwd')));
```

```python async
Expand Down Expand Up @@ -179,60 +179,48 @@ await using var browser = await BrowserType.LaunchAsync(new()
});
```

When specifying proxy for each context individually, **Chromium on Windows** needs a hint that proxy will be set. This is done via passing a non-empty proxy server to the browser itself. Here is an example of a context-specific proxy:
Its also possible to specify it per context:

```js tab=js-test title="playwright.config.ts"
import { defineConfig } from '@playwright/test';
export default defineConfig({
use: {
launchOptions: {
// Browser proxy option is required for Chromium on Windows.
proxy: { server: 'per-context' }
},
```js tab=js-test title="example.spec.ts"
import { test, expect } from '@playwright/test';

test('should use custom proxy on a new context', async ({ browser }) => {
const context = await browser.newContext({
proxy: {
server: 'http://myproxy.com:3128',
}
}
});
const page = await context.newPage();

await context.close();
});
```

```js tab=js-library
const browser = await chromium.launch({
// Browser proxy option is required for Chromium on Windows.
proxy: { server: 'per-context' }
});
const browser = await chromium.launch();
const context = await browser.newContext({
proxy: { server: 'http://myproxy.com:3128' }
});
```

```java
Browser browser = chromium.launch(new BrowserType.LaunchOptions()
// Browser proxy option is required for Chromium on Windows.
.setProxy(new Proxy("per-context"));
BrowserContext context = chromium.launch(new Browser.NewContextOptions()
.setProxy(new Proxy("http://myproxy.com:3128"));
Browser browser = chromium.launch();
BrowserContext context = browser.newContext(new Browser.NewContextOptions()
.setProxy(new Proxy("http://myproxy.com:3128")));
```

```python async
# Browser proxy option is required for Chromium on Windows.
browser = await chromium.launch(proxy={"server": "per-context"})
browser = await chromium.launch()
context = await browser.new_context(proxy={"server": "http://myproxy.com:3128"})
```

```python sync
# Browser proxy option is required for Chromium on Windows.
browser = chromium.launch(proxy={"server": "per-context"})
browser = chromium.launch()
context = browser.new_context(proxy={"server": "http://myproxy.com:3128"})
```

```csharp
var proxy = new Proxy { Server = "per-context" };
await using var browser = await BrowserType.LaunchAsync(new()
{
// Browser proxy option is required for Chromium on Windows.
Proxy = proxy
});
await using var browser = await BrowserType.LaunchAsync();
await using var context = await browser.NewContextAsync(new()
{
Proxy = new Proxy { Server = "http://myproxy.com:3128" },
Expand Down
6 changes: 1 addition & 5 deletions packages/playwright-core/src/server/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import * as os from 'os';
import { TimeoutSettings } from '../common/timeoutSettings';
import { createGuid, debugMode } from '../utils';
import { mkdirIfNeeded } from '../utils/fileUtils';
Expand Down Expand Up @@ -700,11 +699,8 @@ export function validateBrowserContextOptions(options: channels.BrowserNewContex
options.recordVideo.size!.width &= ~1;
options.recordVideo.size!.height &= ~1;
}
if (options.proxy) {
if (!browserOptions.proxy && browserOptions.isChromium && os.platform() === 'win32')
throw new Error(`Browser needs to be launched with the global proxy. If all contexts override the proxy, global proxy will be never used and can be any string, for example "launch({ proxy: { server: 'http://per-context' } })"`);
if (options.proxy)
options.proxy = normalizeProxySettings(options.proxy);
}
verifyGeolocation(options.geolocation);
}

Expand Down
6 changes: 0 additions & 6 deletions packages/playwright-core/src/server/chromium/chromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@ export class Chromium extends BrowserType {
artifactsDir,
downloadsPath: options.downloadsPath || artifactsDir,
tracesDir: options.tracesDir || artifactsDir,
// On Windows context level proxies only work, if there isn't a global proxy
// set. This is currently a bug in the CR/Windows networking stack. By
// passing an arbitrary value we disable the check in PW land which warns
// users in normal (launch/launchServer) mode since otherwise connectOverCDP
// does not work at all with proxies on Windows.
proxy: { server: 'per-context' },
originalLaunchOptions: {},
};
validateBrowserContextOptions(persistent, browserOptions);
Expand Down
8 changes: 0 additions & 8 deletions packages/playwright-core/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9372,10 +9372,6 @@ export interface Browser {

/**
* Network proxy settings to use with this context. Defaults to none.
*
* **NOTE** For Chromium on Windows the browser needs to be launched with the global proxy for this option to work. If
* all contexts override the proxy, global proxy will be never used and can be any string, for example `launch({
* proxy: { server: 'http://per-context' } })`.
*/
proxy?: {
/**
Expand Down Expand Up @@ -20847,10 +20843,6 @@ export interface BrowserContextOptions {

/**
* Network proxy settings to use with this context. Defaults to none.
*
* **NOTE** For Chromium on Windows the browser needs to be launched with the global proxy for this option to work. If
* all contexts override the proxy, global proxy will be never used and can be any string, for example `launch({
* proxy: { server: 'http://per-context' } })`.
*/
proxy?: {
/**
Expand Down
25 changes: 0 additions & 25 deletions tests/library/browsercontext-proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,12 @@ import { browserTest as it, expect } from '../config/browserTest';

it.skip(({ mode }) => mode.startsWith('service'));

it.use({
launchOptions: async ({ launchOptions }, use) => {
await use({
...launchOptions,
proxy: { server: 'per-context' }
});
}
});


it.beforeEach(({ server }) => {
server.setRoute('/target.html', async (req, res) => {
res.end('<html><title>Served by the proxy</title></html>');
});
});

it('should throw for missing global proxy on Chromium Windows', async ({ browserName, platform, browserType, server }) => {
it.skip(browserName !== 'chromium' || platform !== 'win32');

let browser;
try {
browser = await browserType.launch({
proxy: undefined,
});
const error = await browser.newContext({ proxy: { server: `localhost:${server.PORT}` } }).catch(e => e);
expect(error.toString()).toContain('Browser needs to be launched with the global proxy');
} finally {
await browser.close();
}
});

it('should work when passing the proxy only on the context level', async ({ browserName, platform, browserType, server, proxyServer }) => {
// Currently an upstream bug in the network stack of Chromium which leads that
// the wrong proxy gets used in the BrowserContext.
Expand Down
9 changes: 0 additions & 9 deletions tests/library/fetch-proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,6 @@

import { contextTest as it, expect } from '../config/browserTest';

it.use({
launchOptions: async ({ launchOptions }, use) => {
await use({
...launchOptions,
proxy: { server: 'per-context' }
});
}
});

it.skip(({ mode }) => mode !== 'default');

it('context request should pick up proxy credentials', async ({ browserType, server, proxyServer }) => {
Expand Down

0 comments on commit 1b220c5

Please sign in to comment.