Skip to content

fix: keep the url's query string when probing /json/version - #625

Merged
route merged 1 commit into
mainfrom
fix/external-url-query
Sep 1, 2026
Merged

fix: keep the url's query string when probing /json/version#625
route merged 1 commit into
mainfrom
fix/external-url-query

Conversation

@route

@route route commented Aug 29, 2026

Copy link
Copy Markdown
Member

Process#parse_json_version built the probe with URI.join(url, "/json/version"), which drops the query string. A browser reached through a proxy that authenticates with one — Ferrum::Browser.new(url: "http://chrome:3000?token=X") — therefore got a 401 HTML page, JSON::ParserError was swallowed, and ws_url= was handed nil:

undefined method 'host' for nil (NoMethodError)
  lib/ferrum/browser/process.rb:211:in 'ws_url='
  lib/ferrum/browser/process.rb:54:in 'initialize'

The probe url is now built by rewriting only the scheme and path, keeping the query (and everything else) intact. Same path is used for ws_url:, so a token on the websocket url survives too.

Also: an endpoint that answers but yields no websocket url now raises Ferrum::NoWebSocketUrlError naming the url, instead of the NoMethodError above.

Two system specs in spec/browser_spec.rb put a token-authenticated proxy in front of a real external browser — one connects through it, one asserts the new error on a bad token. Both reproduce the reported NoMethodError on main. Full suite: 609 examples, 0 failures.

@route
route force-pushed the fix/external-url-query branch 2 times, most recently from 3b92a7d to e9daa69 Compare September 1, 2026 17:32
`/json/version` was probed with the url's query string dropped, so an
endpoint that authenticates with `?token=...` answered 401 and `:url` died
with `undefined method 'host' for nil`.
@route
route force-pushed the fix/external-url-query branch from e9daa69 to 4c522a5 Compare September 1, 2026 17:49
@route
route merged commit e14c947 into main Sep 1, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant