setServer(domain, ...) dangerous/dangling string pointer?

[binary1230]

I originally reported this in #751 but, I'm not sure it's the same issue and, it's potentially serious enough to move into its own issue. it might also be related to #794 and other heap-corruption crashes, not sure.

In here:

PubSubClient& PubSubClient::setServer(const char * domain, uint16_t port) {
    this->domain = domain;
    this->port = port;
    return *this;
}

the problem is with 'domain':

this->domain = domain;

it's a member variable inside class PubSubClient:

const char* domain;

The problem here is, it's just directly storing the char* pointer that was passed in. if the caller's string goes out of scope, then PubSubClient is holding onto a dangling pointer.

I noticed this issue in my code when the string I was using was re-allocated and 'domain' pointed to the string my client ID was using. (I happened to catch this because I was experiencing repeated connection failures and was using TinyGSM with verbose logging enabled. I noticed it was opening a TCP connection to the string I was using as a client ID, which in my case was a mac address. it amazingly wasn't crashing)

The solution is, PubSubClient needs to not store the string pointer directly, but have its own string buffer (or use a string class) and copy the string from the pointer passed in.

The simplest solution is probably to not store 'domain' as char*, but as a 'string' or 'String', which will automatically copy and do the right thing here.

If I'm not too tired right now and reading this right, this is a pretty serious bug.