Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add setting unicast bit #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add setting unicast bit #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1608,8 +1608,12 @@ question.encode = function (q, buf, offset) {

buf.writeUInt16BE(types.toType(q.type), offset)
offset += 2
if (q.qu === undefined || q.qu !== true) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check does not make sense

buf.writeUInt16BE(classes.toClass(q.class === undefined ? 'IN' : q.class), offset)
} else {
buf.writeUInt16BE(classes.toClass(q.class === undefined ? 'IN' : q.class) + QU_MASK, offset)
}

buf.writeUInt16BE(classes.toClass(q.class === undefined ? 'IN' : q.class), offset)
offset += 2

question.encode.bytes = offset - oldOffset
Expand All @@ -1630,7 +1634,14 @@ question.decode = function (buf, offset) {
q.type = types.toString(buf.readUInt16BE(offset))
offset += 2

q.class = classes.toString(buf.readUInt16BE(offset))
const classValue = buf.readUInt16BE(offset)
if (classValue >= QU_MASK) {
q.qu = true
q.class = classes.toString(classValue - QU_MASK)
} else {
q.class = classes.toString(classValue)
}

offset += 2

const qu = !!(q.class & QU_MASK)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section is still problematic (and there doesn't seem to be a test for decoding a question with the QU bit set) - if the bit is set, then we end up attempting a bitwise AND operation on the q.class string value (in line 1648 just below), which wipes out the string value (the result is 0).

Thank you very much for this effort - would you mind also adding a test for the decode function?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly, this change was quite some time ago. I recall this PR request occurred in 2018 or 2019. This weekend, I will take a look and make some adjustments.

Expand Down
39 changes: 39 additions & 0 deletions test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,45 @@ tape('query', function (t) {
name: 'hello.srv.com'
}]
})
testEncoder(t, packet, {
type: 'query',
questions: [{
type: 'A',
qu: true,
name: 'hello.a.com'
}, {
type: 'SRV',
name: 'hello.srv.com'
}]
})

testEncoder(t, packet, {
type: 'query',
id: 42,
questions: [{
type: 'A',
qu: true,
class: 'IN',
name: 'hello.a.com'
}, {
type: 'SRV',
name: 'hello.srv.com'
}]
})

testEncoder(t, packet, {
type: 'query',
id: 42,
questions: [{
type: 'A',
qu: true,
class: 'CH',
name: 'hello.a.com'
}, {
type: 'SRV',
name: 'hello.srv.com'
}]
})

t.end()
})
Expand Down