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

Jump to bottom

crash on Buffer error in v6 #6510

Closed
Hamper opened this issue May 2, 2016 · 2 comments
Closed

crash on Buffer error in v6 #6510

Hamper opened this issue May 2, 2016 · 2 comments
Labels
buffer Issues and PRs related to the buffer subsystem.

Comments

@Hamper
Copy link

Hamper commented May 2, 2016

  • Version: v6.0.0
  • Platform: Linux notebook 4.5.2-calculate test: don't remove empty.txt on win32 #2 SMP PREEMPT Sat Apr 23 00:07:11 MSK 2016 x86_64 AMD A10-5750M APU with Radeon(tm) HD Graphics AuthenticAMD GNU/Linux
  • Subsystem:
b = new Buffer([1,2])
b.lastIndexOf('123')

Result:

node: ../src/string_search.h:655: size_t node::SearchString(const Char*, size_t, const Char*, size_t, size_t, bool) [with Char = unsigned char; size_t = long unsigned int]: Assertion `haystack_length >= needle_length' failed.
[1]    6843 abort (core dumped)  node

In v5 last string just return -1.
@addaleax addaleax added the buffer Issues and PRs related to the buffer subsystem. label May 2, 2016
@addaleax
Copy link
Member

addaleax commented May 2, 2016

Thanks for reporting this! I’m looking into it.

@addaleax addaleax mentioned this issue May 2, 2016
Closed
3 tasks
addaleax added a commit to addaleax/node that referenced this issue May 3, 2016
@addaleax
buffer: fix lastIndexOf crash for overlong needle
4220cb9 
Return -1 in `Buffer.lastIndexOf` if the needle is longer than the
haystack. The previous check only tested the corresponding
condition for forward searches.

This applies only to Node.js v6, as `lastIndexOf` was added in it.

Fixes: nodejs#6510
@jasnell
Copy link
Member

jasnell commented May 4, 2016

@Hamper ... it turns out that we had several issues to resolve in this bit of code. @addaleax did an amazing job of tracking those down and just landed the fix in master. Thank you for reporting the issue! (Note, parts of the issue actually impact v5 and v4 also so we'll be backporting those fixes to those versions).

evanlucas pushed a commit that referenced this issue May 17, 2016
@addaleax @evanlucas
buffer: fix lastIndexOf crash for overlong needle
6fc20c5 
Return -1 in `Buffer.lastIndexOf` if the needle is longer than the
haystack. The previous check only tested the corresponding
condition for forward searches.

This applies only to Node.js v6, as `lastIndexOf` was added in it.

Fixes: #6510
PR-URL: #6511
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jasnell @addaleax @Hamper