Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fixes non buffers in data event issue #75

Closed
wants to merge 1 commit into from

2 participants

@dscape

[main,test-body] fixes non buffers in data event issue when setting encoding manually

  • added unit test to test/test-body.js
  • fixed bug by checking if chunk is buffer in main.js
  • fixes #74
@dscape dscape [main,test-body] fixes non buffers in data event issue when setting e…
…ncoding manually

* added unit test to test/test-body.js
* fixed bug by checking if chunk is buffer in main.js
* fixes #74
fcac113
@mikeal
Owner

i think i see what the problem is. if you setEncoding the chunks won't be buffers, they'll be strings. the code @issacs wrote to merge the array together assumes buffers.

it would be lower impact to add this isBuffer check to where it gets concatenated rather than creating new buffers around each chunk if they are already encoded in to a string.

@dscape

I think I tried what you are suggesting first @mikeal but then the string was being trimmed? Then I thought it oughta be because of the length variable that was being aggregated, so as a simple fix moved it up.

@mikeal mikeal closed this
@mikeal
Owner

Fixed in a different commit.

@dscape

@mikeal I think you forgot the test, will help prevent that future improvements re-introduce the bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 9, 2011
  1. @dscape

    [main,test-body] fixes non buffers in data event issue when setting e…

    dscape authored
    …ncoding manually
    
    * added unit test to test/test-body.js
    * fixed bug by checking if chunk is buffer in main.js
    * fixes #74
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 0 deletions.
  1. +1 −0  main.js
  2. +5 −0 tests/test-body.js
View
1  main.js
@@ -310,6 +310,7 @@ Request.prototype.request = function () {
var buffer = []
var bodyLen = 0
options.on("data", function (chunk) {
+ chunk = Buffer.isBuffer(chunk) ? chunk : new Buffer(chunk)
buffer.push(chunk)
bodyLen += chunk.length
})
View
5 tests/test-body.js
@@ -25,6 +25,11 @@ var tests =
])
, expectBody: "Ω☃"
}
+ , testChunkNotABuffer :
+ { resp : server.createChunkResponse(['BM:\u0000\u0000\u0000'])
+ , encoding : 'binary'
+ , expectBody : 'BM:\u0000\u0000\u0000'
+ }
, testGetJSON :
{ resp : server.createGetResponse('{"test":true}', 'application/json')
, json : true
Something went wrong with that request. Please try again.