Crash after saving MQTT hostname with trailing spaces

[maurice1]

Using ahoy 0.5.15 I've set the MQTT hostname on the setup page to

broker.emqx.io                                                                        

Please note the trailing white spaces. The string consists of 86 characters in total.

After saving, the controller obviously crashed. I wasn't able to get a HTTP connection. Even after disconnecting the power source and connecting it again the controller doesn't even boot up anymore :-(

[maurice1]

After reflashing, the serial console outputs:

I: connect to network 'YOUR_WIFI_SSID' ...
16:01:43.658 > ...........
16:01:44.758 > ........................................
16:01:48.763 > ........................................
16:01:52.766 > ........................................
16:01:57.673 > W: your settings are not valid! check [IP]/setup
16:01:57.675 > I: 
16:01:57.675 > ---------
16:01:57.678 > AP MODE
16:01:57.678 > SSID: AHOY-DTU
16:01:57.678 > PWD: esp_8266
16:01:57.681 > Active for: 60 seconds
16:01:57.681 > ---------
16:01:57.683 > 
16:01:58.574 > I: RF24 Amp Pwr: RF24_PA_I: HIGH
16:01:58.576 > I: Radio Config:
16:01:58.576 > SPI Frequency            = 1 Mhz
16:01:58.579 > Channel                  = 3 (~ 2403 MHz)
16:01:58.582 > Model                    = nRF24L01+
16:01:58.582 > RF Data Rate             = 250 KBPS
16:01:58.585 > RF Power Amplifier       = PA_HIGH
16:01:58.587 > RF Low Noise Amplifier   = Enabled
16:01:58.590 > CRC Length               = 16 bits
16:01:58.593 > Address Length           = 5 bytes
16:01:58.596 > Static Payload Length    = 32 bytes
16:01:58.598 > Auto Retry Delay = 250 microseconds
16:01:58.601 > Auto Retry Attempts      = 0 maximum
16:01:58.604 > Packets lost on
16:01:58.607 >     current channel      = 0
16:01:58.607 > Retry attempts made for
16:01:58.610 >     last transmission    = 0
16:01:58.612 > Multicast                = Disabled
16:01:58.615 > Custom ACK Payload       = Disabled
16:01:58.618 > Dynamic Payloads = Enabled
16:01:58.618 > Auto Acknowledgment      = Disabled
16:01:58.622 > Primary Mode             = RX
16:01:58.624 > TX address               = 0xe7e7e7e7e7
16:01:58.627 > pipe 0 (closed) bound    = 0xe7e7e7e7e7
16:01:58.630 > pipe 1 ( open ) bound    = 0x1234567801
16:01:58.633 > pipe 2 (closed) bound    = 0xc3
16:01:58.635 > pipe 3 (closed) bound    = 0xc4
16:01:58.638 > pipe 4 (closed) bound    = 0xc5
16:01:58.641 > pipe 5 (closed) bound    = 0xc6

This suggests at least the wifi settings have been reset.

[stefan123t]

Hi @maurice1 is this issue about the fact that you were able to supply a mqtt broker hostname with 86 chars which is more than the allowed 63 chars for our field and also includes unallowed space chars in the hostname ?

@aschiffler we could add validation functions / regex [a-z0-9A-Z\-\.]{63} to verify mqtt hostname length and validity before saving it to the settings.

Is there anything we need to take into account for acutes allowed in domain names nowadays ?

See here for prevalidating in the browser using a HTML5 pattern: https://stackoverflow.com/questions/58210633/using-html5-input-pattern-to-enforce-valid-hostname

And here is a more complete pttern and discussion for hostnames and ip addresses: ^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])(\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9]))*$

https://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address

[maurice1]

It's more about the fact, that invalid values are allowed and result in a somehow broken controller upon saving. I like your idea to apply HTML5 validation. Although it's not server side validation this seems much simpler to implement and might be sufficient to solve a lot of problems with the setup page.

HTML5 validation or similar restrictions could be easily added to other setup fields like the inverter serial number (pattern or maxlength), max module power, intervals and retries (numeric up/down) and NTP server/port fields.

[stefan123t]

Yeah thought about Server side validation but then the regex example came up and this seems a lot easier than doing the heavy lifting of regex parsing on the ESP side. Despite that we should implement a least effort length validation indeed on ESP side

[fosndr]

I entered as Broker / Server IP following : b10a427b13604b1ea5a595b2258baac3.s1.eu.hivemq.cloud this is shorten after saving to b10a427b13604b1ea5a595b2258baac and after clearing it and resaving the string b10a427b13604b1ea5a595b2258baac appears again

[aschiffler]

Mqtt broker connections over websockets are not supported.

[maurice1]

aschiffler closed this as completed 2 days ago

@aschiffler Is this issue closed because it's not considered a bug?

[aschiffler]

two things:

1. yes it is a bug because of the missing sanity checks in the web interface ("trailing spaces") --> label "bug" --> is on todo list to enhance the web UI UX
2. transport over websockets is not supported (yet) and is actual not planed. --> close this issue.

[maurice1]

I suggest to leave issues open until they are fixed or resolved otherwise. Please correct me if I'm wrong, but I don't see how this issue is about websockets. Actually, it's about input string validation (1) and the fact, that the flash memory gets corrupted somehow due to missing length checks, which breaks the controller software (2).

[stefan123t]

Needs missing length checks as observed by @maurice1 Regex on client side is a plus but not required. Transport over websocket is not planned and not part of this issue

[lumapu]

@maurice1 can you test with latest development build again?

[maurice1]

@lumapu yes, will test it asap :)

[maurice1]

@lumapu Tested with current development02 branch and it works just fine. Thank you, lumapu!

[lumapu]

great to hear! can you close this issue?

[maurice1]

fixed :+1:

[stefan123t]

@lumapu @aschiffler neben wontfix wäre ein implemented (in dev) und fixed label toll.

[lumapu]

@stefan123t sind angelegt