browserify / http-browserify

node's http module, but for the browser
MIT License
244 stars 110 forks source link

Silent failure if content-length is manually set in browserified http request #12

Closed brycebaril closed 11 years ago

brycebaril commented 11 years ago

This took me a little while to track down today -- I browserified an api client that was setting content-length when POSTing. This resulted in the posts hitting the server, but complete silence in the browser client code. It turns out this line was my problem (request.js line 23):

            if (!self.isSafeRequestHeader(key)) return;

Now, of course, my server doesn't actually care if content-length isn't sent, so I just stopped setting that header. I'm mostly apathetic about browserified code sending it versus node code (now) not sending it, since it works either way. But...

Is there a better way to do this?

More importantly -- is this the right way to handle "unsafe" headers? With no error, warning, etc. it makes it difficult to debug.

brycebaril commented 11 years ago

Looking at the code history this looks like just a simple bug -- the bare return; used to be exiting a forEach loop which has since been converted into a C-style for loop in d50a19c. Now the return drops you entirely out of context and you never get your callback executed. It should probably just be continue;

            if (!self.isSafeRequestHeader(key)) continue;
brycebaril commented 11 years ago

Thanks for merging my PR, closing this issue now.