browserify / http-browserify

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

Support "ArrayBuffer" to be part of the string. #60

Closed eiriksm closed 8 years ago

eiriksm commented 10 years ago

This is the PR mentioned in #59

eiriksm commented 10 years ago

Also added a test case for this.

It is admittedly a bit more monkey patched than the rest of the tests in the test suite, but at least it proves the point: It fails with current codebase and passes on the patched.

I tried to match the coding style in the tests as well, but I am happy to change it around if you feel differently about something there.

eiriksm commented 10 years ago

Very shamelessly pinging @maxogden , since I saw that's where the code in question came from.

Also noting that this is not because I think 2 days is a long time to wait for a PR to be responded to, just I know substack has 1M projects. Hm, probably applies to maxogden too. Oh well.

tmpvar commented 10 years ago

+1 just ran into this with while requesting http://wzrd.in/standalone/concat-stream@latest

@eiriksm's fix looks good - @substack can we get some love here?

eiriksm commented 10 years ago

Thanks for that @tmpvar

Just noticed my pull request contained merge conflicts. Now updated against latest master

esundahl commented 10 years ago

Any chance at getting this merged? Thanks

tmpvar commented 10 years ago

bump

hughsk commented 10 years ago

+1

Janpot commented 10 years ago

Having the same issue

eiriksm commented 9 years ago

Just shamelessly bumping this. And for those who only have read this and not my original bug report: Here is also a POC site that explains the problem with an example: https://eiriksm.github.io/browserify-test/

eiriksm commented 8 years ago

This bug is no longer a problem in latest browserify. It was probably fixed when browserify switched to stream-http instead of this package.

So as a reference, for anyone randomly coming here so long after it was fixed: You should probably upgrade browserify.