Danp2 / au3WebDriver

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

Functions should be checked for escaping strings #470

Closed mlipok closed 1 year ago

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()

Originally posted by @mlipok in https://github.com/Danp2/au3WebDriver/issues/469#issuecomment-1537521321

Danp2 commented 1 year ago

@mlipok Please share your thoughts on this. Why do you think further encoding / decoding is needed here?

Shouldn't the user be responsible for encoding / decoding the string outside of _WD_Storage when needed?

Danp2 commented 1 year ago

Also, see https://github.com/Danp2/au3WebDriver/issues/466#issuecomment-1523859474, which could also be the solution here.

mlipok commented 1 year ago

_WD_GetElementByRegEx() https://github.com/Danp2/au3WebDriver/blob/81154aba09b8d2f894170b276659ce99f26b8fd0/wd_helper.au3#L3059-L3075

@TheDcoder I wonder is this StringReplace still needed ? https://github.com/Danp2/au3WebDriver/blob/81154aba09b8d2f894170b276659ce99f26b8fd0/wd_helper.au3#L3073

I mean after we changed how __WD_EscapeString is used: https://github.com/Danp2/au3WebDriver/pull/469/files

_WD_ExecuteScript() usage image

and __WD_EscapeString() image

TheDcoder commented 1 year ago

@mlipok Not sure really, the change is complicated and I have forgotten the context. Best if you trace it out and see if it's still required :smile:

Danp2 commented 1 year ago

https://github.com/Danp2/au3WebDriver/blob/81154aba09b8d2f894170b276659ce99f26b8fd0/wd_helper.au3#L3073 Based on my testing, this line is no longer required.

mlipok commented 1 year ago

Any testing snippet ?

Danp2 commented 1 year ago

https://github.com/Danp2/au3WebDriver/pull/301#issuecomment-1137863798

😆

mlipok commented 1 year ago

my memory doesn't go that far ;)

Danp2 commented 1 year ago

Regarding _WD_Storage, I would suggest that this be handled in the same way as I recommend in https://github.com/Danp2/au3WebDriver/issues/466#issuecomment-1544627814