Skip to content

Unable to set vhost with amqp DSN #696

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

Closed
rpanfili opened this issue Dec 18, 2018 · 1 comment
Closed

Unable to set vhost with amqp DSN #696

rpanfili opened this issue Dec 18, 2018 · 1 comment
Labels

Comments

@rpanfili
Copy link
Contributor

rpanfili commented Dec 18, 2018

Hello there, I got some troubles using a DSN string to configure my connection to rabbitMQ with the default %2f vhost name

Expected Behavior

<?php
include('vendor/autoload.php');
use Enqueue\AmqpTools\ConnectionConfig;

$dsn = "amqp://guest:guest@localhost:5672/%2f";
$config = (new ConnectionConfig($dsn))->parse();

var_dump($config->getVhost()); // string(1) "/"

Actual Behavior

<?php
include('vendor/autoload.php');
use Enqueue\AmqpTools\ConnectionConfig;

$dsn = "amqp://guest:guest@localhost:5672/%2f";
$config = (new ConnectionConfig($dsn))->parse();

var_dump($config->getVhost()); // string(0) ""

Steps to Reproduce the Problem

  1. composer require enqueue/enqueue enqueue/amqp-lib
  2. execute the given piece of code

Version:

sw vers
enqueue/amqp-lib 0.9.2
enqueue/amqp-tools 0.9.2
enqueue/dsn 0.9.2
enqueue/enqueue 0.9.3
enqueue/null 0.9.2
PHP 7.2.12

I think the problem is here:

'vhost' => null !== $dsn->getPath() ? ltrim($dsn->getPath(), '/') : null,

Since the given vhost is %f the path is urldecoded to // and the ltrim command remove everything, leaving an empty string.

I tried to patch (with success) like this:

'vhost' => null !== ($path = $dsn->getPath()) ?
                (0 === strpos($path, '/') ? substr($path, 1) : $path)
                : null,

Is there anything I'm missing about this kind of configuration?

@makasim
Copy link
Member

makasim commented Dec 18, 2018

That's a bug, only the first / has to be removed. Could you work on a fix?

@makasim makasim added the bug label Dec 18, 2018
makasim added a commit that referenced this issue Dec 18, 2018
[amqp] fix #696 parsing vhost from amqp dsn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants