avoid 'invalid byte sequence in UTF-8'

[shao1555]

when Graph API server responses with invalid UTF-8 string, fb_graph's node#handle_response method raise error.

invalid byte sequence in UTF-8
fb_graph (2.7.8) lib/fb_graph/node.rb:138:in `handle_response'

to avoid this problem, strip invalid string from response body. use String#scrub method introduced in Ruby 2.1, older version use backport of this method or known workaround

[shao1555]

oops, failed in MRI 1.9.3 . I'll fix it.

[shao1555]

@nov ok. ready to review it.

[nov]

Why are you putting this hack in this layer? I don't think it's facebook specific issue.

[shao1555]

httpclient returns response body with binary format. (not in UTF-8 or US-ASCII. just a binary.) so fb_graph library should convert response body to proper encoding (e.g. response.body.force_encoding(Encoding::UTF-8)) also, have to get rid of invalid string in this context. So I've implemented "scrub" or convert encoding after response received. another solution is monkey patch to Regex#=~, but it's too drastic.

how do you think about this problem?

[nov]

umm, I don't think this is more important than ruby 1.8 support..

[shao1555]

on my service, there are a lot of 'invalid byte sequence in UTF-8' error raised from this method. your consideration is compatibility for Ruby 1.8.x ? should I add codes for supporting 1.8.x ?

[nov]

I've just pushed a commit which defines a httpclient request filter for scrubbing. Can you try it?

If you need 1.9 support too, let's modify the filter.

[shao1555]

HTTP::Message#body is immutable and there are no accessors method on HTTP::Message::Body.

to set scrubbed string, use HTTP::Message::Body#init_response. I think it is not perfect solution but works well.

do you have any idea?

[nov]

probably your test failed because String#scrub! isn't defined, not because HTTP::Message#body is immutable (a.k.a "freezed" in ruby).

[shao1555]

merged your topic branch then added 2.0.0 support (still not care for 1.9.3). thank you for your cooperation!

[nov]

for 2.0, you can require 'string/scrub' gem in your app, so it shouldn't be needed here.

[shao1555]

thanks a lot! I appreciate you.