Open pusateri opened 6 years ago
Yeah that seems unneccesary. I think we should get rid of flags
entirely and just output all flags/values in these two octets separately.
Gave it some more thought, I guess we'll have to keep flags
for compatibility.
As for flags: flags & 32767
, I don't see a clear reason why this bit is zeroed in both encode
and decode
. This was intruced in https://github.com/mafintosh/dns-packet/commit/1d241f5f9728334ff51d3d5aeadb75cb297542da, maybe @mafintosh remembers.
Another option could be to limit flags
to just the 7 bit that are actually flags, e.g. bits 5 to 11. From https://tools.ietf.org/html/rfc5395#section-2:
1 1 1 1 1 1
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
|QR| OpCode |AA|TC|RD|RA| Z|AD|CD| RCODE |
+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
I could also see us transforming flags
into an object. That way, the user does not have to do binary operations and we could round-trip the same object through decode/encode. For example:
flags: {
aa: false,
tc: true,
rd: true,
...
}
Yeah, I like the object. Currently, there's no setting individual flags. The set/get should be more uniform.
Currently, tere's no setting individual flags
It's possible with the or-ed constants. As I see, the intention for flags & 32767
is so that flags
corresponds to bits 1-15, with bit 0 being encoded separately in the type
.
Not sure yet what to do about it. The object might have benefits, but it also might cost some performance as compared to bitflags.
Currently, the header flags are decoded as:
flags: flags & 32767
I understand the Q/R flag is pulled out as 'type' but it is technically part of the flags and probably should be included in the flags field. Currently, this breaks a test where I encode a packet and decode it and compare the fields.
Thoughts?
Would it break backward compatibility to change it now? Maybe it's better to leave it alone and mask out the flags comparison in the test.