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.

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

[🚀 Feature]: Add an option to remove "remote-allow-origins" from ChromeOptions #11949

Closed
gtach2o opened this issue Apr 25, 2023 · 12 comments
Closed

Comments

@gtach2o
Copy link

gtach2o commented Apr 25, 2023

Feature and motivation

After this happend

    // Allowing any origin "*" might sound risky but an attacker would need to know
    // the port used to start DevTools to establish a connection. Given these sessions
    // are relatively short-lived, the risk is reduced. Also, this will be removed when
    // we only support Java 11 and above.
    addArguments("--remote-allow-origins=*");

our tests stoped working. We can't update to Selenium 4.8.2 or higher
We awlays use latest chrome driver, current is 112.0.5615.49
JDK native http client
Java 20
and everything was working fine until 4.8.2 released
{args=[--remote-allow-origins=*, is breaking our automation.
How to remove it from default chrome options? We don't need it.

Usage example

removeArguments("--remote-allow-origins=*");

@github-actions
Copy link

@gtach2o, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

@diemol
Copy link
Member

diemol commented Apr 25, 2023

What error do you get?

@gtach2o
Copy link
Author

gtach2o commented Apr 25, 2023

What error do you get?

Well, there is no exact error I can share. Adding --remote-allow-origins=* in some way affects the iteraction with browser.
Next step after click fails, seems like synchronization doesn't work properly, Selenium doesn't wait properly for page loading.
When we swtich back to 4.8.1 everything works with no issue.

@gtach2o
Copy link
Author

gtach2o commented Apr 25, 2023

Maybe adding --remote-allow-origins=* does help when netty is still used.
But have you tested it with org.seleniumhq.selenium:selenium-http-jdk-client:4.8.2 and System.setProperty("webdriver.http.factory", "jdk-http-client"); ?

@titusfortner
Copy link
Member

@diemol are we setting it based on the condition that system property is not set?

@gtach2o
Copy link
Author

gtach2o commented Apr 25, 2023

Actually our tests fail with

org.openqa.selenium.InvalidSelectorException: invalid selector: Unable to locate an element with the xpath expression 

And I have just noticed this
https://www.selenium.dev/blog/2023/invalid-selector-exception-has-changed/

The expected result before this change would be that the driver waits until the timeout expires and then throw an InvalidSelectorException.

This doesn’t make much sense because a broken/invalid selector would never fix itself, and hence should throw immediately.

Really? What about SPA when click can change DOM without page reload? The xpath is valid.

Please note that this may have an impact on backwards compatibility

Why it was released under 4.8.2 and not 5.0.0 ?

@gtach2o
Copy link
Author

gtach2o commented Apr 25, 2023

Got it working after adding InvalidSelectorException into catch
Tested with 4.9.0

  • --remote-allow-origins=* doesn't affect jdk-http-client usage.

I was misled by the lots of important changes made in just a patch release.

Thank you everyone!

@gtach2o gtach2o closed this as completed Apr 25, 2023
@titusfortner
Copy link
Member

  1. We do not do semantic versioning because there are too many different changes that need to be made in different languages
  2. That "breaking change" was a bug fix not a feature change. If you get InvalidSelectorException you will never locate an element with it, regardless of page changes. This is why we changed the error; important information about your code was being hidden by an unrelated error.

@titusfortner
Copy link
Member

I'm going to reopen this because I still think we should consider only setting that value if users are using the old http client.
We can close once we decide if we want to do that or not.

@titusfortner titusfortner reopened this Apr 25, 2023
@diemol
Copy link
Member

diemol commented Apr 25, 2023

Makes sense, there is no need to add this option when the Java 11 client is used.

@diemol diemol closed this as completed in dfe0784 Apr 25, 2023
Copy link

github-actions bot commented Dec 9, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants