search

igrigorik / em-http-request

Asynchronous HTTP Client (EventMachine + Ruby)
1.22k stars 222 forks source link

Don't modify header for CONNECT style proxies. #282

Closed psschroeter closed 9 years ago

psschroeter commented 9 years ago

Nginx at least does not like it in certain cases, such as when basic auth is used to create a session token.

psschroeter commented 9 years ago

Yes more nil port bugs! Well I fixed the socks one also, though I didn't test the socks stuff. I improved it in the sense it couldn't possibly be right before as em-socksify would raise an error trying to call pack on a nil value. I see its sort of addressed in some other pull request as well. So if you're using addressable::uri, it really seems you should always be using .inferred_port in most places. .port is better than .inferred_port almost never.

You mentioned tests in that PR, but I'm not sure what you meant? Don't see any many unit style tests to test something like a function returns a certain value. Not sure if I see an easy way to add in an integration type test for this beyond what's there as it doesn't seem like the the stallion spec server speaks https?

igrigorik commented 9 years ago

:+1: ... and thanks for your patience :)