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

Invalid user/password are not reported #16

Open
dmak opened this issue Feb 17, 2022 · 3 comments
Open

Invalid user/password are not reported #16

dmak opened this issue Feb 17, 2022 · 3 comments

Comments

@dmak
Copy link

dmak commented Feb 17, 2022

If MQTT user/password are incorrectly configured the library (v5.28.1) anyway tries to send the message ignoring the server error. Server log (Mosquitto v1.5.7):

1645136144: Socket error on client <unknown>, disconnecting.
1645136275: New connection from 127.0.0.1 on port 1883.

Expected that there is a mechanism to handle this issue e.g. $mqtt->publish('info', 'OK') or die ...
Code:

my $mqtt = Net::MQTT::Simple->new('localhost');
$ENV{MQTT_SIMPLE_ALLOW_INSECURE_LOGIN} = 1; # disabled secured connection check as we are connecting to localhost
$mqtt->login('unknown', 'invalid');
$mqtt->publish('info', 'OK');
$mqtt->disconnect();
@Juerd
Copy link
Owner

Juerd commented Feb 17, 2022 via email

@dmak
Copy link
Author

dmak commented Feb 18, 2022

I understand this. Maybe you could introduce

$mqtt->connect() or die ...;

so that the client at least can have a possibility to check initial connect? Basically there is such function, but it is not exposed as official API. Indeed, the situation may change over time i.e. the user may be removed at the server etc, but I think the ability to check the first login would help a lot to resolve trivial authentication issues.
Also I think functions like publish(), tick() should return an error if connection fails. As far as I can see the client has no way to check if the message was really delivered. That's why I would vote for

$mqtt->publish('info', 'OK') or die ...

as well. Alternatively, if you don't like the die bloat, the library could work in the mode when it reports the errors to stderr and/or remembers the last error in internal variable:

$mqtt->publish('info', 'OK');
...
print $mqtt->error_message() if $mqtt->has_failed();

Also what I see from tcpdump, the library ignores the authentication error and sends the message to the server. So it just pushes everything to the server without looking around.

@mrdvt92
Copy link

mrdvt92 commented May 12, 2023

I wrote a wrapper to die on bad credintials. But, it uses the _connect internal method so I don't expect it to be supported forever.

our $MQTT_CLASS     = 'Net::MQTT::Simple';
our $MQTT_CLASS_SSL = 'Net::MQTT::Simple::SSL';
 
sub mqtt {
  my $self = shift;
  unless ($self->{'mqtt'}) {
    my $host        = join(':', $self->host, $self->port);
    my $class       = $self->secure ? $MQTT_CLASS_SSL : $MQTT_CLASS;
    $self->{'mqtt'} = $class->new($host);
    $self->{'mqtt'}->login($self->user, $self->password) if $self->user;
    {
      #Test connection with die instead of warn
      local $SIG{__WARN__} = sub {
                                  my $msg = shift;
                                  chomp $msg;
                                  my (undef, undef, $text) = split /\s*:\s*/, $msg;
                                  die(qq{MQTT Error: connection: $text\n});
                                 };
      $self->{'mqtt'}->_connect; #why private method?
    }
  }
  return $self->{'mqtt'};
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants