BIND 10 #459: Dangerously counter-intuitive isc::auth::Query interface

[BIND 10 Development]

#459: Dangerously counter-intuitive isc::auth::Query interface
-----------------------+----------------------------------------------------
  Reporter:  vorner    |            Owner:     
      Type:  defect    |           Status:  new
  Priority:  major     |        Milestone:     
 Component:  b10-auth  |         Keywords:     
 Sensitive:  0         |   Estimatedhours:  0  
     Hours:  0         |         Billable:  1  
Totalhours:  0         |         Internal:  0  
-----------------------+----------------------------------------------------
 The Query constructor takes several const something& parameters. This
 allows for a temporary value to be passed as the parameter. And it is
 quite common to use such interface in this kind of manner.

 But the Query class stores const references and uses them later on. So, if
 the Query is use like this:

 {{{
 Query mx_query(memory_datasrc, Name("mx.example.com"), RRType::MX(),
 response);
 mx_query.process();
 }}}

 then it does not do what is expected. It creates the Name. Then reference
 to it is passed to the Query constructor and stored. After the constructor
 is terminated, the Name is destroyed. So, when we call .process(), the
 reference already points to a place on the stack where the Name is no
 longer present, which seems quite dangerous. However, this code works like
 expected:

 {{{
 Name qname("mx.example.com");
 Query mx_query(memory_datasrc, qname, RRType::MX(), response);
 mx_query.process();
 }}}

 This behaviour is counter-intuitive and isn't even mentioned in the
 documentation. I think it should be changed (either use non-const
 reference, pointer or make a copy of the parameters). If there really is
 reason for such strange expectation about the parameters, it should be
 documented at last.

-- 
Ticket URL: <http://bind10.isc.org/ticket/459>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development