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

Default constructed LDAPFilter seg faults when calling LDAPFilter::Match(...) #251

Closed
jeffdiclemente opened this Issue Nov 10, 2017 · 2 comments

Comments

Projects
2 participants
@jeffdiclemente
Contributor

jeffdiclemente commented Nov 10, 2017

Example:

ServiceReferenceBase sr;
LDAPFilter filter; 
filter.Match(sr); // This will segfault

@jeffdiclemente jeffdiclemente added the bug label Nov 10, 2017

@jeffdiclemente jeffdiclemente self-assigned this Nov 10, 2017

@jeffdiclemente jeffdiclemente added this to In Progress in Release 3.3 Nov 10, 2017

jeffdiclemente added a commit that referenced this issue Nov 10, 2017

Fix seg faults when using default constructed LDAPFilter
Fixes #251 

Signed-off-by: The MathWorks, Inc. Roy.Lurie@mathworks.com
@saschazelzer

This comment has been minimized.

Member

saschazelzer commented Nov 15, 2017

The LDAPFilter constructor states that a default constructed filter object is invalid and calling any other method then the boolean conversion operator results in undefined behavior. So the semantics are at least correct according to the documentation.

Doing the changes as in the linke PR means that you would not be able to tell if e.g. a false return value of the Match function is due to the filter not matching the expression or due to the filter being invalid. It is like if dereferecing a null (shared) pointer returns some sort of "default" instance.

In my opinion, we should keep it as is and test LDAPFilter instances for validity if we do not know how they were constructed, or throw an exception in case a method is called on an invalid instance. Either way, client code needs to prepare for one or the other situation.

@saschazelzer

This comment has been minimized.

Member

saschazelzer commented Nov 16, 2017

After thinking more about it, I realized that I was trying to hold on to the original semantics, although the LDAPFilter class actually is a strange mix of a value and a pointer type. So if we change the meaning of the default constructor to create a valid instance that by definition matches nothing, we don't need to require clients to check the validity or handle exceptions.

We still have the issue mentioned in the second paragraph above, but the pointer analogy was not a good one to begin with.

jeffdiclemente added a commit that referenced this issue Nov 20, 2017

Fix seg faults when using default constructed LDAPFilter (#252)
* Fix seg faults when using default constructed LDAPFilter

Fixes #251 

Signed-off-by: The MathWorks, Inc. Roy.Lurie@mathworks.com

@saschazelzer saschazelzer moved this from In Progress to Done in Release 3.3 Nov 29, 2017

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