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

NullReferenceException when setting null Uri into ConnectionFactory #1622

Closed
lechu445 opened this issue Jun 28, 2024 · 2 comments · Fixed by #1626
Closed

NullReferenceException when setting null Uri into ConnectionFactory #1622

lechu445 opened this issue Jun 28, 2024 · 2 comments · Fixed by #1626
Assignees
Labels
Milestone

Comments

@lechu445
Copy link
Contributor

Describe the bug

It is not clear if ConnectionFactory can have null Uri property. If ConnectionFactory is created via constructor, Uri property returns null. But, ConnectionFactory does not allow explicit set null Uri.

Reproduction steps

Simple reproduction:

using RabbitMQ.Client;

var c = new ConnectionFactory();

Console.WriteLine(c.Uri == null); // prints True

c.Uri = null; // throws NullReferenceException

Expected behavior

NullReferenceException is not thrown.

Additional context

This issue was detected by Roslyn analyzer during work on #1596.

@lechu445 lechu445 added the bug label Jun 28, 2024
@lechu445
Copy link
Contributor Author

I don't fully understand the design choice to allow users to set and override Uri after ConnectionFactory is created. Maybe the design should be changed to leave only getter and move Uri setting logic into ConnectionFactory constructior? Then we are able to add null check for it.

Here is what I mean:

    public sealed class ConnectionFactory : ConnectionFactoryBase, IConnectionFactory
    {
        private Uri? _uri;

        public ConnectionFactory(Uri uri) // added parameter uri
        {
            if (uri is null) // added null check
            {
                throw new ArgumentNullException(nameof(uri));
            }
            SetUri(uri); // this is now called in constructor
            ClientProperties = new Dictionary<string, object>(DefaultClientProperties);
        }

        public Uri? Uri
        {
            get { return _uri; }
            // removed setter
        }
    }

This is the cleanest solution, but require breaking change in ConnectionFactory and IConnectionFactory.

@lukebakken lukebakken self-assigned this Jun 28, 2024
@lukebakken lukebakken added this to the 8.0.0 milestone Jun 28, 2024
@lukebakken
Copy link
Contributor

@lechu445 please realize that this project's API is very, very old, so issues like this crop up.

Considering that you are the first person to notice or care about this behavior with regard to ConnectionFactory.Uri shows that it is low-priority to address.

I've marked this as a version 8 item so that better design choices can be made. Thanks.

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

Successfully merging a pull request may close this issue.

2 participants