-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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]: Need a way to enforce HTTP1.1 usage with the Java HTTP client #12918
Comments
@mykola-mokhnach, thank you for creating this issue. We will troubleshoot it as soon as we can. Info for maintainersTriage this issue by using labels.
If information is missing, add a helpful comment and then
If the issue is a question, add the
If the issue is valid but there is no time to troubleshoot it, consider adding the
If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C),
add the applicable
After troubleshooting the issue, please add the Thank you! |
I started working on this, looks straightforward, hope to have something we can put in nightly |
I think this is nothing the client should do. The inital request is done with http 1.1 in any case and will ony upgrade to http/2 when the server will respond with switch protocol. The server should remove the http Upgrade header (not the one for WebSockets ;)) to avoid the http/2 upgrade. |
maybe. What I observe for now is that we did not see connection issues with previous client versions using netty @asolntsev got a similar results with their tests. And enforcing HTTP version was the only workaround I was able to google. |
@mykola-mokhnach I just had a look at the initial request and it is like expected. The only difference in the request are the 'Connection', 'HTTP2-Settings', 'Upgrade' headers. HTTP2 enabled client:
HTTP2 disabled client:
So i assume either the Appium http server or one of the drivers are not able to process this headers. If you need to have a workaround for now, it might be possible to create a system property that selenium will read and force the http1.1 version. @diemol @titusfortner what do you think about this? this should be removed in selenium 5 and is only a workaround for this issue |
As I see it:
We should at least file an issue for the first one I prefer 4 to 3 from the Selenium side. |
|
Is there any consideration of switching off ExpressJS or using spdy package with it? |
this would be a huge refactoring without any visible improvements and multiple breaking changes
The last commit to spdy package happened 2 years ago. It has been incorporated recently, although is only used if server-side https is enabled. It's too much of risk to enable it by default for plain http. cc @jlipps |
@mykola-mokhnach i do not think the appium server must support http/2 to fix this. The upgrade to http/2 is only done by the client when the server does answer with So again the question, does the appium server allready remove these headers from the request before sending it to the driver? |
If this is only about removing particular headers then I could try something like appium/java-client@924838f |
These headers are added by the java http client automatically, there is no way to suppress this on the client side. These headers are part of the protocol and the http client does not allow to break the protocol. The filter must be added to the server side, i think there is no other way to handle this. |
Then I would rather use the solution provided by @titusfortner
|
I agree you need a workaround to this for some time in any case. |
Is what was implemented in Selenium in 4.14.1 sufficient to close this? |
yes, I would say it is ok now. |
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. |
Feature and motivation
Currently it is not possible to enforce HTTP1.1 usage in the client. This feature is needed because Appium server only supports HTTP1.1 so far (SPDY support has been added recently, but it's optional), so we'd like to default to HTTP1.1 for good.
Usage example
Not sure yet where exactly this needs to live.
The text was updated successfully, but these errors were encountered: