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.

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

potential bugfix for TXT records splitting after whitespace #18

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

Conversation

lastarel
Copy link

Hi,

For TXT records that can have whitespaces such as:
SPF records:
IE: "v=spf1 include:_spf.google.com ~all"
Prior to commit this would've been parsed as:
dig(['google.com','TXT']).then((res)=>{
console.log(res.answer)
})

{
domain: 'google.com.',
type: 'TXT',
ttl: '3591',
class: 'IN',
value: '~all"'
},

Post commit:
dig(['google.com','TXT']).then((res)=>{
console.log(res.answer)
})

{
domain: 'google.com.',
type: 'TXT',
ttl: '3561',
class: 'IN',
value: '"v=spf1 include:_spf.google.com ~all"'
},

@StephanGeorg
Copy link
Owner

StephanGeorg commented Oct 13, 2021

Hi @lastarel

Thank you for the PR. Could you please write a test for your scenario so I can review your changes?

@lastarel
Copy link
Author

Hi,

Not really sure what you refer to by a test as I've commented above pre-post the responses received.

As to not make another commit with a simple test you can run this test :
it('Should query TXT for spf.test.com and return a full valid value for spf', (done) => { dig(['spf.test.com', 'TXT']) .then((result) => { expect(result).to.be.an('object') .and.to.have.property('question') .and.to.be.an('array'); expect(result).to.have.property('answer') .and.to.be.an('array'); expect(result.answer[0].value).to.be.a('string') expect(result).to.have.property('time') .and.to.be.an('number'); expect(result).to.have.property('server') .and.to.be.an('string'); expect(result).to.have.property('datetime') .and.to.be.an('string'); expect(result).to.have.property('size') .and.to.be.an('number'); expect(result.answer[0].value).to.contain('v=spf1 ~all') expect(result.answer[0].type).to.equal("TXT") done(); }) .catch((err) => { console.log('Error:', err); }); });
It's a spin from your MX test so you can check the properties and values as well as the TXT return value.
Good luck hope it helps.

@clementcheung
Copy link

This bugfix uses joins(' ') to re-combine the improperly split TXT record by the previous line.split(/\s+/g). This does not work for a train of consecutive whitespaces inside the TXT record.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants