issue in _WD_ExecuteScript / __WD_EscapeString when comments // was used in JavaScript string

mlipok commented 1 year ago

Bug report

Describe the bug

$sData = StringRegExpReplace($sData, '[\v\t]', '') from here: https://github.com/Danp2/au3WebDriver/blob/42dda7d9a9e78b3c7cc2d419b0e84eaf1335f6a8/wd_core.au3#L1747-L1749

strips [\v] chars and when // comment is used entire $sJavaScript becames commented out.

How to reproduce

_Example()
Func _Example()
    Local $sJavaScript = _
            "// comment" & @CRLF & _
            "return document.readyState;" & @CRLF & _
            ""
    _WD_DebugSwitch($_WD_DEBUG_Full)
    _WD_ExecuteScript($sSession, $sJavaScript)
    _WD_DebugSwitch()
EndFunc   ;==>_Example

Expected behavior

get result from: return document.readyState;

Screenshots

none

Additional context

https://www.autoitscript.com/forum/topic/208640-webdriver-udf-help-support-iv/?do=findComment&comment=1521863

System under test

not related

mlipok commented 1 year ago

interesting. Using:

_Example()
Func _Example()
    Local $sJavaScript = _
            "// comment" & @CRLF & _
            "return document.readyState;" & @CRLF & _
            ""
    _WD_DebugSwitch($_WD_DEBUG_Full)
    _WD_ExecuteScript($sSession, $sJavaScript)
    _WD_DebugSwitch()
EndFunc   ;==>_Example

I get proper results:

__WD_Post: URL=HTTP://127.0.0.1:4444/session/e1192324-9090-4743-8d1c-c390f190cdda/execute/sync; Data={"script":"\/\/ comment\r\nreturn document.readyState;\r\n", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":"complete"}

the same result with:

_Example()
Func _Example()
    Local $sJavaScript = _
            "// comment" & @CR & _
            "return document.readyState;" & @CR & _
            ""
    _WD_DebugSwitch($_WD_DEBUG_Full)
    _WD_ExecuteScript($sSession, $sJavaScript)
    _WD_DebugSwitch()
EndFunc   ;==>_Example

and with:

_Example()
Func _Example()
    Local $sJavaScript = _
            "// comment" & @LF & _
            "return document.readyState;" & @LF & _
            ""
    _WD_DebugSwitch($_WD_DEBUG_Full)
    _WD_ExecuteScript($sSession, $sJavaScript)
    _WD_DebugSwitch()
EndFunc   ;==>_Example

Danp2 commented 1 year ago

I get proper results:

Not me. I got this --

     __WD_Post: URL=HTTP://127.0.0.1:4444/session/0cab983f-b61b-4acf-afa7-055cb4397c0a/execute/sync; Data={"script":"\/\/ commentreturn document.readyState;", "args":[]}
     __WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":null}

Edit: Notice the difference in the Data portion of mine VS yours. Are you sure you are testing with the latest release?

mlipok commented 1 year ago

I get proper results:

__WD_Post: URL=HTTP://127.0.0.1:4444/session/e1192324-9090-4743-8d1c-c390f190cdda/execute/sync; Data={"script":"\/\/ comment\r\nreturn document.readyState;\r\n", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":"complete"}

oops.. my mistake. I had little modified verison because I was testing this issue.

my results are:

_WD_DebugSwitch:  /  stack size: 2
__WD_Post: URL=HTTP://127.0.0.1:4444/session/758b3de6-2777-4a45-a73b-c4171a079a57/execute/sync; Data={"script":"\/\/ commentreturn document.readyState;", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":null}
_WD_ExecuteScript ==> Success [0]

the issue can be solved by:

Func __WD_EscapeString($sData, $iOption = 0)
    If BitAND($iOption, $JSON_MLREFORMAT) Then
;~      $sData = StringRegExpReplace($sData, '[\v\t]', '') ; Strip tabs and CR/LFs ; original
        $sData = StringRegExpReplace($sData, '\t', '') ; Strip tabs ; modified

and results are:

_WD_DebugSwitch:  /  stack size: 2
__WD_Post: URL=HTTP://127.0.0.1:4444/session/cbaddcad-3adb-4f2e-91da-8fb50c008226/execute/sync; Data={"script":"\/\/ comment\nreturn document.readyState;\n", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":"complete"}
_WD_ExecuteScript ==> Success [0]

mlipok commented 1 year ago

we should re-test: https://github.com/Danp2/au3WebDriver/issues/465

mlipok commented 1 year ago

from: https://github.com/Danp2/au3WebDriver/issues/465 REPRO: Using file: UserTesting_EscapeString.au3 with following content:

ConsoleWrite("! Start: UserTesting_EscapeString.au3" & @CRLF)

ConsoleWrite("- Test 1:" & @CRLF)
_WD_Navigate($sSession, 'https://www.google.com')
_WD_LoadWait($sSession)
MsgBox($MB_OK + $MB_TOPMOST + $MB_ICONWARNING, "Warning #" & @ScriptLineNumber, "If you see COOKIE accept panel close them before you will continue.")

ConsoleWrite("- Test 2:" & @CRLF)
; REMARK:  __SetVAR($IDX_VAR, $value) will set  $_VAR[$IDX_VAR] = $value
__SetVAR(0, _WD_FindElement($sSession, $_WD_LOCATOR_ByXPath, "//textarea[@name='q']"))

ConsoleWrite("- Test 3:" & @CRLF)
__SetVAR(1, ' " ! @ # $ % ^ & * ( ) - _ + = { } [ ] ; : \ | , . < > / ? ~ ' & " ' " & ' @CR' & @CR & ' @CRLF' & @CRLF & ' @LF' & @LF & ' @TAB' & @TAB & 'END')
_WD_ElementAction($sSession, $_VAR[0], "VALUE", $_VAR[1])
;~ _WD_SetElementValue($sSession, $_VAR[0], $_VAR[1], $_WD_OPTION_Advanced)

ConsoleWrite("! $_VAR[1] -" & @CRLF & $_VAR[1] & @CRLF)

ConsoleWrite("! __WD_EscapeString($_VAR[0]) -" & @CRLF & __WD_EscapeString($_VAR[1]) & @CRLF)
ConsoleWrite("! END: UserTesting_EscapeString.au3" & @CRLF)

together with current modification:

Func __WD_EscapeString($sData, $iOption = 0)
    If BitAND($iOption, $JSON_MLREFORMAT) Then
;~      $sData = StringRegExpReplace($sData, '[\v\t]', '') ; Strip tabs and CR/LFs ; original
        $sData = StringRegExpReplace($sData, '\t', '') ; Strip tabs ; modified

I get:

_WD_FindElement ==> Success [0] : Parameters:   Strategy=xpath   Selector=//textarea[@name='q']   StartNodeID=Default   Multiple=Default   ShadowRoot=Default
- Setting $_VAR[0] = 9559b64a-277b-4171-b3fb-e1ace0755842
- Test 3:
- Setting $_VAR[1] =  " ! @ # $ % ^ & * ( ) - _ + = { } [ ] ; : \ | , . < > / ? ~  '  @CR
 @CRLF
 @LF
 @TAB   END
__WD_Post ==> Success [0] : HTTP status = 200
_WD_ElementAction ==> Success [0] : Parameters:   Command=VALUE   Option=<masked>
! $_VAR[1] -
 " ! @ # $ % ^ & * ( ) - _ + = { } [ ] ; : \ | , . < > / ? ~  '  @CR
 @CRLF
 @LF
 @TAB   END
! __WD_EscapeString($_VAR[0]) -
 \" ! @ # $ % ^ & * ( ) - _ + = { } [ ] ; : \\ | , . < > \/ ? ~  '  @CR\r @CRLF\r\n @LF\n @TAB\tEND
! END: UserTesting_EscapeString.au3

for me the following fix

Func __WD_EscapeString($sData, $iOption = 0)
    If BitAND($iOption, $JSON_MLREFORMAT) Then
;~      $sData = StringRegExpReplace($sData, '[\v\t]', '') ; Strip tabs and CR/LFs ; original
        $sData = StringRegExpReplace($sData, '\t', '') ; Strip tabs ; modified

works fine.

mlipok commented 1 year ago

DemoUpload from wd_demo.au3 also works fine.

mlipok commented 1 year ago

@Danp2 If you agree with my testing route and results, I pass the solution of this problem to you (please make PR).

mlipok commented 1 year ago

btw. __WD_EscapeString tested also with: https://github.com/Danp2/au3WebDriver/issues/486#issuecomment-1656792340

results - works fine.

Danp2 commented 1 year ago

we should re-test: #465

Yes, I think it is important to understand why the original change was made before reverting so that we can hopefully avoid creating a different issue.

mlipok commented 1 year ago

I think this was my mistake.

mlipok commented 1 year ago

quick review and I think it starts here: https://github.com/Danp2/au3WebDriver/issues/465#issuecomment-1536057694

mlipok commented 1 year ago

finall fix proposal:

; #INTERNAL_USE_ONLY# ===========================================================================================================
; Name ..........: __WD_EscapeString
; Description ...: Escapes designated characters in string.
; Syntax ........: __WD_EscapeString($sData[, $iOption = 0])
; Parameters ....: $sData   - the string to be escaped
;                  $iOption - [optional] Any combination of $JSON_* constants. Default is 0.
; Author ........: Danp2, mLipok
; Modified ......:
; Remarks .......: $JSON_MLREFORMAT will strip tabs from a multiline string (ie. indentation) as they are not supported by WebDriver, use \t where it is needed
;                  See $JSON_* constants in json.au3 for other $iOption possibilities.
; Related .......:
; Link ..........:
; Example .......: No
; ===============================================================================================================================
Func __WD_EscapeString($sData, $iOption = 0)
    If BitAND($iOption, $JSON_MLREFORMAT) Then
        $sData = StringRegExpReplace($sData, '\t', '') ; Strip tabs
        $iOption = BitXOR($iOption, $JSON_MLREFORMAT) ; Flip bit off
    EndIf

    $sData = Json_StringEncode($sData, $iOption) ; Escape JSON Strings
    Return SetError($_WD_ERROR_Success, 0, $sData)
EndFunc   ;==>__WD_EscapeString

Danp2 commented 1 year ago

An alternative is to drop the use of $JSON_MLREFORMAT in _WD_ExecuteScript, but still awaiting your clarification here.

mlipok commented 1 year ago

take a look here: https://github.com/Danp2/au3WebDriver/commit/5bb7a7a5c360cab52c0613a93ae1e0515ebbb26b#r123141584

from this time my experience says to mee each time I use TABS in $sJavaScript they need to be stripped.

https://github.com/Danp2/au3WebDriver/blob/5bb7a7a5c360cab52c0613a93ae1e0515ebbb26b/wd_helper.au3#L2276

mlipok commented 1 year ago

I will do test today to clarify if TABS are allowed or not.

btw. https://www.autoitscript.com/forum/topic/208640-webdriver-udf-help-support-iv/?do=findComment&comment=1521937

Danp2 commented 1 year ago

If your results match mine, then I would suggest that we drop the usage of $JSON_MLREFORMAT

mlipok commented 1 year ago

So please do this test:

run DemoSelectOptions from wd_demo.au3 for Chrome browser
check results
open wd_helper.au3
change all , @TAB, '') to , '@TAB', '')
skip If BitAND($iOption, $JSON_MLREFORMAT) Then
run DemoSelectOptions from wd_demo.au3 for Chrome browser
check results

this should explain why I said that TABS used directly in $sJavaScript are not supported by WebDriver

my results are:

! Error = 8 occurred on: DemoSelectOptions
! _WD_LastHTTPResult = 404
! _WD_LastHTTPResponse = {"value":{"error":"no such element","message":"no such element: Unable to locate element: {\"method\":\"xpath\",\"selector\":\"//*[@name=\"pets\"]\"}\n  (Session info: chrome=115.0.5790.110)","stacktrace":"Backtrace:\n\tGetHandleVerifier [0x0058A813+48355]\n\t(No symbol) [0x0051C4B1]\n\t(No symbol) [0x00425358]\n\t(No symbol) [0x004509A5]\n\t(No symbol) [0x00450B3B]\n\t(No symbol) [0x0047E232]\n\t(No symbol) [0x0046A784]\n\t(No symbol) [0x0047C922]\n\t(No symbol) [0x0046A536]\n\t(No symbol) [0x004482DC]\n\t(No symbol) [0x004493DD]\n\tGetHandleVerifier [0x007EAABD+2539405]\n\tGetHandleVerifier [0x0082A78F+2800735]\n\tGetHandleVerifier [0x0082456C+2775612]\n\tGetHandleVerifier [0x006151E0+616112]\n\t(No symbol) [0x00525F8C]\n\t(No symbol) [0x00522328]\n\t(No symbol) [0x0052240B]\n\t(No symbol) [0x00514FF7]\n\tBaseThreadInitThunk [0x767D7D59+25]\n\tRtlInitializeExceptionChain [0x77A5B79B+107]\n\tRtlClearBits [0x77A5B71F+191]\n"}}

mlipok commented 1 year ago

last post edited (sorry for the confusion)

mlipok commented 1 year ago

my results are:

! Error = 8 occurred on: DemoSelectOptions
! _WD_LastHTTPResult = 404
! _WD_LastHTTPResponse = {"value":{"error":"no such element","message":"no such element: Unable to locate element: {\"method\":\"xpath\",\"selector\":\"//*[@name=\"pets\"]\"}\n  (Session info: chrome=115.0.5790.110)","stacktrace":"Backtrace:\n\tGetHandleVerifier [0x0058A813+48355]\n\t(No symbol) [0x0051C4B1]\n\t(No symbol) [0x00425358]\n\t(No symbol) [0x004509A5]\n\t(No symbol) [0x00450B3B]\n\t(No symbol) [0x0047E232]\n\t(No symbol) [0x0046A784]\n\t(No symbol) [0x0047C922]\n\t(No symbol) [0x0046A536]\n\t(No symbol) [0x004482DC]\n\t(No symbol) [0x004493DD]\n\tGetHandleVerifier [0x007EAABD+2539405]\n\tGetHandleVerifier [0x0082A78F+2800735]\n\tGetHandleVerifier [0x0082456C+2775612]\n\tGetHandleVerifier [0x006151E0+616112]\n\t(No symbol) [0x00525F8C]\n\t(No symbol) [0x00522328]\n\t(No symbol) [0x0052240B]\n\t(No symbol) [0x00514FF7]\n\tBaseThreadInitThunk [0x767D7D59+25]\n\tRtlInitializeExceptionChain [0x77A5B79B+107]\n\tRtlClearBits [0x77A5B71F+191]\n"}}

oops. Something went really wrong. Wait a moment I need some more time to investigate this new issue.

Danp2 commented 1 year ago

I think the simplest way to perform this testing is to change the following line in _WD_ExecuteScript --

$sScript = __WD_EscapeString($sScript, $JSON_MLREFORMAT)

to

$sScript = __WD_EscapeString($sScript)

Then test all JS functionality to see if there are any negative effects.

mlipok commented 1 year ago

I ran into problems handling frames on Chrome , so as so far I`m not able to test this. Will open separate issue for handling frames.

mlipok commented 1 year ago

Could you test DemoFrames from wd_demo.au3 and say if it works fine for you on each browser ?

mlipok commented 1 year ago

I think the simplest way to perform this testing is to change the following line in _WD_ExecuteScript --

$sScript = __WD_EscapeString($sScript, $JSON_MLREFORMAT)

to

$sScript = __WD_EscapeString($sScript)

Then test all JS functionality to see if there are any negative effects.

After some more testing I think this should be fine.

I think that here: https://github.com/Danp2/au3WebDriver/commit/81154aba09b8d2f894170b276659ce99f26b8fd0

https://github.com/Danp2/au3WebDriver/blob/81154aba09b8d2f894170b276659ce99f26b8fd0/wd_core.au3#L1690-L1699

this following part was bad as it was no taking all cases:

Local $sRegEx = "([" & $_WD_ESCAPE_CHARS & "])"

and thus I wanted to add:

$sData = StringRegExpReplace($sData, '[\v\t]', '') ; Strip tabs and CR/LFs

but you propose to use:

$sData = Json_StringEncode($sData, $iOption) ; Escape JSON Strings

your proposal was covering also my request about fixing \t so currently $JSON_MLREFORMAT should be treated as useless and can be romoved from wd_core.au3 especially from: https://github.com/Danp2/au3WebDriver/blob/81154aba09b8d2f894170b276659ce99f26b8fd0/wd_core.au3#L1691-L1694

mlipok commented 1 year ago

If your results match mine, then I would suggest that we drop the usage of $JSON_MLREFORMAT

https://github.com/Danp2/au3WebDriver/pull/489

Danp2 / au3WebDriver