added optional request headers for remote assests (android & ios) #805

emrahdk · 2017-10-02T21:57:12Z

This pull request adds support for setting arbitrary Request Headers for resolving remote media. A use case could be to add an Authorization header for media that needs authentication. Therefore this PR should be able to close #475.

It can be used as demonstrated in the snippet below, by adding a headers object in the source property of the Video component.

<Video
  source={{
    uri: videoUrl,                        .
      headers: {
        Authorization: 'bearer some-token-value',
        'X-Custom-Header': 'some value'
      }
  }}
</Video>

It has been tested on iOS and Android.

Note: It was currently not possible to add the same functionality for Windows. System.Windows.Media.MediaPlayer only supports a single Uri class for remote/local media.

wja123 · 2017-10-07T07:01:02Z

This would be great, I can't wait till this is implemented.

ArtisanalPickle · 2017-12-08T17:08:46Z

Great stuff. Any update on this? Would love to see this merged.

sampurcell93 · 2017-12-13T18:24:12Z

Video.js

+    const strObj = {};
+
+    Object.keys(obj).forEach(x => {
+      strObj[x] = obj[x].toString();


This will set objects to [object Object] - do we want a check here for typeof obj[x] and to use JSON.stringify in those cases?

Yes you're correct - I was kinda of unsure whether to leave it or do a "smarter" string conversion. After your review, I choose the latter. 👍

Conversion is as follows:

Booleans, numbers, strings => prototype.toString

Objects and nulls => JSON.stringify'

Date => prototype.toISOString

undefined => returns empty string

sampurcell93 · 2017-12-14T20:11:12Z

Video.js

@@ -190,6 +200,7 @@ export default class Video extends Component {
        type: source.type || '',
        mainVer: source.mainVer || 0,
        patchVer: source.patchVer || 0,
+        requestHeaders: source.headers ? this.stringsOnlyObject(source.headers) : {}


I believe the key here should be headers and not requestHeaders. That or VideoViewManager.PROP_SRC_HEADERS needs to be changed to requestHeaders. Without these changes, this line in VideoViewManager:

src.getMap(PROP_SRC_HEADERS)

throws a runtime error due to the fact that 'headers' is not a key on 'src'.

Nice catch! 🙏 I've just pushed updates. I can't test it out on Android right now - it would be nice if you could verify it works. 🙌

Thanks for the reviews btw! 👍

ptfly · 2017-12-24T15:03:42Z

Thank you very much for this! Hope it gets merged asap 🥇

JohnGalt1717 · 2018-01-12T18:00:18Z

Yes please! I hope this gets merged ASAP. This is required for any AES Encrypted video playback.

Windows UWP should be possible as well because this is supported. There are samples from the Azure Media Services team.

WPF is the only one that should have issues.

emrahdk · 2018-01-13T20:48:19Z

@JohnGalt1717 could you point me in the direction of these samples? I'd love to add support for UWP too

kyle-ssg · 2018-01-16T15:15:54Z

+1

gouxlord · 2018-02-27T14:39:04Z

Awesome !

JohnGalt1717 · 2018-02-27T14:41:09Z

@emrah88 I believe it's here: https://github.com/Microsoft/Windows-universal-samples/tree/master/Samples/AdaptiveStreaming

I also have UWP code that works for AES.

You can use the Azure Media Player samples library to test the various types as well using that code. That will get you PlayReady, Fairplay, AES and that other one that I can never remember that Google uses.

gouxlord · 2018-02-27T15:40:44Z

@emrah88 I tested your fork on android and got this error, i tried some fixes but my android knowledge is low:

Error while updating property 'src' of a view managed by: RCTVideo

null

couldn't find key requestHeaders in dynamic object

Hope this help :)

cedbale · 2018-02-27T16:03:53Z

👍 should be great to have this feature merged soon :)

vwmoose · 2018-04-17T11:08:27Z

Any news on this being merged anytime soon?

davidsneal · 2018-04-17T11:14:18Z

Also in need of this!

sanchesking9 · 2018-04-26T12:16:57Z

@sampurcell93 sorry, when can you merge it? it's really needed feature

rterysen · 2018-04-27T13:06:32Z

Really need this!!! can I get a merge or a alternative?

kyle-ssg · 2018-04-27T13:46:21Z

You can of course can fork the pr in the meantime and point to that in your package.json, but yeah definitely needs merging

sanchesking9 · 2018-04-27T13:49:08Z

It has an error on android

Error while updating property 'src' of a view managed by: RCTVideo

null

couldn't find key requestHeaders in dynamic object

so fork couldn't help in this case

@emrah88, sorry, when can you fix it?

sampurcell93 · 2018-04-29T04:03:26Z

@sanchesking9 I do not have write permissions.

sanchesking9 · 2018-05-01T12:31:16Z

@isaiahgrey93 sorry, maybe you have write permissions??

emrahdk · 2018-05-04T20:04:19Z

Thanks for your feedback!! 🙏 🙏

I don't have that much Android experience. Sadly, this PR can't be merged as it is now since the Android player throws errors. No problems in iOS though, I am currently using it production.

I've tried to fix it, but I only get the first video playing. As soon as I try to load another video I get exceptions. I'll keep working on it though.

For you that are willing to try it out add the following to dependencies in package.json:
react-native-video: emrah88/react-native-video

emrahdk · 2018-05-04T20:07:07Z

@JohnGalt1717 cool, I'll look in to the AdaptiveStreaming samples

cobarx · 2018-05-29T22:09:29Z

ios/RCTVideo.m

  NSURL *url = (isNetwork || isAsset) ?
    [NSURL URLWithString:uri] :
    [[NSURL alloc] initFileURLWithPath:[[NSBundle mainBundle] pathForResource:uri ofType:type]];

  if (isNetwork) {
+    if ([headers count] > 0) {


This will ignore cookies if you opt to use headers. You should build up the options object so both are supported.

Ahh, good point! 🙌 I see it now 👏

cobarx · 2018-05-29T22:11:08Z

@emrah88 I can get this merged once the remaining issues get resolved. What sort of problems are you running into on Android, I should be able to help you get them sorted out. I left a note on the iOS code.

@sampurcell93 Thanks for helping review this. We could use more maintainers, if you'd like to help out lmk and I can get you added.

emrahdk · 2018-06-09T18:06:56Z

Hey @cobarx that sounds great! I should be able to give you some logcat output at Monday. The gist is: first video plays fine first time around, when selecting other video nothing happens.

emrahdk · 2018-06-09T21:29:57Z

android/src/main/java/com/brentvatne/react/ReactVideoView.java

@@ -254,7 +254,7 @@ public void setSrc(final String uriString, final String type, final boolean isNe
                    headers.putAll(toStringMap(mRequestHeaders));
                }

-                setDataSource(uriString);
+                setDataSource(mThemedReactContext, parsedUrl, headers);


In master (of the main repo) no headers are being passed to setDataSource. Even though the headers HashMap is being created (lines 247-251) and the cookies are added to it.

Is it me or was the headers never used?

Luckily setDataSource had an overload that accepted headers which I could pass them to.

emrahdk · 2018-06-09T21:33:19Z

I managed to get it working on Android now 🎉
... but I'm unsure about it's the right approach.

@cobarx could you have a look at my comment in the latest commit? (49cd5b6)

sanchesking9 · 2018-06-11T07:38:13Z

hi @emrah88
something happened with ios version, check it please

/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:55:22: warning: designated initializer invoked a non-designated initializer [-Wobjc-designated-initializers]
if ((self = [super init])) {
^
In file included from /Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:2:
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.h:29:1: note: method marked as designated initializer of the class here

(instancetype)initWithEventDispatcher:(RCTEventDispatcher *)eventDispatcher NS_DESIGNATED_INITIALIZER;
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:53:1: warning: designated initializer missing a 'super' call to a designated initializer of the super class [-Wobjc-designated-initializers]
(instancetype)initWithEventDispatcher:(RCTEventDispatcher *)eventDispatcher
^
In file included from /Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:2:
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.h:29:1: note: method marked as designated initializer of the class here
(instancetype)initWithEventDispatcher:(RCTEventDispatcher *)eventDispatcher NS_DESIGNATED_INITIALIZER;
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:333:31: error: unexpected '@' in program
[assetOptions setObject:@headers forKey:@"AVURLAssetHTTPHeaderFieldsKey"]
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:336:29: error: unexpected '@' in program
[assetOptions setObject:@cookies forKey:@AVURLAssetHTTPCookiesKey]
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:339:46: error: use of undeclared identifier 'asset'
return [AVPlayerItem playerItemWithAsset:asset];
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:364:27: warning: initializing 'NSString *__strong' with an expression of incompatible type 'id<NSObject,NSCopying> _Nullable'
NSString *value = item.value;
^ ~~~~~~~~~~
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:858:1: warning: method possibly missing a [super insertReactSubview:atIndex:] call [-Wobjc-missing-super-calls]
}
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:871:1: warning: method possibly missing a [super removeReactSubview:] call [-Wobjc-missing-super-calls]
}
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:14:17: warning: method override for the designated initializer of the superclass '-initWithFrame:' not found [-Wobjc-designated-initializers]
@implementation RCTVideo
^
In module 'UIKit' imported from /Users/user/www/GetAlertReact/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTConvert.h:9:
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.4.sdk/System/Library/Frameworks/UIKit.framework/Headers/UIView.h:150:1: note: method marked as designated initializer of the class here
(instancetype)initWithFrame:(CGRect)frame NS_DESIGNATED_INITIALIZER;
^
/Users/user/www/GetAlertReact/node_modules/react-native-video/ios/RCTVideo.m:14:17: warning: method override for the designated initializer of the superclass '-initWithCoder:' not found [-Wobjc-designated-initializers]
@implementation RCTVideo
^
In module 'UIKit' imported from /Users/user/www/GetAlertReact/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTConvert.h:9:
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.4.sdk/System/Library/Frameworks/UIKit.framework/Headers/UIView.h:151:1: note: method marked as designated initializer of the class here
(nullable instancetype)initWithCoder:(NSCoder *)aDecoder NS_DESIGNATED_INITIALIZER;
^
7 warnings and 3 errors generated.

emrahdk · 2018-06-11T09:53:12Z

@sanchesking9 thanks for the heads up!
I've had some syntax erros - which is fixed now.

cobarx · 2018-06-23T02:43:51Z

Ok I've tested this on all 3 platforms, made a few small fixes, and it's working great!

There are only 2 things I noticed that were worth mentioning. All the data is showing up in the Apache logs URL encoded. For example:

headers: {
  'X-HTTP-Object': { key: 'value' },
  'X-HTTP-Date': new Date()
}

becomes:

X-HTTP-Object:{"key"%3a"value"}|X-HTTP-Date:2018-06-23T02%3a27%3a58.302Z

I don't know if this is an issue or not as I don't have a web app setup for testing headers. I suppose I can throw something together pretty quick in PHP and check it.

Also, as a general coding convention I tend to like keeping the variable names the same throughout. Is there any reason we need to switch back and forth between headers and requestHeaders in the native code rather than picking one? It's pretty minor and I would be ok merging without changing working code.

I went ahead and added a note about changing the Android MediaPlayer setDataSource call so we can know what's up if someone reports an issue.

This thing is pretty much ready to merge. I also need to go through and document it, which I'll probably do in another PR.

@JohnGalt1717 If you have time to add UWP support that would be fantastic.

JohnGalt1717 · 2018-06-25T18:18:45Z

I'm burried at the moment but will see if I can get some bandwidth soon to take a look.

edersiebert · 2018-06-29T01:18:03Z

I need to pass a header with a video access authorization token, since I want to block the external access in .htaccess and open access only by the mobile phone through a token.

I'm using Android
but this is not working ...

source={ { uri: 'http://10.0.0.5:5000/50045454', headers: { Authorization: 'my token' } } }
in the logs I get only this

User-Agent: ReactNativeVideo/1.0 (Linux;Android 7.0) ExoPlayerLib/2.7.3 Connection: Keep-Alive Range: bytes=124042930- Host: 10.0.0.5:5000 Accept-Encoding: identity

cobarx · 2018-06-29T20:46:20Z

This feature isn't in 2.3.1, it's only available on the master branch. I'll be releasing 3.0 with it later today.

edersiebert · 2018-06-29T21:22:50Z

thank, very much
this update will help me a lot (:

emrahdk · 2018-07-07T12:15:08Z

@cobarx thanks for the awesome job and putting this together!

I'll look out for any mentions regarding the changes to setDataSource.

Regarding the Apache Logs, did you figure out something? I'll try do some request to my IIS server too see if I get the same encoding.

Regarding the naming of headers and requestHeaders, I agree completely with keeping the same names. But there was a reason for it which I can't remember. I'll do a PR soon to get them aligned again.

YoranRoels · 2018-09-28T09:19:31Z

Is there any sort of documentation for this? I can't seem to pass my Authorization header successfully.

cobarx · 2018-09-28T17:32:59Z

@YoranRoels I've added docs to the README. It only works out of the box on ExoPlayer, it's disabled by default on iOS because the code uses a private API that could get the app rejected from the App Store. The README has instructions for enabling it.

To troubleshoot, you can set breakpoints in the native code. Also, enable logging on your server to make sure the headers are arriving.

added optional request headers for remote assests (android & ios)

270fdfb

sampurcell93 reviewed Dec 13, 2017

View reviewed changes

sampurcell93 reviewed Dec 14, 2017

View reviewed changes

ptfly mentioned this pull request Dec 29, 2017

poster prop needs to reflect Image 'source' prop #887

Closed

emrahdk added 2 commits January 13, 2018 21:28

stringsOnlyObject does typechecking before coercion to string

57a16b2

constants renamed to requestheader

db1d83b

emrahdk added 2 commits January 15, 2018 16:15

ensuring datasource is build with headers

5c8e1bd

null pointer

12f93b5

Merge branch 'master' into master

fefbe80

cobarx reviewed May 29, 2018

View reviewed changes

emrahdk added 4 commits June 9, 2018 21:30

resolving conflicts

623b562

Merge branch 'master' into master

2d89a3f

respecting cookies when opting in for headers (ios)

f340025

passing headers to setDataSource when network video

49cd5b6

emrahdk commented Jun 9, 2018

View reviewed changes

fix syntax errors

6cd7dfe

cobarx added 4 commits June 22, 2018 18:46

Remove duplicate outdated stringsOnlyObject implementation

2649242

Merge branch 'master' into master

c67dd7b

Handle converting maps that are null or empty

bb2b3eb

Add note about IOException when using bigger setDataSource call

448e57b

cobarx mentioned this pull request Jun 23, 2018

Add headers back to android remote assets #1046

Closed

cobarx merged commit 5f1ce1a into TheWidlarzGroup:master Jun 25, 2018

ThomasCarca mentioned this pull request May 19, 2020

Headers supported for iOS #2012

Closed

jkirschnik12 mentioned this pull request Apr 10, 2024

[BUG]: Request headers don't work for Windows #3670

Closed

added optional request headers for remote assests (android & ios) #805

added optional request headers for remote assests (android & ios) #805

Conversation

emrahdk commented Oct 2, 2017

wja123 commented Oct 7, 2017

ArtisanalPickle commented Dec 8, 2017

sampurcell93 Dec 13, 2017

Choose a reason for hiding this comment

emrahdk Jan 13, 2018

Choose a reason for hiding this comment

sampurcell93 Dec 14, 2017

Choose a reason for hiding this comment

emrahdk Jan 13, 2018

Choose a reason for hiding this comment

ptfly commented Dec 24, 2017 • edited Loading

JohnGalt1717 commented Jan 12, 2018

emrahdk commented Jan 13, 2018 • edited Loading

kyle-ssg commented Jan 16, 2018

gouxlord commented Feb 27, 2018

JohnGalt1717 commented Feb 27, 2018 • edited Loading

gouxlord commented Feb 27, 2018

cedbale commented Feb 27, 2018

vwmoose commented Apr 17, 2018

davidsneal commented Apr 17, 2018

sanchesking9 commented Apr 26, 2018

rterysen commented Apr 27, 2018

kyle-ssg commented Apr 27, 2018 • edited Loading

sanchesking9 commented Apr 27, 2018

sampurcell93 commented Apr 29, 2018

sanchesking9 commented May 1, 2018

emrahdk commented May 4, 2018

emrahdk commented May 4, 2018

cobarx May 29, 2018

Choose a reason for hiding this comment

emrahdk Jun 9, 2018

Choose a reason for hiding this comment

cobarx commented May 29, 2018

emrahdk commented Jun 9, 2018

emrahdk Jun 9, 2018

Choose a reason for hiding this comment

emrahdk commented Jun 9, 2018

sanchesking9 commented Jun 11, 2018

emrahdk commented Jun 11, 2018

cobarx commented Jun 23, 2018

JohnGalt1717 commented Jun 25, 2018

edersiebert commented Jun 29, 2018

cobarx commented Jun 29, 2018

edersiebert commented Jun 29, 2018

emrahdk commented Jul 7, 2018

YoranRoels commented Sep 28, 2018

cobarx commented Sep 28, 2018

ptfly commented Dec 24, 2017 •

edited

Loading

emrahdk commented Jan 13, 2018 •

edited

Loading

JohnGalt1717 commented Feb 27, 2018 •

edited

Loading

kyle-ssg commented Apr 27, 2018 •

edited

Loading