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

Content-Type does not respect optional charset encoding for iOS RCTJavascriptLoader #15791

Closed
justinr1234 opened this issue Sep 4, 2017 · 7 comments
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@justinr1234
Copy link
Contributor

justinr1234 commented Sep 4, 2017

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

  1. react-native -v: 0.42.0
  2. node -v: v6.11.2
  3. npm -v: 4.6.1
  4. yarn --version: 0.27.5
  • Target Platform: iOS
  • Development Operating System: macOS
  • Build tools: XCode

Steps to Reproduce

  1. Try to run a react native app and download a bundle from a server which returns a Content-Type header that includes both the MIME Type and an optional character encoding as specified here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type
  2. The React Native code for iOS will fail on this line of code as it strictly checks the Content-Type header as if the optional character encoding doesn't exist: https://github.com/facebook/react-native/blame/master/React/Base/RCTJavaScriptLoader.mm#L277

Expected Behavior

The code should follow the specification of Content-Type and respect only MIMEType checking instead of strictly checking the Content-Type header for an exact match on MIMEType.

Per Apple documentation, NSURLResponse contains the MIMEType field that should be checked for application/javascript instead of checking the raw Content-Type header: https://developer.apple.com/documentation/foundation/nsurlresponse

Actual Behavior

Though this was encountered while using Expo, the error is nonetheless coming from the RCTJavaScriptLoader

screen shot 2017-09-04 at 7 52 51 am

Expected JavaScript, but got content type 'application/javascript; chartset=UTF-8'. Check that your internet connection is working.

Reproducible Demo

https://github.com/sysgears/apollo-universal-starter-kit

Follow starter instructions and checkout this commit: sysgears/apollo-universal-starter-kit@bbcd52a

The problem is worked around after this commit in the project.

@joearasin
Copy link

I just tripped over this when attempting to get Haul to make a build for react-native 0.48.2: callstack/haul#228

@anp
Copy link
Contributor

anp commented Sep 5, 2017

cc'ing @janicduplessis as he was cc'd on an Expo bug report.

@grabbou
Copy link
Contributor

grabbou commented Sep 11, 2017

Up @janicduplessis, there are some commits in the history of this issue. Is any of them ready for merging? If yes, I can go ahead and cherry-pick into respective branches.

@janicduplessis
Copy link
Contributor

The PR has not been merged yet #15792, let's cherry pick it once it is.

facebook-github-bot pushed a commit that referenced this issue Sep 20, 2017
Summary:
To fix this issue: #15791
Closes #15792

Differential Revision: D5813101

Pulled By: shergin

fbshipit-source-id: fd3eb6f1d9ccdeb5373d1ba2b2df173ff7a8e986
@stale
Copy link

stale bot commented Nov 10, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 10, 2017
@justinr1234
Copy link
Contributor Author

@janicduplessis did this end up getting merged somewhere?

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 10, 2017
@stale
Copy link

stale bot commented Jan 9, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 9, 2018
@stale stale bot closed this as completed Jan 16, 2018
@facebook facebook locked and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

5 participants