(WIP) LibDNS: Let's make >90% of the websites fail to resolve because dnssec #3883
Conversation
Previously this function's return value was not reliable, as available data on the underlying socket did not necessarily translate to available application data on the TLS socket. This commit makes it so this function only returns true when we actually have data buffered and/or available.
| key_tag += (bit_cast<u16>(NetworkOrdered<u16>(flags)) & 0xff) << 8; | ||
| key_tag += (bit_cast<u16>(NetworkOrdered<u16>(flags)) >> 8) & 0xff; |
There was a problem hiding this comment.
Would a bit_cast<NetworkOrdered<u16>>(flags) & 0xff << 8 work as well?
Not sure which is more idiomatic and or what is actually meant to happen here, especially as flags is read as network ordered and directly converted.
Note that Endian allows for implicit conversion
There was a problem hiding this comment.
that could actually just be convert_between_host_and_blahblah, should do that.
| auto protocol = TRY(ctx.stream.read_value<u8>()); | ||
| key_tag += static_cast<u16>(protocol) << 8; | ||
| auto algorithm = static_cast<DNSSEC::Algorithm>(static_cast<u8>(TRY(ctx.stream.read_value<u8>()))); | ||
| key_tag += static_cast<u16>(algorithm); |
There was a problem hiding this comment.
you could replace the autos with explicit types here.
(or even hope that << coerces to int, which I am not sure about)
There was a problem hiding this comment.
They're used later with the deduced type, can't really change the variable types like that without adding an extra static_cast later.
| auto public_key = TRY(ctx.stream.read_until_eof()); | ||
| return Records::DNSKEY { flags, protocol, algorithm, move(public_key) }; | ||
| for (size_t i = 0; i < public_key.size(); ++i) { |
There was a problem hiding this comment.
Can this be odd-sized?
There was a problem hiding this comment.
Possibly? Not sure
| TRY(stream.write_value(static_cast<u8>(algorithm))); | ||
| TRY(stream.write_value(static_cast<u8>(digest_type))); |
There was a problem hiding this comment.
If the enums are properly sized this should be able to take them without casts
| TRY(stream.write_value(static_cast<NetworkOrdered<u32>>(serial))); | ||
| TRY(stream.write_value(static_cast<NetworkOrdered<u32>>(refresh))); | ||
| TRY(stream.write_value(static_cast<NetworkOrdered<u32>>(retry))); | ||
| TRY(stream.write_value(static_cast<NetworkOrdered<u32>>(expire))); | ||
| TRY(stream.write_value(static_cast<NetworkOrdered<u32>>(minimum))); |
There was a problem hiding this comment.
Don't these static_casts just call the normal constructor?
so you could shorten this quite a bit
There was a problem hiding this comment.
Yeah it calls the ctor, sure.
obviously needs cleanup, more chrome wiring, and some more cleanup.