-
-
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
JDKHttpClient: Handle RQST with no Body [POST/PUT] #11445
JDKHttpClient: Handle RQST with no Body [POST/PUT] #11445
Conversation
Here's how I have tested this
version: "3"
services:
node-docker:
image: selenium/node-docker:local-20221213
volumes:
- ./config.toml:/opt/bin/config.toml
- ./assets:/opt/selenium/assets
depends_on:
- selenium-hub-dynamic
environment:
- SE_EVENT_BUS_HOST=selenium-hub-dynamic
- SE_EVENT_BUS_PUBLISH_PORT=4442
- SE_EVENT_BUS_SUBSCRIBE_PORT=4443
ports:
- "5005:5005"
selenium-hub-dynamic:
image: selenium/hub:local-20221213
container_name: selenium-hub-dynamic
ports:
- "4442:4442"
- "4443:4443"
- "4444:4444"
[docker]
configs = [
"selenium/standalone-firefox:local-20221213","{\"browserName\": \"firefox\"}"
]
# Windows: make sure Docker Desktop exposes the daemon via tcp, and use http://host.docker.internal:2375.
# macOS: install socat and run the following command, socat -4 TCP-LISTEN:2375,fork UNIX-CONNECT:/var/run/docker.sock,
# then use http://host.docker.internal:2375.
# Linux: varies from machine to machine, please mount /var/run/docker.sock. If this does not work, please create an issue.
# I am on mac
url = "http://host.docker.internal:2375"
import java.net.MalformedURLException;
import org.openqa.selenium.firefox.FirefoxOptions;
import org.openqa.selenium.remote.RemoteWebDriver;
import java.net.URL;
public class Sample {
public static void main(String[] args) throws MalformedURLException {
RemoteWebDriver driver = null;
try {
driver = new RemoteWebDriver(new URL("http://localhost:4444"), new FirefoxOptions());
driver.get("http://www.google.com");
System.err.println("Title " + driver.getTitle());
} finally {
if (driver != null) {
driver.quit();
}
}
}
} |
Logic-wise LGTM. |
3707a56
to
ccf6278
Compare
ccf6278
to
6fd1537
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @krmahadevan!
@diemol I'm assuming this is still not in the latest docker images (4.7.2-20221219) as I am still seeing on dynamic grid: Hub:
Node:
Any timescales for release? |
After upgraded to 4.7.2-20221219, I came across to this issue. |
+1, either this did not get released in |
Folks, just to clarify. This PR was merged on Dec 20, 2022 and |
We have a 4.8 released plan soon (in about a week or so). |
Hi, Below are the error logs and my dynamic-grid yml file as well as config.toml selenium-hub
node
The error logs in terminal look like:
This is my docker-compose-v3-dynamic-grid.yml
and this is my dynamic-grid-config.toml
This is how I am initializing the remote webdriver for each of the browsers e.g. for MicrosoftEdge
The Grid and Node is setup but the docker does not creates new containers based on session requests dynamically. |
Based on the logs, the content length is not zero, which is the root cause of this PR is trying to solve. Please file an issue or reach out on https://www.selenium.dev/support/#ChatRoom to get further help. |
@pujagani What's the best way to be notified about the next release? |
We release it - https://github.com/SeleniumHQ/selenium/releases (you can set Github notifications to watch for just releases) |
Fixes: #11342
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Here's what I have as analysis:
org.openqa.selenium.remote.http.jdk.JdkHttpMessages#createRequest
which internally callsorg.openqa.selenium.remote.http.jdk.JdkHttpMessages#notChunkingBodyPublisher
for aPOST
message.Note:
We cannot set a payload also for this request, since when we do that, I hit an error as below
with a
400 BAD REQUEST
This PR fixes this discrepancy by ensuring that when we are confronted with empty payloads for POST|PUT messages and when we are using the JDK provided HTTP Client, we resort to using an empty body
Motivation and Context
Types of changes
Checklist