Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS: Fix special character issue in image URL. #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

iOS: Fix special character issue in image URL. #157

wants to merge 1 commit into from

Conversation

gokcan
Copy link

@gokcan gokcan commented Apr 10, 2018

Hello @alwx,

If the external URL contains special characters such as 你好 and مرحبا,
image could not be displayed. We have faced this issue in zulip/zulip-mobile#2113.

To resolve this,
we need to escape special characters in uri using UTF-8 encoding.

Fixes #129

If the external URL contains special characters such as 你好 and مرحبا,
image could not be displayed.

To resolve this issue,
we need to escape special characters in `uri` using UTF-8 encoding.

Fixes #129
@gnprice
Copy link

gnprice commented Apr 12, 2018

I think this is not a correct fix -- it risks double-encoding. For example, if uri is a properly percent-encoded URL like https://commons.wikimedia.org/wiki/File:Mezquita_de_Nasirolmolk,_Shiraz,_Ir%C3%A1n,_2016-09-24,_DD_60-62_HDR.jpg, we would make a request for https://commons.wikimedia.org/wiki/File:Mezquita_de_Nasirolmolk,_Shiraz,_Ir%25C3%25A1n,_2016-09-24,_DD_60-62_HDR.jpg, which does not work.

Inside RN itself, RCTConvert has some fallback logic to try to accept both proper URLs and proto-URLs containing arbitrary characters that need to be percent-encoded. It looks like this:
https://github.com/facebook/react-native/blob/f8d6b97140cffe8d18b2558f94570c8d1b410d5c/React/Base/RCTConvert.m#L83-L97

I think the ideal fix would be to use RCTConvert itself, if that's possible, in order to support exactly the same set of uri values as RN's own Image component. See the original report of this issue, #129, which observes that non-ASCII characters work fine in RN's Image.

If that's not possible to use directly (I don't know enough about the internals of a library like this to know if it is), probably the next-best is to copy-paste that bit of logic from RCTConvert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Special characters
2 participants