Skip to content
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

[SendToHttp] Do not set HTTP authentication when no user/pass set #4205

Merged
merged 13 commits into from Aug 25, 2022

Conversation

TD-er
Copy link
Member

@TD-er TD-er commented Aug 16, 2022

A number of setups may report HTP error 401 since the 20220809 build.
This fix should not add an auth header if no user/pass is set.

Fixes: #4203
Fixes: #4208

A number of setups may report HTP error 401 since the 20220809 build.
This fix should not add an auth header if no user/pass is set.
@tonhuisman
Copy link
Contributor

This should fix #4203 , @fly74 can you check if it now again works as expected, using the Github Actions build?

@fly74
Copy link

fly74 commented Aug 16, 2022

@tonhuisman Sure but what is a action build?

@tonhuisman
Copy link
Contributor

On top you see a tab called Actions, there you see a list of recently run Actions, one of them being the build of this Pull Request:
image

Clicking the title will bring you to the overview page for that Action, that has a link to the results:
image

In that Binaries zip file, you will find a (bunch of other) zip file with the build you normally use. Extract that and you will find the .bin files, in a subfolder.

@fly74
Copy link

fly74 commented Aug 16, 2022

Testet with ESP_Easy_mega_20220816_normal_ESP8266_4M1M, same problem, not fixed.

@TD-er
Copy link
Member Author

TD-er commented Aug 16, 2022

Can you give an example of what you're trying to send and perhaps to what kind of webserver?

@fly74
Copy link

fly74 commented Aug 17, 2022

@TD-er

242670 : Info : EVENT: MS#State=1
242678 : Debug : timeStringToSeconds: "1" --> invalid
242678 : Debug : timeStringToSeconds: "1" --> invalid
242681 : Debug : conditionMatch: "1=1" --> "1" = "1" --> true (1 = 1)
242681 : Debug : conditionMatchExtended: true "1=1"
242682 : Debug : Lev.1: [if true]=true
242684 : Info : ACT : SendToHTTP,esphost2,80,"control?cmd=event,pon"
242686 : Debug : Command: SendToHTTP
242687 : Debug : SendToHTTP,esphost2,80,"control?cmd=event,pon"
242688 : Debug : Par1: 846230272 Par2: 80 Par3: 1073710592 Par4: 0 Par5: 0
242691 : Debug : SendToHTTP: Host: esphost2 port: 80 path: control?cmd=event,pon
242738 : Info : HTTP : Start Digest Authorization for esphost2
242806 : Error : HTTP : SendToHTTP esphost2 GET... failed HTTP code: 401
242812 : Info : ACT : SendToHTTP,esphost1,80,"control?cmd=event,pon"
242815 : Debug : Command: SendToHTTP
242815 : Debug : SendToHTTP,esphost1,80,"control?cmd=event,pon"
242816 : Debug : Par1: 829453056 Par2: 80 Par3: 1073709568 Par4: 0 Par5: 0
242819 : Debug : WiFi : Set TX power to 0dBm sensitivity: -69dBm RSSI: -47dBm
242820 : Debug : SendToHTTP: Host: esphost1 port: 80 path: control?cmd=event,pon
247062 : Error : HTTP : SendToHTTP esphost1 GET... failed HTTP code: -1 connection failed
247069 : Debug : Lev.1: [else]=false
247075 : Debug : EVENT: MS#State=1 Processing time:4405 milliSeconds
247079 : Info : EVENT: http#esphost2=401
247081 : Debug : EVENT: http#esphost2=401 Processing time:2 milliSeconds
247083 : Debug : LoopStats: shortestLoop: 57 longestLoop: 4568311 avgLoopDuration: 213.17 loopCounterMax: 526315 loopCounterLast: 152269

I hope I used the action build correctly, the esphost1host is not running and the connection failure for this is correct.

Screenshot 2022-08-17 065117

@ghtester
Copy link

FYI I have just quickly tested the Generic HTTP & Generic HTTP Advanced controllers on latest Custom ESP firmware (compiled yesterday without PR4205) with Port Listener app. as a target.
The strange thing is that despite I have the Check Reply: option set to Ignore Acknowledgement, I am getting the connection errors due to timeout (which is set to 200ms and the connection is local):

5503126: HTTP : C008 192.168.44.34 GET... failed HTTP code: -11 read Timeout
85503127: HTTP : C008 connection failed (521/0)
85503129: EVENT: http#192.168.44.34=-11
85503531: HTTP : C008 192.168.44.34 GET... failed HTTP code: -11 read Timeout
85503532: HTTP : C008 connection failed (522/0)
85503536: EVENT: http#192.168.44.34=-11
85503813: HTTP : C008 192.168.44.34 GET... failed HTTP code: -11 read Timeout
85503814: HTTP : C008 connection failed (523/0)
85503817: EVENT: http#192.168.44.34=-11

And the target accepted the connection repeatedly (as it is just listener, without any reply to initiator):

Client connected
GET /demo.php?name=ESP_04&task=Voltage&valuename=Analog&value=39 HTTP/1.1
Host: 192.168.44.34
User-Agent: ESP Easy/20116/Aug 16 2022 14:06:46
Accept-Encoding: identity;q=1,chunked;q=0.1,*;q=0
Authorization: Basic Og==
Connection: keep-alive
Content-Length: 0

Client disconnected

@TD-er
Copy link
Member Author

TD-er commented Aug 23, 2022

I also fixed some issue where (especially on ESP32) a second call to some HTTP server may fail until some (long) timeout or another call to a different host was made.
However there is still something I need to look into as a number of HTTP calls in short intervals may still lead to missed calls even though the call will be registered as a success (or at least the same HTTP code as the previous call)

@ghtester can you test this PR?

@ghtester
Copy link

Thanks for your work, as already reported in #4208 thread, unfortunately I still see the Timeout issues.

WiFiClient has a setTimeout function which expects seconds.
The parent class, Client, does expect msec.
So by casting to its parent and then calling setTimeout, we can again set the timeout in msec.
Otherwise it is set to 4000 msec.
For servers that won't send an ack, you would otherwise always hit this 4000 msec timeout, which is way too long.
The set timeout is now considered a minimum timeout.
If it isn't enough, it will dynamically rise until it is enough.

Typical used timeout is 3x the last encountered duration, with a minimum set in the controller settings.
This setting is set for a reason, like the server may never reply/ack.
If set, the duration will always match the set timeout and thus increase over time.
@TD-er
Copy link
Member Author

TD-er commented Aug 24, 2022

For the controllers, I made the timeout dynamic (unless it is set to ignore ack).
The timeout will now be 3x the last actual duration, with the set timeout as minimum.
If it is set to ignore the server ack. this will obviously not apply.

Still thinking whether I should make this dynamic behavior optional.

@TD-er TD-er merged commit 275a9f1 into letscontrolit:mega Aug 25, 2022
@TD-er TD-er deleted the bugfix/sendToHttp_401_error branch August 25, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants