__WD_EscapeString() should be suplemented

mlipok commented 1 year ago

Bug report

Describe the bug

One o my client has very strong passwords generated and stored in tools lik Kepassa The problem is that this password contains many different kind of characters.

How to reproduce

just try to use _WD_SetElementValue() with one of this characters: :\" or even NewLine .

some valid JSON samples:

{"text":"|"}
{"text":"$"}
{"text":"*"}
{"text":";"}
{"text":"'"}

some invalid JSON samples:

{"text":"\"}
{"text":"""}
{"text":":"}

some fixed JSON samples:

{"text":"\\"}
{"text":"\""}
{"text":"\:"}

btw. also NEW LINE should be fixed, for example like this:

$sData = StringRegExpReplace($sData, '\R', '\\n')

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)

I get such results:

__WD_Post ==> Invalid argument [5] : HTTP status = 400
_WD_ElementAction ==> Invalid argument [5] : Parameters:   Command=VALUE   Option=<masked>
! $_VAR[1] -
 " ! @ # $ % ^ & * ( ) - _ + = { } [ ] ; : \ | , . < > / ? ~  '  @CR
 @CRLF
 @LF
 @TAB   END
! __WD_EscapeString($_VAR[0]) -
 \" ! @ # $ % ^ & * ( ) - _ + = { } [ ] ; : \\ | , . < > / ? ~  '  @CR
 @CRLF
 @LF
 @TAB   END
! END: UserTesting_EscapeString.au3
+ wd_demo.au3: Finished: UserFile

Expected behavior

setting values should support characters: :\" or even NewLine .

Screenshots

none

Additional context

better repro code will be in: https://github.com/Danp2/au3WebDriver/issues/466

https://www.tutorialspoint.com/json_simple/json_simple_escape_characters.htm

Backspace to be replaced with \b
Form feed to be replaced with \f
Newline to be replaced with \n
Carriage return to be replaced with \r
Tab to be replaced with \t
Double quote to be replaced with \"
Backslash to be replaced with \\

https://www.lambdatest.com/free-online-tools/json-escape

https://stackoverflow.com/a/27516892/5314940 Crockford's and ECMA's version

System under test

not related

mlipok commented 1 year ago

REPRO SCRIPT updated, and require: https://github.com/Danp2/au3WebDriver/pull/464

Danp2 commented 1 year ago

Some initial thoughts --

This function was originally created to escape quotation marks
Later it was updated to allow modifications via $_WD_ESCAPE_CHARS
Your PR seems to focus on JSON strings, which isn't the sole usage of this function
_WD_SelectFiles would need to be updated as it manually converts @LF to \n

mlipok commented 1 year ago

I think that we should create separate function __JSON_EscapeString() and __JS_EscapeString() $_WD_ESCAPE_CHARS should'nt be declared globally but within those function as separate cases.

Danp2 commented 1 year ago

That's one option. Another would be to expand __WD_EscapeString with a flag to control which conversion is performed.
However, it still isn't clear to me why we need an internal routine to escape JSON strings. Can you explain the use case?
The current implementation allows the ability to override the characters included in the conversion process. We should evaluate if this is still needed.

FYI, __WD_EscapeString is currently used by _WD_FindElement, _WD_ElementAction, _WD_ExecuteScript, and _WD_SelectFiles

mlipok commented 1 year ago

However, it still isn't clear to me why we need an internal routine to escape JSON strings. Can you explain the use case?

In the links (opening post) I only pointed to JSON requriments, so my question is Do JS requriments for Escaping strings are the same ?

Danp2 commented 1 year ago

I don't know the difference between JS and JSON requirements for escaping strings, and I'll leave it to you to research this.
I would appreciate it if you would answer the original question regarding your use case. Please don't repeat or refer to information you already posted as that isn't helpful IMO.
Keep in mind that this UDF is Webdriver focused, not JS or JSON, thus features should make sense from that POV.
If needed, the user can encode / escape the string prior to passing it to one of the WD functions, and the WD function needs to work correctly when the string has been "preformatted".

mlipok commented 1 year ago

first approach

; #INTERNAL_USE_ONLY# ===========================================================================================================
; Name ..........: __WD_EscapeJSONString
; Description ...: Escapes designated characters in string that will be used as value in JSON
; Syntax ........: __WD_EscapeJSONString($sData)
; Parameters ....: $sData               - a string value.
; Return values .: None
; Author ........: mLipok
; Modified ......:
; Remarks .......:
; Related .......:
; Link ..........: https://stackoverflow.com/a/19176131/5314940
; Example .......: No
; ===============================================================================================================================
Func __WD_EscapeJSONString($sData)
    $sEscaped = StringReplace($sData, '\', '\\')
    $sEscaped = StringReplace($sEscaped, '\', '\"')
    $sEscaped = StringReplace($sEscaped, @CR, '\r')
    $sEscaped = StringReplace($sEscaped, @LF, '\n')
    $sEscaped = StringReplace($sEscaped, @CRLF, '\r\n')
    $sEscaped = StringReplace($sEscaped, @TAB, '\t')
    $sEscaped = StringReplace($sEscaped, Chr(12), '\f')
    $sEscaped = StringReplace($sEscaped, Chr(8), '\b')
    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

; #INTERNAL_USE_ONLY# ===========================================================================================================
; Name ..........: __WD_EscapeJSString
; Description ...: Escapes designated characters in string to fullfill "WebDriver" requirements
; Syntax ........: __WD_EscapeJSString($sData)
; Parameters ....: $sData               - a string value.
; Return values .: None
; Author ........: mLipok
; Modified ......:
; Remarks .......:
; Related .......:
; Link ..........:
; Example .......: No
; ===============================================================================================================================
Func __WD_EscapeJSString($sData)
    Local $sEscaped = StringRegExpReplace($sData, '[\R\t]', "")
    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

__WD_EscapeJSONString should be used only to escape values not entire JSON structure

__WD_EscapeJSString is rather some kind of STRINGIFY from MultiLineJS to OneLineJS which should be posted to WebDriver via __WD_Post as $sCmd

__WD_EscapeJSString should be used here: https://github.com/Danp2/au3WebDriver/blob/744a945b7314db93a87d760d4c704a76a3b93821/wd_core.au3#L830-L843 I mean to use:

$sScript = __WD_EscapeJSString($sScript)

Instead:

$sScript = __WD_EscapeString($sScript)

btw. Could you prepare any UserTesting_JavaScript_Strings.au3 example which will fail when I remove :

$sScript = __WD_EscapeString($sScript)

or at least: Can you prepare any JavaScript example which will fail when I remove this mentioned line and then I create UserTesting_JavaScript_Strings.au3

?

mlipok commented 1 year ago

Already created UserTesting_JavaScript_Strings.au3

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

MsgBox($MB_OK + $MB_TOPMOST + $MB_ICONWARNING, "Open JS CONSOLE in browser", "")

ConsoleWrite("- Test 1:" & @CRLF)
_WD_ExecuteScript($sSession, StringReplace('console.log("#")', '#', "test#1")) ; console.log(("test#1")

ConsoleWrite("- Test 2:" & @CRLF)
_WD_ExecuteScript($sSession, StringReplace('console.log("#")', '#', "'test#2'" )) ; console.log("'test#2'")

ConsoleWrite("- Test 3:" & @CRLF)
_WD_ExecuteScript($sSession, StringReplace('console.log("#")', '"#"', "'test#3'")) ; console.log('test#3')

ConsoleWrite("! End: UserTesting_JavaScript_Strings.au3" & @CRLF)

also modified:

Func __WD_EscapeString($sData)
    Local $sRegEx = "([" & $_WD_ESCAPE_CHARS & "])"
    Local $sEscaped = StringRegExpReplace($sData, $sRegEx, "\\$1")
    If Not @compiled Then ConsoleWrite("> Escaped JavaScript = " & $sEscaped & @CRLF)
    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

and got this results:

! Start: UserTesting_JavaScript_Strings.au3
- Test 1:
> Escaped JavaScript = console.log(\"test#1\")
__WD_Post: URL=HTTP://127.0.0.1:4444/session/d1dea147-37f4-48fc-bd38-b8fb9a7e09b4/execute/sync; Data={"script":"console.log(\"test#1\")", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":null}
_WD_ExecuteScript ==> Success [0]
- Test 2:
> Escaped JavaScript = console.log(\"'test#2'\")
__WD_Post: URL=HTTP://127.0.0.1:4444/session/d1dea147-37f4-48fc-bd38-b8fb9a7e09b4/execute/sync; Data={"script":"console.log(\"'test#2'\")", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":null}
_WD_ExecuteScript ==> Success [0]
- Test 3:
> Escaped JavaScript = console.log('test#3')
__WD_Post: URL=HTTP://127.0.0.1:4444/session/d1dea147-37f4-48fc-bd38-b8fb9a7e09b4/execute/sync; Data={"script":"console.log('test#3')", "args":[]}
__WD_Post ==> Success [0] : HTTP status = 200 ResponseText={"value":null}
_WD_ExecuteScript ==> Success [0]
! End: UserTesting_JavaScript_Strings.au3

Will work/test later tonight

Danp2 commented 1 year ago

It still isn't clear to me why we need an internal routine to escape JSON strings. Can you explain the USE CASE?

I would appreciate it if you would answer the original question regarding your use case. Please don't repeat or refer to information you already posted as that isn't helpful IMO.

There's no point in proceeding until you answer the above to my satisfaction.

mlipok commented 1 year ago

However, it still isn't clear to me why we need an internal routine to escape JSON strings. Can you explain the use case?

In what sense INTERNAL ? You mean separate from JScript ?

mlipok commented 1 year ago

btw. As for now I test all my solutions with:

; #INTERNAL_USE_ONLY# ===========================================================================================================
; Name ..........: __WD_EscapeString
; Description ...: Escapes designated characters in string.
; Syntax ........: __WD_EscapeString($sData)
; Parameters ....: $sData - the string to be escaped
; Return values..: Escaped string.
; Author ........: Danp2
; Modified ......:
; Remarks .......:
; Related .......:
; Link ..........:
; Example .......: No
; ===============================================================================================================================
Func __WD_EscapeString_old($sData)
    Local $sRegEx = "([" & $_WD_ESCAPE_CHARS & "])"
    Local $sEscaped = StringRegExpReplace($sData, $sRegEx, "\\$1")
    If Not @compiled Then ConsoleWrite("> Escaped JavaScript = " & $sEscaped & @CRLF)
    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

; #INTERNAL_USE_ONLY# ===========================================================================================================
; Name ..........: __WD_EscapeJSONString
; Description ...: Escapes designated characters in string that will be used as value in JSON
; Syntax ........: __WD_EscapeJSONString($sData)
; Parameters ....: $sData               - a string value.
; Return values .: None
; Author ........: mLipok
; Modified ......:
; Remarks .......:
; Related .......:
; Link ..........: https://stackoverflow.com/a/19176131/5314940
; Example .......: No
; ===============================================================================================================================
Func __WD_EscapeString($sData)
    Local $sEscaped = StringReplace($sData, '\', '\\')
    $sEscaped = StringReplace($sEscaped, '"', '\"')
    $sEscaped = StringReplace($sEscaped, @CR, '\r')
    $sEscaped = StringReplace($sEscaped, @LF, '\n')
    $sEscaped = StringReplace($sEscaped, @CRLF, '\r\n')
    $sEscaped = StringReplace($sEscaped, @TAB, '\t')
    $sEscaped = StringReplace($sEscaped, Chr(12), '\f')
    $sEscaped = StringReplace($sEscaped, Chr(8), '\b')
    If Not @compiled Then ConsoleWrite("> Input $sData = " & $sData & @CRLF)
    If Not @compiled Then ConsoleWrite("> Escaped JavaScript = " & $sEscaped & @CRLF)
    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

I just start using my __WD_EscapeString to cover all cases that should be taken into consideration.

Danp2 commented 1 year ago

However, it still isn't clear to me why we need an internal routine to escape JSON strings. Can you explain the use case?

In what sense INTERNAL ? You mean separate from JScript ?

Internal meaning "internal use only". I want to know how your proposed function would be used by the existing _WD functions.

As for now I test all my solutions with

I don't believe that your proposal would pass this test --

If needed, the user can encode / escape the string prior to passing it to one of the WD functions, and the WD function needs to work correctly when the string has been "preformatted".

Danp2 commented 1 year ago

Something else to consider is that the JSON UDF already contains functions for encoding and decoding JSON strings --

#include "wd_core.au3"

$sValue = 'testing' & @TAB & '12345' & @crlf
$sResult = Json_StringEncode($sValue)
ConsoleWrite('@@ Debug(' & @ScriptLineNumber & ') :  Json_StringEncode = ' &  $sResult & @CRLF & '>Error code: ' & @error & @CRLF)
ConsoleWrite('@@ Debug(' & @ScriptLineNumber & ') :  Json_StringDecode = ' &  Json_StringDecode($sResult) & @CRLF & '>Error code: ' & @error & @CRLF)

Results

 @@ Debug(6) :  Json_StringEncode = testing\t12345\r\n
    >Error code: 0
 @@ Debug(7) :  Json_StringDecode = testing 12345
    
    >Error code: 0

Danp2 commented 1 year ago

Theoretically we could eliminate $_WD_ESCAPE_CHARS and change __WD_EscapeString so that it is a wrapper for Json_StringEncode

Edit: For flexibility, we could have $_WD_ESCAPE_OPTIONS, which would be passed as a parameter to Json_StringEncode

mlipok commented 1 year ago

Awesome. Must do more test

mlipok commented 1 year ago

so we should use it like :

Func __WD_EscapeString($sData)
    Local $sEscaped2 = Json_StringEncode($sData)
    Return SetError($_WD_ERROR_Success, 0, $sEscaped2)
EndFunc   ;==>__WD_EscapeString

test in progress....

mlipok commented 1 year ago

About JavaScript Parsing I was thinking about such example code:

    Local $s_JavaScript = _
            "   let style = document.createElement('style');                                    " & @CRLF & _
            "   style.id = 'WEBDRIVER_CSSStyles-theme" & $IDX_Frame & "';                           " & @CRLF & _
            "   style.innerHTML = `" & $s_CSS_Parser & "`;                                      " & @CRLF & _
            "   document.documentElement.insertBefore(style, document.body.nextSibling);        " & @CRLF & _
            ""
    ConsoleWrite($s_JavaScript & @CRLF)
    Local $v_Result = _WD_ExecuteScript($sSession, StringRegExpReplace($s_JavaScript, '[\R\t]',''))

IMO this part:

StringRegExpReplace($s_JavaScript, '[\R\t]','')

should be done automatically/internally by UDF.

EDIT: btw. If I'm right it is called linearization

mlipok commented 1 year ago

So in _WD_ExecuteScript https://github.com/Danp2/au3WebDriver/blob/51934eccedffc51f9fe3c164af670354d61e2a48/wd_core.au3#L830-L844

I changed:

    If IsString($vSubNode) Then
;~      $sScript = __WD_EscapeString($sScript)
        $sScript = __WD_EscapeString(StringRegExpReplace($sScript, '[\R\t]',''))

And I keep testing

Danp2 commented 1 year ago

Edit: For flexibility, we could have $_WD_ESCAPE_OPTIONS, which would be passed as a parameter to Json_StringEncode

We could avoid adding this additional global by making it an optional parameter to __WD_EscapeString --

Func __WD_EscapeString($sData, $iOption = 0)
    Local $sEscaped = Json_StringEncode($sData, $iOption)
    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

Danp2 commented 1 year ago

Instead of changing _WD_ExecuteScript, I would propose enhancing __WD_EscapeString further --

Global Const $JSON_MLREFORMAT = 1024 

Func __WD_EscapeString($sData, $iOption = 0)
    Local $sEscaped = Json_StringEncode($sData, $iOption)

    If BitAND($iOption, $JSON_MLREFORMAT) Then
        $sEscaped = StringRegExpReplace($sEscaped, '[\R\t]','')
    Endif

    Return SetError($_WD_ERROR_Success, 0, $sEscaped)
EndFunc   ;==>__WD_EscapeString

And the __WD_EscapeString line in _WD_ExecuteScript would change to

$sScript = __WD_EscapeString($sScript, $JSON_MLREFORMAT)

mlipok commented 1 year ago

rather:

Func __WD_EscapeString($sData, $iOption = 0)
    If BitAND($iOption, $JSON_MLREFORMAT) Then ; JavaScript linearization first
        $sData = StringRegExpReplace($sData, '[\R\t]','')
    Endif

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

    Return SetError($_WD_ERROR_Success, 0, $sData)
EndFunc   ;==>__WD_EscapeString

JavaScript must be done before JSON as JavaScirpt string will be encapsulated into the JSON

If you firstly Escape JSON Strings then, after that, new line or tab will be already Escaped so using JavaScript linearization first will be useless.

mlipok commented 1 year ago

small fix \R\t not [\R\t]

Func __WD_EscapeString($sData, $iOption = 0)
    If BitAND($iOption, $JSON_MLREFORMAT) Then ; JavaScript linearization first
        $sData = StringRegExpReplace($sData, '\R\t','')
    Endif

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

    Return SetError($_WD_ERROR_Success, 0, $sData)
EndFunc   ;==>__WD_EscapeString

mlipok commented 1 year ago

something is wrong with my regexp

Danp2 commented 1 year ago

small fix \R\t not [\R\t]

You need the brackets so that it is treated as a set of characters instead of the combination of \R\t.

FWIW, my testing with [\v\t] appears to be working correctly.

Edit: This also works -- (?m)[\R\t]

mlipok commented 1 year ago

Moving discussion to PR

Danp2 commented 1 year ago

IMO, it's helpful to keep some discussion / questions here so that the PR stays less cluttered.

Danp2 / au3WebDriver