Implement charset handling in WebRequestConcern #950
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knu
force-pushed
the
web_content_charset
branch
from
2b5c7dd
to
c047ed8
Compare
- The `force_encoding` and `unzip` options in WebsiteAgent are moved to WebRequestConcern so other users of the concern such as RssAgent can benefit from them. - WebRequestConcern detects a charset specified in the Content-Type header to decode the content properly, and if it is missing the content is assumed to be encoded in UTF-8 unless it has a binary MIME type. Not all Faraday adopters handle character encodings, and Faraday passes through what is returned from the backend, so we need to do this on our own. (cf. lostisland/faraday#139) - WebRequestConcern now converts text contents to UTF-8, so agents can handle non-UTF-8 data without having to deal with encodings themselves. Previously, WebsiteAgent in "json"/"text" modes and RssAgent would suffer from encoding errors when dealing with non-UTF-8 contents. WebsiteAgent in "html"/"xml" modes did not have this problem because Nokogiri would always return results in UTF-8 independent of the input encoding. This should fix #608.
knu
force-pushed
the
web_content_charset
branch
from
c047ed8
to
6f667a4
Compare
| # Not all Faraday adapters support automatic charset | ||
| # detection, so we do that. | ||
| case env[:response_headers][:content_type] | ||
| when /;\s*charset\s*=\s*([^()<>@,;:\\\"\/\[\]?={}\s]+)/i |
There was a problem hiding this comment.
Would https://github.com/cantino/guess_html_encoding be useful here?
There was a problem hiding this comment.
I think so. More detection logics can be added later. BOM, XML declaration, HTML <meta> elements, etc.
|
This looks really good! |
knu
added a commit
that referenced
this pull request
Aug 3, 2015
Implement charset handling in WebRequestConcern
|
@cantino Please feel free to improve the charset detection part! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
force_encodingandunzipoptions in WebsiteAgent are moved toWebRequestConcern so other users of the concern such as RssAgent can
benefit from them.
header to decode the content properly, and if it is missing the
content is assumed to be encoded in UTF-8 unless it has a binary MIME
type. Not all Faraday adopters handle character encodings, and
Faraday passes through what is returned from the backend, so we need
to do this on our own. (cf. response.body is ASCII-8BIT when Content-Type is text/xml; charset=utf-8 lostisland/faraday#139)
handle non-UTF-8 data without having to deal with encodings
themselves. Previously, WebsiteAgent in "json"/"text" modes and
RssAgent would suffer from encoding errors when dealing with non-UTF-8
contents. WebsiteAgent in "html"/"xml" modes did not have this
problem because Nokogiri would always return results in UTF-8
independent of the input encoding.
This should fix #608.