fix(web): keep resource-id a string under form named-property access#3320
Open
ChaMinGyuSoftleaf wants to merge 1 commit into
Open
fix(web): keep resource-id a string under form named-property access#3320ChaMinGyuSoftleaf wants to merge 1 commit into
ChaMinGyuSoftleaf wants to merge 1 commit into
Conversation
…obile-dev-inc#2944) On a <form> with a child control named "id"/"name" (common on login forms, e.g. <input name="id">), node.id/node.name resolve to that child element instead of the string attribute via HTML named property access. maestro-web.js passed that element into resource-id, which then threw while walking the hierarchy: ClassCastException: LinkedHashMap cannot be cast to String (parseDomAsTreeNodes) This deterministically broke any flow navigating to such a page (typically a login screen). - maestro-web.js: fall back to getAttribute('id')/getAttribute('name') when node.id/node.name isn't a string (keeps resource-id a string, preserves the form's real id). - CdpWebDriver / WebDriver parseDomAsTreeNodes: defensively coerce resource-id as a second line of defense. - Add unit tests for both drivers covering the non-string resource-id case.
Contributor
|
From your description, you're suggesting abandoning #3306? |
Author
|
No, they're complementary. This PR fixes the trigger I could actually reproduce Fine to land both as far as I'm concerned, or I can rebase this on top of #3306 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the web (Chromium)
ClassCastException: LinkedHashMap cannot be cast to Stringcrash from #2944. It deterministically breaks any flow that navigates to a page with a
<form>whose child control is namedidorname— extremely common on login screens(
<input name="id">).This is complementary to #3306 (the
boundshardening): that PR guardsbounds, whilethis one addresses the attribute that actually triggers #2944 —
resource-id. In currentmain,boundsandtextare always built as strings, soresource-idis the onlyattribute that can be non-string from this path.
Root cause
maestro-web.jsbuildsresource-idfromnode.id || node.ariaLabel || node.name || ….HTMLFormElement(and a few other elements) expose their child controls as properties viaHTML named property access,
so for
<form><input name="id"></form>,form.idreturns the child<input>element,not the string
idattribute. That element serializes to an object and then hits theunguarded
as Stringcast inparseDomAsTreeNodes:Changes
maestro-web.js(root cause): whennode.id/node.nameisn't a string, fall backto
getAttribute('id')/getAttribute('name'). Keepsresource-ida string andpreserves the form's real id instead of dropping to the next fallback.
CdpWebDriver/WebDriverparseDomAsTreeNodes(defense-in-depth): defensivelycoerce
resource-id(as? String ?: toString()) so a stray non-string value degradesgracefully instead of crashing the hierarchy walk. Applied to both drivers.
CdpWebDriverTest,WebDriverTest): cover the normal stringresource-idand the non-string (named-property) case for both drivers.Reproduction
Two static pages served via
python3 -m http.server:index.htmllogin.htmlFlow:
Tapping the link navigates to
login.html; the post-tapwaitForAppToSettleparses thenew DOM and crashes on
main.Verification
./gradlew :maestro-client:test— the new tests pass; they fail with theClassCastExceptionwithout the driver change.maestro-web.jsswapped into the 2.6.0 jar (Chromium driver). Both theminimal repro above and a real login page crashed before / pass after. Renaming only
name="id"→name="memberId"also avoids the crash, confirming the trigger.Closes #2944