Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

src: fix intermittent SIGSEGV in resolveTxt #9300

Closed
wants to merge 1 commit into from

Conversation

evanlucas
Copy link

Fixes a SIGSEGV by making sure txt_chunk is not empty before setting
it on txt_records

Ref: #9285

This is the same fix applied in nodejs/node#960

Fixes a SIGSEGV by making sure `txt_chunk` is not empty before setting
it on `txt_records`
@cjihrig
Copy link

cjihrig commented Mar 1, 2015

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Mar 4, 2015

LGTM

@evanlucas
Copy link
Author

ping

@maleadt
Copy link

maleadt commented Jul 7, 2015

Any update on this PR?

@maxtaco
Copy link

maxtaco commented Jul 24, 2015

Also would love to see a merge of this fix.

@celesteking
Copy link

Node v0.12.7 (Stable)

This is incorrect. 0.12 nodejs isn't stable and should not be used in production.

@jasnell
Copy link
Member

jasnell commented Aug 27, 2015

@celesteking ... minor correction: v0.12.7 is stable and will be fully supported until Dec 31, 2016.

@celesteking
Copy link

yeah, especially when you get a SIGSEGV on production box. Right.

@maxtaco
Copy link

maxtaco commented Aug 27, 2015

BTW, we switched to native-dns as a workaround.

jasnell pushed a commit that referenced this pull request Aug 27, 2015
Fixes a SIGSEGV by making sure `txt_chunk` is not empty before setting
it on `txt_records`

Fixes: #9285
PR-URL: #9300
Reviewed-By: cjihrig - Colin Ihrig <[email protected]>
Reviewed-By: jasnell - James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 27, 2015

Landed in 261fa36

@jasnell jasnell closed this Aug 27, 2015
@evanlucas evanlucas deleted the dns-txt-fix branch August 27, 2015 18:42
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Fixes a SIGSEGV by making sure `txt_chunk` is not empty before setting
it on `txt_records`

Fixes: nodejs#9285
PR-URL: nodejs#9300
Reviewed-By: cjihrig - Colin Ihrig <[email protected]>
Reviewed-By: jasnell - James M Snell <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants