selenoid icon indicating copy to clipboard operation
selenoid copied to clipboard

Support missing for JSON Version

Open LukeIGS opened this issue 1 year ago • 18 comments

Some clients (for example, Ferrum for ruby) utilize this endpoint to gather version information about the browser, as well as the websocket debugger url.

LukeIGS avatar Mar 13 '23 20:03 LukeIGS

Probably we just need to support this: https://github.com/aerokube/selenoid/issues/1063

vania-pooh avatar Mar 14 '23 08:03 vania-pooh

in Ferrum's case at least, it doesn't seem to have any care "where" the websocket endpoint lives. Selenium for ruby doesn't really care beyond its initial checks either; However, for some reason if you bypass those checks it dies when you attempt to call anything using The bidirectional api with a network timeout.

If you bypass any of that both "will" connect to the endpoint, allowing you to execute raw cdp commands just fine, though for a good connection from Ferrum at least, we would need the json/version endpoint to be supported alongside the current json/protocal as per this spec, as ferrum uses the former to gather version information like the websocket endpoint for the cluster.

GET /json/version

{
    "Browser": "Chrome/72.0.3601.0",
    "Protocol-Version": "1.3",
    "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3601.0 Safari/537.36",
    "V8-Version": "7.2.233",
    "WebKit-Version": "537.36 (@cfede9db1d154de0468cb0538479f34c0755a0f4)",
    "webSocketDebuggerUrl": "ws://localhost:9222/devtools/browser/b0b8a4fb-bb17-4359-9533-a8d9f3908bd8"
}

Given this selenium and ferrum both don't really care where the websocket lives, but for whatever reason, Target.getTargets will always return an empty response... so while you can execute raw cdp commands just fine like Performance.getMetrics, any event based responses just time out.

Edit: Upon closer inspection, selenium's ruby code seems to look for the CDP websocket the same way, so just supporing json/version per spec would likely fix a lot of our CDP related woes.

LukeIGS avatar Mar 15 '23 11:03 LukeIGS

specs for relevance. https://chromedevtools.github.io/devtools-protocol

LukeIGS avatar Mar 15 '23 11:03 LukeIGS

Ferrum attempting to fetch this endpoint: https://github.com/rubycdp/ferrum/blob/5bf72255a1fa2b27783e3549c13ccbd83529154c/lib/ferrum/browser/process.rb#L66

Selenium-webdriver for ruby also attempting to fetch this endpoint: https://github.com/SeleniumHQ/selenium/blob/trunk/rb/lib/selenium/webdriver/chromium/driver.rb#L50

LukeIGS avatar Mar 15 '23 13:03 LukeIGS

Looks like the change would take place here... https://github.com/aerokube/images/blob/e943c3a5f4706ca3b8a5d284fc09f63b642e0806/static/chrome/devtools/devtools.go#L81

Selenoid's CDP router seems to only define json/protocol, it should probably be more up to the linked specs and support the following at the bare minimum: GET /json/list and GET /json - used by multiple cdp clients to get a target list for bidirectional apis GET /json/version - Used to describe version information and websocket location WebSocket /devtools/page/{targetId} - Used to attach to a given taget via websocket

Though it might be a good idea to just follow the specs completely for CDP functionality. se:cdp is a vendor namespaced extension, if selenoid wants to masquerade as selenium, we would need that defined, as it would just make it compatible with a lot of clients out of box, even if it's not in the w3c spec, however that's a separate concern than this is I think.

LukeIGS avatar Mar 16 '23 11:03 LukeIGS

@LukeIGS we were aware of these API when this binary was initially created. However the issue is that all these APIs return WebSocket URLs pointing to 127.0.0.1 whereas in Selenoid case this should be Selenoid host and some session ID. So we decided to not patch response of this URL as too complicated operation.

vania-pooh avatar Mar 16 '23 14:03 vania-pooh

would it be possible to simply take the incoming host of the request and return that as part of the response? I believe that's how selenium handles it.

LukeIGS avatar Mar 20 '23 14:03 LukeIGS

@LukeIGS what do you mean by "incoming host"? How is this expected to work when our tools are running behind one or several reverse-proxies and behind a fault-tolerant load balancer?

vania-pooh avatar Mar 20 '23 15:03 vania-pooh

most load balancers and proxies should be forwarding the host header [of the request] to the service in question given they are configured in a sane way. Granted this wouldn't work if there were context paths involved, at least not on its own. The proxy shoudl also be forwarding the path header in most cases of the request i'd expect.

LukeIGS avatar Mar 20 '23 15:03 LukeIGS

this is all especially true if you're using any sort of https termination, as the proxy would need to forward tls encrypted requests to the service stack in question (be it kubernetes or docker), and the host header is used to compare against the expected host of a served tls certificate.

LukeIGS avatar Mar 20 '23 16:03 LukeIGS

quick sketch of a go function that would probably do what we need

type versionTransport struct {
	http.RoundTripper
}

func (t *versionTransport) RoundTrip(req *http.Request) (resp *http.Response, err error) {
	resp, err = t.RoundTripper.RoundTrip(req)
	if err != nil {
			return nil, err
	}
	b, err := ioutil.ReadAll(resp.Body)
	if err != nil {
			return nil, err
	}
	err = resp.Body.Close()
	if err != nil {
			return nil, err
	}
	b = bytes.Replace(b, 
		[]byte("\"webSocketDebuggerUrl\": \"ws://localhost:9222"), 
		[]byte( fmt.Sprint("\"webSocketDebuggerUrl\": \"ws://%s", req.Host)), 
		-1,
	)
	body := ioutil.NopCloser(bytes.NewReader(b))
	resp.Body = body
	resp.ContentLength = int64(len(b))
	resp.Header.Set("Content-Length", strconv.Itoa(len(b)))
	return resp, nil
}


func version(w http.ResponseWriter, r *http.Request) {


	h, err := devtoolsHost()
	if err != nil {
		log.Printf("[DEVTOOLS_HOST_ERROR] [%v]", err)
		http.Error(w, fmt.Sprintf("Failed to detect devtools host: %v", err), http.StatusInternalServerError)
		return
	}
	u := &url.URL{
		Host:   h,
		Scheme: "http",
		Path:   "/json/version",
	}
	log.Printf("[PROTOCOL] [%s]", u.String())
	(&httputil.ReverseProxy{
		Director: func(r *http.Request) {
			r.Host = "localhost"
			r.URL = u
		},
		Transport: &versionTransport{http.DefaultTransport},
	}).ServeHTTP(w, r)
}

With my (albeit) limited knowledge of go, this function would probably do something akin to what we'd need provided we were working with a proxy that was routing on host alone..

I'm sure someone with better go would be able to write something way better though.

LukeIGS avatar Mar 20 '23 19:03 LukeIGS

@LukeIGS have you tried to build a custom Selenoid version with the changes you proposed?

andrii-rymar avatar Sep 01 '23 09:09 andrii-rymar

I have not. I've mostly been switching to just using playwright instead of w3c driver based solutions.

LukeIGS avatar Sep 01 '23 11:09 LukeIGS

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jan 12 '24 01:01 github-actions[bot]

I'd still love support for Ferrum if possible, but it might be worth considering playwright-ruby-client instead (haven't done that yet, it seems much less popular than Ferrum).

Juice10 avatar Jan 22 '24 15:01 Juice10

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Apr 12 '24 01:04 github-actions[bot]

bump

Juice10 avatar Apr 15 '24 16:04 Juice10

PRs are welcome.

vania-pooh avatar Apr 16 '24 13:04 vania-pooh