1661390 - Use a UUID as session id for targets

Status: VERIFIED FIXED

(Remote Protocol :: CDP, defect, P1)

Description

[ys]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36

Steps to reproduce:

Run firefox with "firefox --headless --remote-debugging-port"

Use Target.attachToTarget method to create a page.

Actual results:

The returned sessionId is an integer type

Expected results:

The sessionId should be string according to the protocol definition from here: https://chromedevtools.github.io/devtools-protocol/tot/Target/#type-SessionID

Comment 1

[ys]

Related to this issue: https://github.com/go-rod/rod/issues/193

Comment 2

[Virginia (please send need info request to Brindusa Tot)]

Hi Ys,

Thanks for your report and for attaching a link with more information regarding issue.

I'll add this ticket to the Widget: cocoa product in the hope someone from their team can take a look.

Regards,
Virginia

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There is indeed the wrong data type returned:
https://searchfox.org/mozilla-central/rev/84922363f4014eae684aabc4f1d06380066494c5/remote/domains/parent/Target.jsm#136-140

We might also have to check additional places where this happens.

ys, is this a blocking bug for you or how manifests it?

Comment 4

[ys]

Henrik Skupin, yes, it's blocking my lib to support firefox: https://github.com/go-rod/rod/issues/193.
A statically typed language like Golang or Java usually needs the JSON type to be consistent.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This is great to hear! Whenever you have issues feel free to file appropriate bugs if those don't exist yet. I also filed bug 1662784 to track all of them.

Regarding this bug I will make sure to take care of it early next week when I'm back.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Currently we set the session id based on the last index used for the previous session. To be in par with Chrome and to lower the attack factor in discovering active sessions we should move to also use a UUID. By doing that we will always have a string type and won't have to run special code when serializing the session id.

Comment 7

[ys]

Sure, UUID is even better, good for avoiding race condition, debugging log, etc.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1661390 - [remote] Use a UUID as session id for targets.

A session is defined as string in CDP but target specific commands
and events currently return a number based on the last used index.
This also makes it very easy to discover active sessions.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adc5e45e75aa
[remote] Use a UUID as session id for targets. r=remote-protocol-reviewers,jgraham

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/adc5e45e75aa

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

ys, this will be fixed in the next Firefox Nightly build that will be available in the next few hours. Please let us know how it works, and if other issues occur please file new bugs. Thanks again for adding Firefox support!

Comment 12

[ys]

Thank you! Checked, now it works as expected.