search

NLnetLabs / ldns

LDNS is a DNS library that facilitates DNS tool programming
https://nlnetlabs.nl/ldns
BSD 3-Clause "New" or "Revised" License
292 stars 98 forks source link

Fixes #173 #174

Open vzuevsky opened 2 years ago

vzuevsky commented 2 years ago

Fixes https://github.com/NLnetLabs/ldns/issues/173

wtoorop commented 2 years ago

Thanks @vzuevsky , that looks like a valuable contribution! And properly documented as well! Much appreciated. I'll try to review it shortly.

psvz commented 2 years ago

Hi @wtoorop, any news on release with the fix? Cheers

wtoorop commented 2 years ago

@psvz I do think I can have a good look coming Friday. I'll keep you posted

wtoorop commented 2 years ago

Thank you Vitaly! Very clever! Is this algorithm by your own making?

W.r.t. the PR, I don't want to change the function definitions (in order not to break backwards compatibility), so I'm happy to change the default in ldns_pkt2buffer_wire(), but I'd like to keep the definitions of ldns_dname2buffer_wire_compress(), ldns_rdf2buffer_wire_compress(), ldns_rr2buffer_wire_compress() and ldns_pkt2buffer_wire_compress() and then add new definitions for them that use ldns_llnode_t. I'd also prefer a better name for ldns_llnode_t. For example ldns_compression_node_t. We can add text in the description of the old definitions in which we strongly recommend to use the new definitions.

So I will make those changes after RIPE84 in the run up to a new ldns release, or you can do them if you want to help me out :) It would be appreciated, but I also really don't mind if you leave that to me.

In any case thanks for this excellent contribution/improvement!

vzuevsky commented 2 years ago

Thank you Vitaly! Very clever! Is this algorithm by your own making?

@wtoorop Thanks for your consideration. The algo is my own development. I am happy with the way forward you suggest. They are simple edits, so I would wait for a due stable release with all security fixes including mine. Any tentative date for that?

wtoorop commented 2 years ago

@vzuevsky I'm going to postpone this for after the next release, because we think we found an even better way to do this.

psvz commented 1 year ago

Hi @wtoorop is it fixed in a release yet?