FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: Examples work when data exceeds buffer size by garrows · Pull Request #4811 · nodejs/node · GitHub

/ node Public

doc: Examples work when data exceeds buffer size - #4811

Closed
garrows wants to merge 1 commit into
nodejs:masterfrom
garrows:fix/crypto-clarity
ConversationCommits (1)ChecksFiles changed
Closed

doc: Examples work when data exceeds buffer size#4811
garrows wants to merge 1 commit into
nodejs:masterfrom
garrows:fix/crypto-clarity

Conversation

garrows commented Jan 22, 2016

Copy link
Copy Markdown
Contributor

The crypto examples would only work when the input data was under 16 bytes. Above that, important data was being thrown away and decrypting them would give a difficult to debug error (see below).

Error given when .update data was being discarded:
Error: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt

mscdex added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Jan 22, 2016

jasnell commented Jan 22, 2016

Copy link
Copy Markdown
Member

@nodejs/crypto

bnoordhuis commented Jan 22, 2016

Copy link
Copy Markdown
Member

LGTM but the commit log should conform to the guidelines. Can you update it according to CONTRIBUTING.md?

shigeki commented Jan 22, 2016

Copy link
Copy Markdown
Contributor

I think it is a kind of topics how to write a stream example in a doc rather than crypto feature.
Consistencies with other stream examples in hash and other API needs to be considered. I would like to have a comment from @nodejs/documentation .

eljefedelrodeodeljefe commented Jan 22, 2016

Copy link
Copy Markdown
Contributor

From a doc point of view LGTM. I think we need to pick up the stream docmore globally.

shigeki commented Jan 22, 2016

Copy link
Copy Markdown
Contributor

@eljefedelrodeodeljefe Okay, thanks. LGTM too.
@garrows Please revise your commit messages as Ben's comment.

garrows force-pushed the fix/crypto-clarity branch from 2cbcc10 to b4a0f20 Compare January 22, 2016 23:06
garrows changed the title Examples work when data exceeds buffer size (16 bytes) doc: Examples work when data exceeds buffer size Jan 22, 2016

garrows commented Jan 22, 2016

Copy link
Copy Markdown
Contributor Author

I've updated the commit message. Cheers.

jasnell commented Jan 23, 2016

Copy link
Copy Markdown
Member

LGTM

1 similar comment

indutny commented Jan 23, 2016

Copy link
Copy Markdown
Member

LGTM

jasnell pushed a commit that referenced this pull request Jan 23, 2016
doc: Examples work when data exceeds buffer size
26073dd 
PR-URL: #4811
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Jan 23, 2016

Copy link
Copy Markdown
Member

Landed in 26073dd

jasnell closed this Jan 23, 2016

garrows commented Jan 24, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for accepting!
Will this be backported to the current 5.5.0 docs? It should be safe to go back to 4.x.x.

rvagg pushed a commit that referenced this pull request Jan 25, 2016
doc: Examples work when data exceeds buffer size
5a860d9 
PR-URL: #4811
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

bnoordhuis commented Jan 25, 2016

Copy link
Copy Markdown
Member

@jasnell added the lts-watch-v4.x label so this should get back-ported.

rvagg mentioned this pull request Feb 8, 2016
Closed

MylesBorins commented Feb 17, 2016

Copy link
Copy Markdown
Contributor

this commit relies on a number of other doc changes to land in LTS first.

MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
doc: Examples work when data exceeds buffer size
7db57ea 
PR-URL: #4811
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
doc: Examples work when data exceeds buffer size
3f876b1 
PR-URL: #4811
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
doc: Examples work when data exceeds buffer size
90e0197 
PR-URL: #4811
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
doc: Examples work when data exceeds buffer size
d1225cf 
PR-URL: nodejs#4811
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL