[🐛 Bug]: Connection.java fails to call event callback for DOM.documentUpdated() event

[jstermask]

What happened?

When using selenium chrome devtools and adding a listener DOM.documentUpdated(), the event is triggered, but an JSON exception is thrown when parsing the 'data' to handle to json. This exception prevents the listener to be actually fired.
e.g. in kotlin

val devTools = myChromeDriver.getDevTools()
devTools.send(DOM.enable(Optional.empty())
devTools.addListener(DOM.documentUpdated()) {
     doSomething()
}

Then when executing, doSomething() is never called because there is an exception in Connection.java line 255 selenium class.

While reading the Connection.java code, line 255, we see that data string to parse is :

{"method":"DOM.documentUpdated","params":{},"sessionId":"33B3953725B65540168B551E57BFF135"}

But code did not expect that "params" attribute value would be another nested JSON object ( {} ). Therefore, when calling input.nextName(), this method fails because it can't parse {} as a string NAME.

To fix this, algorithm should at least try {} catch {} around input.nextName() because sometimes input does contain nested objects, not only strings.

How can we reproduce the issue?

On DevTools, add a Listener on DOM.documentUpdated().

Relevant log output

ov 07, 2023 3:48:11 PM org.openqa.selenium.devtools.Connection$Listener lambda$onText$0
WARNING: Unable to process: {"method":"DOM.documentUpdated","params":{},"sessionId":"33B3953725B65540168B551E57BFF135"}
org.openqa.selenium.json.JsonException: Expected to read a NAME but instead have: START_MAP. Last 41 characters read: {"method":"DOM.documentUpdated","params":
Build info: version: '4.15.0', revision: '1d14b5521b'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '5.15.90.1-microsoft-standard-WSL2', java.version: '17.0.8.1'
Driver info: driver.version: unknown
	at org.openqa.selenium.json.JsonInput.expect(JsonInput.java:467)
	at org.openqa.selenium.json.JsonInput.nextName(JsonInput.java:190)
	at org.openqa.selenium.devtools.Connection.lambda$handle$5(Connection.java:305)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.stream.ReferencePipeline$15$1.accept(ReferencePipeline.java:541)
	at java.base/java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1850)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.openqa.selenium.devtools.Connection.handle(Connection.java:297)
	at org.openqa.selenium.devtools.Connection$Listener.lambda$onText$0(Connection.java:225)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

Operating System

Ubuntu

Selenium version

4.15.0

What are the browser(s) and version(s) where you see this issue?

Chrome 119.0.6045.105

What are the browser driver(s) and version(s) where you see this issue?

Chrome 116, 119.0.6045.105

Are you using Selenium Grid?

No

[github-actions]

@jstermask, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[github-actions]

Hi, @jstermask.
Please follow the issue template, we need more information to reproduce the issue.

Either a complete code snippet and URL/HTML (if more than one file is needed, provide a GitHub repo and instructions to run the code), the specific versions used, or a more detailed description to help us understand the issue.

Note: If you cannot share your code and URL/HTML, any complete code snippet and URL/HTML that reproduces the issue is good enough.

Reply to this issue when all information is provided, thank you.

[joerg1985]

@jstermask could you check if this issue has been fixed in the latest snapshot?

[jstermask]

Hello,
I confirm that issue is still in latest snapshot and also on trunk (latest branch)

selenium/java/src/org/openqa/selenium/devtools/Connection.java

Line 255 in b5822ce

switch (input.nextName()) {

L255, the JSON payload is parsed using nextName() method. This method expects the JSON to be only made of simple strings. But the reality is not that.

// Here is the DOM.documentUpdated() that has to be handled by Connection.java component
{"method":"DOM.documentUpdated","params":{},"sessionId":"33B3953725B65540168B551E57BFF135"}

In the above payload, that is handled by selenium Connection.java, we can see that "params" value is not a string but a JSON object. Therefore the inputName() method crashes in this case.

[joerg1985]

@jstermask did you run it or did you only read the code? I am pretty sure this has been fixed in a5591ef the mapping method does read the empty object now and the next thing to read is a name again, so the nextName call will not fail.

[jstermask]

Hi , I encountered the bug initially by running the code (4.15.0). Then once you asked me to check again, I only read the code in trunk branch, so it's likely is misread it. Indeed, commit a5591ef looks good. Thanks Le jeu. 9 nov. 2023, 18:00, joerg1985 ***@***.***> a écrit :

…

@jstermask <https://github.com/jstermask> did you run it or did you only read the code? I am pretty sure this has been fixed in a5591ef <a5591ef> the mapping method does read the empty object now and the next thing to read is a name again, so the nextName call will not fail. — Reply to this email directly, view it on GitHub <#13109 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE6EOIDAQVME26ATAGHMUVDYDUDZHAVCNFSM6AAAAAA7BKYVIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBUGIYDQNZXGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[joerg1985]

@jstermask sorry for the confusion and thank you for the bug report.

I will close this now, the issue should be fixed.

[github-actions]

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.