-
Notifications
You must be signed in to change notification settings - Fork 23
WinHttp.WinHttpRequest.5.1 usage #512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Hi @mlipok , since you use the same code block ... If @error Then
Local $oHTTP = ObjCreate("WinHttp.WinHttpRequest.5.1")
$oHTTP.Open("GET", $sURL, False)
$oHTTP.SetRequestHeader("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:121.0) Gecko/20100101 Firefox/121.0")
$oHTTP.Send("")
$sName = $oHTTP.ResponseBody
EndIf ... three times (for Best regards |
I expected such request. |
No pressure @mlipok , thanks for your stable contribution actions on this project 👌 . |
Cool, thanks @mlipok . I try to test it in the next day(s). Because I believe, the |
; #FUNCTION# ==================================================================================================================== | ||
; Name ..........: _WD_DownloadAsBinary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the function header should be ; #INTERNAL_USE_ONLY# ======
and the function should be __WD_DownloadAsBinary
instead of _WD_DownloadAsBinary
. Actually it does not matter, but it would fulfill the consistency 😇 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have any reason to change _WD_DownloadAsBinary
into internal usage only then you should try to apply the same reason to _WD_DownloadFile
and see if this reason also makes sense here.
If you say that _WD_DownloadFile
save data to file I would say that the destination does not matter here as I always try to avoid writing data to files on disk. Instead, I process the data in memory and write to database.
This is because of security reason (i.e. GDPR law) - I simply try to minimize the number of places where data leaves a trace (files on disc).
Summary: if any user of this UDF needs to use the function for download and writing data to a file, then the function of writing data to memory without writing it to disk is equally necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood and fair enough @mlipok 👍 .
Then at least the README.md file also has to be adjusted, right?
I mean the additional function should be described in the README like the other functions too.
Is this usually made by @Danp2 ? I believe he do a "Prep release" commit which will contain such things, right? Or do we as PR authors also adjust these files 🤔 ?
Best regards
Sven
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Danp2 can you look here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't yet read through the past discussions on this PR, but I do have a few questions to add --
- Do we need to support both InetRead and WinHttp downloads?
- Is there functionality from WinHttp.au3 that could be used here instead of WinHttp.WinHttpRequest.5.1?
This is actually a really good question @Danp2. Interesting. |
Do not know. |
I think yes. |
Do you mean to stop using |
maybe from here: also some of my old attempts: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to stop using InetRead in favor of WinHTTP functions ?
Yes, that is what I was wanting you to consider.
Pull request
Proposed changes
Today I hit a problem when trying to update geckodriver where InetRead() returns with error
Checklist
Types of changes
What is the current behavior?
Sometimes InetRead and InetGet can return error thus there is no way to update driver with this udf.
What is the new behavior?
when
InetRead
returns error thenWinHttp.WinHttpRequest.5.1
is used.Influences and relationship to other functionality
none
Additional context
https://www.autoitscript.com/forum/topic/209621-inetget-alernative/
https://www.autoitscript.com/forum/topic/209610-inetget-dont-download-some-pages-since-ie11-is-no-longer-supported-by-some-sites
https://www.autoitscript.com/forum/topic/209472-download-problem-with-inetget/
System under test
FireFox