Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Feature]: Harden playwright against core object manipulation #34443

Open
hakito opened this issue Jan 23, 2025 · 3 comments
Open

[Feature]: Harden playwright against core object manipulation #34443

hakito opened this issue Jan 23, 2025 · 3 comments

Comments

@hakito
Copy link

hakito commented Jan 23, 2025

🚀 Feature Request

See #34328

Sorry, I could not reply earlier, as I was very busy the last week.

Are you using some third-party library that overwrites standard Map or is it you in house solution? If you can point to a public website with the problem or share actual example, that'd be helpful. We've addressed a few similar issues in the past with other web APIs that were broken by some popular libraries.

The tested target application is on SAP systems. We are not part of SAP. Even if SAP would provide a patch for it, it takes a lot of time until the fix is deployed to all our customers.

We saw the map replacement in 2 files on the server:

  • sap(====)/bc/bsp/sap/ic_base/scripts/common/ic_base_utils_map.js
  • sap(====)/bc/bsp/sap/crmcmp_ic_frame/crmcmp_ic_frame_utils_map.js

I am probably not allowed to share the actual sources, but i don't think it is some part of any public library.

We need to understand the scope of the problem here and which Playwright APIs you call, as many parts of the injected script already run in an isolated world and are immune to these kind issues.

We call for example BrowserContext.AddInitScript and Page.EvaluateAsync and need to work with some Map objects within the called scripts. The Init script might still work, as it is executed early enough. But EvaluateAsync might fail when the sap scripts have been evaluated before.

Example

No response

Motivation

We need more information to act on this report. As long as we can't repro it, it is unlikely with can make progress with it. Allow me to close it, but please file a new one and link to this issue when you get back to it!
@pavelfeldman
Copy link
Member

We call for example BrowserContext.AddInitScript and Page.EvaluateAsync and need to work with some Map objects within the called scripts. The Init script might still work, as it is executed early enough. But EvaluateAsync might fail when the sap scripts have been evaluated before.

So your app (I understand that it is not your fault) breaks Map type and you can't use it in your evaluate. Can you save it to window._Map in init script?

@hakito
Copy link
Author

hakito commented Jan 27, 2025

When exposing a callback function, the exception happens within the playwright sources:

playwright/packages/playwright-core/src/server/page.ts

Line 923 in 640e6a8

callbacks = new Map(); 

VM799:24 Uncaught (in promise) TypeError: callbacks.set is not a function
    at <anonymous>:24:64
    at new Promise (<anonymous>)
    at globalThis.<computed> (<anonymous>:24:21)
    at <anonymous>:7607:9
    at <anonymous>:1506:25
    at Map.forEach (<anonymous>)
    at <anonymous>:1492:35
    at Map.forEach (<anonymous>)
    at #forwardEvent (<anonymous>:1481:13)
(anonymous) @ VM799:24
globalThis.<computed> @ VM799:24
(anonymous) @ VM804:7607
(anonymous) @ VM804:1506
(anonymous) @ VM804:1492
#forwardEvent @ VM804:1481
Promise.then
(anonymous) @ VM804:7607
(anonymous) @ VM804:1506
(anonymous) @ VM804:1492
#forwardEvent @ VM804:1481
VM804:990 build-css-selector: 0.953125 ms
VM804:990 build-css-selector: 0.47314453125 ms

So when use the html below and expose a function myExposedFunction() you should be able to reproduce the exception.

<html>
    <body>
        <script language="javascript">
            function Map( )
            {
                this.valueArray = new Array( );
                this.keyArray = new Array( );
                this.keyCount = 0;
            }
            myExposedFunction();
        </script>
    </body>
</html>

@dgozman
Copy link
Contributor

dgozman commented Feb 3, 2025

@hakito I tried to see what we can do without using Map, but unfortunately it is used in too many places. I don't think we'll be able to support such a scenario. I'd recommend providing your own Map implementation through evaluate() or addInitScript(). For example,

// Save it before the page can tamper with it.
await page.addInitScript(() => { window.__Map = window.Map });

// Restore it before accessing your exposed function.
await page.evaluate(() => { window.Map = window.__Map; });

Let me know whether that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@hakito @pavelfeldman @dgozman