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 malformed request, release mega-20191003 #2681

Closed
risros opened this issue Oct 24, 2019 · 20 comments · Fixed by #2667
Closed

sendtohttp malformed request, release mega-20191003 #2681

risros opened this issue Oct 24, 2019 · 20 comments · Fixed by #2667
Labels
Category: Rules Related to the rule engine Type: Bug Considered a bug

Comments

@risros
Copy link

risros commented Oct 24, 2019

If you use variables inside the SentToHTTP command, the request is somehow malformed (server access log shows no query parameters in request) and PHP server doesn't parse the values:
Error: SendToHTTP 192.168.1.1,80,/report.php?val=[Relay#status]
If static values are used, it works. PHP can parse the value of "val":
No error: SendToHTTP 192.168.1.1,80,/report.php?val=1

@giig1967g
Copy link
Contributor

I am not sure, but it can be because the "=" before the "[" is interpreted as a formula delimiter.
Try to use
SendToHTTP 192.168.1.1,80,/report.php?val==[Relay#status]

@TD-er
Copy link
Member

TD-er commented Oct 24, 2019

Related issues:

@giig1967g I think you make a good point here about the '=' messing with the parsing.
Only thing I don't understand is why it is working then as a parameter for logitem and not as an URL in sendToHttp ?

@FxAlex
Copy link

FxAlex commented Oct 24, 2019

due to brackets after '='

@TD-er
Copy link
Member

TD-er commented Oct 24, 2019

due to brackets after '='

As you can see here in my reply on the forum I also had the bracket after the = and it still shows up in the Apache logs just like I expect it to be.

@TD-er
Copy link
Member

TD-er commented Oct 24, 2019

I just changed the handling of logentry command to do exactly the same as the path decoding in the sendtohttp command.

Serial log:

35426 : ACT  : LogEntry,/json.htm?type=command&param=udevice&idx=123456&nvalue=31
35432 : Command: logentry
35432 : /json.htm?type=command&param=udevice&idx=123456&nvalue=31
35444 : ACT  : sendtohttp 192.168.1.4,80,/json.htm?type=command&param=udevice&idx=123456&nvalue=31
35450 : Command: sendtohttp
35451 : SendToHTTP: Host: 192.168.1.4 port: 80
35461 : GET /json.htm?type=command&param=udevice&idx=123456&nvalue=31          HTTP/1.1
Host: 192.168.1.4
User-Agent: ESP Easy/20104/Oct 25 2019 00:00:05
Connection: close


35463 : HTTP : Command_HTTP_SendToHTTP written to client (170/170)
35463 : HTTP : Command_HTTP_SendToHTTP closing connection

And my rules:

On GPIO#2 Do
LogEntry,/json.htm?type=command&param=udevice&idx=123456&nvalue=[rssi#rssi]
sendtohttp 192.168.1.4,80,/json.htm?type=command&param=udevice&idx=123456&nvalue=[rssi#rssi]
EndOn

Entry in the Apache log:

access.log.1:192.168.1.75 - - [25/Oct/2019:00:06:49 +0200] "GET /json.htm?type=command&param=udevice&idx=123456&nvalue=31 HTTP/1.1" 400 0 "-" "-"

@TD-er
Copy link
Member

TD-er commented Oct 24, 2019

Hmm, I noticed wrapping the argument with quotes does not strip them.
So apparently the first and last character of the parsed string is not a quote character.
Adding a trim command before stripping the quotes does remove them.
So I guess the sendtohttp does add a space at the end?

Can you test with this small change in StringConverter.ino :

String parseString(const String& string, byte indexFind, bool toEndOfString, bool toLowerCase) {
  int startpos = 0;

  if (indexFind > 0) {
    startpos = getParamStartPos(string, indexFind);

    if (startpos < 0) {
      return "";
    }
  }
  const int endpos = getParamStartPos(string, indexFind + 1);
  String    result;

  if (toEndOfString || (endpos <= 0)) {
    result = string.substring(startpos);
  } else {
    result = string.substring(startpos, endpos - 1);
  }

  if (toLowerCase) {
    result.toLowerCase();
  }
  result.trim();
  return stripQuotes(result);
}

The added part is the result.trim();

See the differences in the Apache logs:

access.log:192.168.1.75 - - [25/Oct/2019:01:02:48 +0200] "GET /json.htm?type=command&param=udevice&idx=123456&nvalue=-50 HTTP/1.1" 400 0 "-" "-"
access.log:192.168.1.75 - - [25/Oct/2019:01:03:45 +0200] "GET /json.htm?type=command&param=udevice&idx=123456&nvalue=31 HTTP/1.1" 404 453 "-" "ESP Easy/20104/Oct 25 2019 00:00:05"

TD-er added a commit to TD-er/ESPEasy that referenced this issue Oct 24, 2019
…#2660)

The changed rules parsing may have added some spaces in replaced variables.
Adding a trim() call to the parseString does remove any existing or added spaces in the parameter.
@risros
Copy link
Author

risros commented Oct 25, 2019

Thanks for a quick response! Is this commit going to be part of the next automated build?

@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

Thanks for a quick response! Is this commit going to be part of the next automated build?

As soon as it has been verified :)
I made a test build which also includes a lot of other changes.
Can you please test it to see if sendtohttp now works like it should and also if I broke something else?

@TD-er TD-er added Category: Rules Related to the rule engine Type: Bug Considered a bug labels Oct 25, 2019
@clumsy-stefan
Copy link
Contributor

I also have PR #2667 included in all my builds and no issues found so far (just FYI @TD-er)..

@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

I also have PR #2667 included in all my builds and no issues found so far (just FYI @TD-er)..

What part of "test build" and "please let me know if I broke something" did you miss ;)
Thanks for being so brave :)

(I did test it on my node here of course, but still a LOT has changed)

@clumsy-stefan
Copy link
Contributor

Sorry, I don't understand what you're trying to tell me... I just thought I'll tell you that I run these changes on all my nodes and found no issues with rules etc. So you know that someone (me) is also testing these changes.....

@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

Oh I really do like the feedback a lot. (really a lot)
It is just that I think it is a bit daring to install it directly on all nodes.

@clumsy-stefan
Copy link
Contributor

Oh, I love daring things :)

An the chances of finding an issue on wider installations is much higher! Besides, of I do find something I also try to explain and of possible provide a fix for what I found so your work and precious time are saved as much as possible!! Also I'd never expect any kind of (quick) support of something fails! I can always go back to older versions if it's critical to me...

BTW: I did find one small thing: some of the include paths are wrong (Plugin.h) and therefore won't compile in Arduino without changes...

@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

BTW: I did find one small thing: some of the include paths are wrong (Plugin.h) and therefore won't compile in Arduino without changes...

In what file?
I can fix it right now.

@clumsy-stefan
Copy link
Contributor

clumsy-stefan commented Oct 25, 2019

5 files where

"../../src/src/Globals/Plugins.h"

shoud be

"../Globals/Plugins.h"

in

src/Datastructs/Caches.h
src/Datastructs/ControllerSettingsStruct.h
src/Datastructs/ESPEasy_EventStruct.h
src/Datastructs/PortStatusStruct.h
src/Datastructs/SettingsStruct.h

@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

Great, thanks.
Will fix it immediately.

Would be nice if PlatformIO could be set more strict in parsing includes like ArduinoIDE does.

@risros
Copy link
Author

risros commented Oct 25, 2019

HI, TD-er
It works! I can now use variables in sendToHTTP and server can parse query parameters correctly.
This script now works, where previously it didnt:

On System#Boot do //When the ESP boots, do
timerSet,1,1 //Set Timer 1 to trigger in 1s
endon

On Rules#Timer=1 do //When Timer1 expires, do
SendToHTTP 192.168.1.1,80,/report.php?dev=Sonoff2&sw=1&val=[Rele#state]
timerSet,1,1
endon

Thanks!!!
p.s. Tested with mega-20191016-11-PR_2667 from the link from one of the previous comments.

@TD-er
Copy link
Member

TD-er commented Oct 26, 2019

Can you also test this build ? (just to be sure it is still working)

@risros
Copy link
Author

risros commented Oct 27, 2019

Tested, it works!

@TD-er
Copy link
Member

TD-er commented Oct 27, 2019

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Rules Related to the rule engine Type: Bug Considered a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants