Force UTF-8 encoding on JSON Podspecs #629
Conversation
|
Do you think this is related to CocoaPods/CocoaPods#9672? |
|
I've done my best to follow the CONTRIBUTING guide from CocoaPods/CONTRIBUTING.md. These are likely missing before this is mergeable:
|
|
Hi @dnkoutso 👋 As the JSON podspec exists already, |
|
awesome. I can guide you tomorrow (PST timezone here) on how to add the test. Can you give access in this PR for me to push? If not I can open a new PR and ensure you receive credit for the fix of course in the changelog entry. |
|
Amazing. Thanks so much. I'm happy to make changes with a little guidance. Have a good evening! |
|
Have you managed to figure out why the response encoding is reported as ASCII when the header is |
|
@igor-makarov I haven't figured that out. In a simple |
|
I did some additional checking and it appears that this is a Typhoeus bug - in fact, they say that a lot of HTTP libs make this mistake and disregard the I think using |
|
Igor, I can definitely make that change. Would this PR need a unit test? If
so, can you point me in the right direction? Just looking for a sample.
Thanks!
…On Wed, May 13, 2020, 12:54 PM Igor Makarov ***@***.***> wrote:
I did some additional checking and it appears that this is a Typhoeus bug
- in fact, they say <typhoeus/typhoeus#580>
that a lot of HTTP libs make this mistake and disregard the charset
header.
I think using force_encoding on the response to force the responce body
to be treated as UTF-8 might be a better idea. After all, we do know for
sure that the files are UTF-8.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#629 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFWGGYIIRIWUSPZCKU4OWTRRL3I3ANCNFSM4M7OQRGA>
.
|
|
I saw the new fix, seems good. 👍🏻 I'll write an explainer on how to add the unit tests tomorrow, as I'm in GMT+2 and it's nighttime here... |
|
Perhaps the PR title needs to be changed? |
|
we could actually ship 1.9.2 here with this fix. If we want that we should change destination branch to |
jasonschroeder-sfdc
changed the title
|
Title updated. Let me know if you'd like these commits squashed. |
|
I think the 1st commit can be squashed into the 2nd. |
|
As for unit testing, in You can use that method to stub Typhoeus. To check that |
|
Please point to |
|
(accidental close sorry) |
jasonschroeder-sfdc
force-pushed
the
cdn_source_encoding
branch
from
93a7666
to
014f6f7
Compare
There is a bug in Typhoeus. See typhoeus/typhoeus#580 This will force the response_body to always be UTF-8. Some response_body are coming back with encoding=ASCII-8BIT but are not writable to disk, with errors like this: ``` [!] CDN: trunk Repo update failed - 31 error(s): "\xE2" from ASCII-8BIT to UTF-8 ```
Adding PR-629 to the CHANGELOG.md.
Added a unit test for confirming that "force_encoding" is invoked on the Typhoeus response body.
jasonschroeder-sfdc
force-pushed
the
cdn_source_encoding
branch
from
014f6f7
to
40cc469
Compare
|
@dnkoutso: Updated the base to |
|
@igor-makarov want to take a quick look before I press merge? |
|
Looks good to me @dnkoutso ! |
Write the file from CDN to disk without any conversions.
Some
response_bodyare coming back with encoding=ASCII-8BITbut are not writable todisk, with errors like this:
(second line repeated 31 times)
Environment:
rvmEnv vars:
LANG=en_US.UTF-8Here is the stacktrace from
pod install --verboseSome sample "bad" JSON Podspecs:
\x00E2in the Description ):’