Danp2 / au3WebDriver

Web Driver UDF for AutoIt
MIT License
107 stars 21 forks source link

Revise __WD_EscapeString to further encode strings #469

Closed Danp2 closed 1 year ago

Danp2 commented 1 year ago

Pull request

Proposed changes

Use Json_StringEncode to perform string encoding. Also support stripping tabs and CR/LFs

Checklist

Put an x in the boxes that apply. If you're unsure about any of them, don't hesitate to ask. We are here to help!
This is simply a reminder of what we are going to look for before merging your code.

Types of changes

Please check x the type of change your PR introduces:

What is the current behavior?

Please describe the current behavior that you are modifying, or link to a relevant issue.

What is the new behavior?

See #465

Additional context

Add any other context about the problem here.

System under test

Please complete the following information.

mlipok commented 1 year ago

Will look deeply this weekend

mlipok commented 1 year ago

According to https://github.com/Danp2/au3WebDriver/issues/466 https://github.com/Danp2/au3WebDriver/blob/8340123db20a3bcb684f153dc29a8630eadab41d/wd_helper.au3#L2611-L2617

This line https://github.com/Danp2/au3WebDriver/blob/8340123db20a3bcb684f153dc29a8630eadab41d/wd_helper.au3#L2617

should be changed to:

$sResult = _WD_ExecuteScript($sSession, $sScript, __WD_JsonElement($sElement) & ',"' & __WD_EscapeString($sValue) & '"')
mlipok commented 1 year ago

_WD_Storage() SET should be checked if $vValue should be escaped with __WD_EscapeString() _WD_Storage() GET should be checked if $vResult should be de escaped with Json_StringDecode()

Danp2 commented 1 year ago

@mlipok Discussion on _WD_Storage doesn't belong in this PR. Feel free to submit a separate issue or a PR with suggested changes.

mlipok commented 1 year ago

the last thing to consider is:

According to #466

https://github.com/Danp2/au3WebDriver/blob/8340123db20a3bcb684f153dc29a8630eadab41d/wd_helper.au3#L2611-L2617

after which I think this is fine and could be merged.

Danp2 commented 1 year ago

@mlipok Again, separate issue that would be handled, if needed, as a separate PR.

mlipok commented 1 year ago

So here I'm fine with this PR