search

mafintosh / dns-packet

An abstract-encoding compliant module for encoding / decoding DNS packets
MIT License
201 stars 70 forks source link

add setting unicast bit #49

Closed XiXiangFiles closed 10 months ago

XiXiangFiles commented 5 years ago

I have fixed issue for "Support unicast-response bit #21". In the classes.js file, increased 2 lines to support it. I used the string "UNI" to express the "unicast".

pusateri commented 5 years ago

RFC 6762 Section 5.4 calls these "QU" questions for Query Unicast.

XiXiangFiles commented 5 years ago

I have renamed the string "UNI" to "QU". thanks for your comments .

silverwind commented 5 years ago

Read the RFC section again. I think it certainly is better to handle it like a flag, like question.QU = true, I'd say the property should be absent when decoding a response, given that the RFC states the bit is only valid for questions.

XiXiangFiles commented 5 years ago

Many thanks for your comments, i learned a lot in your opinions. i will try to revise it, and i had got an idea for your comments.

XiXiangFiles commented 5 years ago

I had revised the codes, the classes.js had recovered to the recent status and the index.js added the flag in question section. When the question.QU is "true ", the class value will set the bit.

silverwind commented 5 years ago

Could you also add a test?

XiXiangFiles commented 5 years ago

I had already added the test and followed your comments to revise the code. thank you very much for your advice.

ke4nec commented 2 years ago

Is there any progress?

silverwind commented 2 years ago

Should probably raise a new PR with the feedback applied.

MaximKalinin commented 1 year ago

Should probably raise a new PR with the feedback applied.

What kind of feedback should be applied? I might be wrong, but seems like all the essential suggestions are already there.

silverwind commented 10 months ago

These, I would expect a comment on each at least:

https://github.com/mafintosh/dns-packet/pull/49#pullrequestreview-209077128 https://github.com/mafintosh/dns-packet/pull/49#pullrequestreview-209079338 https://github.com/mafintosh/dns-packet/pull/49#pullrequestreview-209079459

Thought it has been far too long, but if you bring the branch up to date, I will have another look.

XiXiangFiles commented 10 months ago

@silverwind update at here