ryanheise / just_audio

Audio Player
1.03k stars 652 forks source link

Recursively Apply Headers for Protected HLS Content (and other types of streaming) #470

Open djsjr opened 3 years ago

djsjr commented 3 years ago

Which API doesn't behave as documented, and how does it misbehave? When using signed cookies in the headers parameter to access protected HLS audio content (an .m3u8 url), just_audio throws an error: [VERBOSE-2:ui_dart_state.cc(199)] Unhandled Exception: Null check operator used on a null value Media is being accessed, or else I would receive an error indicating I do not have permission - such as if the cookie is in the wrong format or if there's a typo). The cookies work fine playing HLS video content using the httpsHeaders param with the flutter video player.

Minimal reproduction project Modified example main.dart I simply replaced setAudioSource with setURL and added an example map for the headers parameter.

To Reproduce (i.e. user steps, not code) Steps to reproduce the behavior:

  1. Use signed cookies with CloudFront to protect remote HLS content
  2. Put signed cookies (in the correct format with working policy, signature, key pair ID) into "headers" parameter of setURL.
  3. Run code
  4. See error

Error messages

[VERBOSE-2:ui_dart_state.cc(199)] Unhandled Exception: Null check operator used on a null value
#0      _ProxyHttpServer.start.<anonymous closure> (package:just_audio/just_audio.dart:1901:45)
#1      _ProxyHttpServer.start.<anonymous closure> (package:just_audio/just_audio.dart:1898:20)
#2      _rootRunUnary (dart:async/zone.dart:1362:47)
#3      _CustomZone.runUnary (dart:async/zone.dart:1265:19)
#4      _CustomZone.runUnaryGuarded (dart:async/zone.dart:1170:7)
#5      _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:341:11)
#6      _BufferingStreamSubscription._add (dart:async/stream_impl.dart:271:7)
#7      _SyncStreamControllerDispatch._sendData (dart:async/stream_controller.dart:733:19)
#8      _StreamController._add (dart:async/stream_controller.dart:607:7)
#9      _StreamController.add (dart:async/stream_controller.dart:554:5)
#10     _HttpServer._handleRequest (dart:_http/http_impl.dart:3209:19)
#11     new _HttpConnection.<anonymous closure> (dart:_http/http_impl.dart:2964<…>

occasionally another error follows shortly after:

[VERBOSE-2:ui_dart_state.cc(199)] Unhandled Exception: (-1008) resource unavailable
#0      AudioPlayer._load (package:just_audio/just_audio.dart:716:9)
<asynchronous suspension>
#1      AudioPlayer._setPlatformActive.setPlatform (package:just_audio/just_audio.dart:1096:28)
<asynchronous suspension>

Expected behavior Audio should play, having access to the protected media files.

Screenshots No screenshots needed

Desktop (please complete the following information):

Smartphone (please complete the following information):

Flutter SDK version Flutter 2.2.2

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 2.2.2, on macOS 11.4 20F71 darwin-x64, locale en-US)
[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.2)
[✓] Xcode - develop for iOS and macOS
[✓] Chrome - develop for the web
[✓] Android Studio (version 4.2)
[✓] VS Code (version 1.57.1)
[✓] Connected device (2 available)
    ! Error: My iPhone is not connected. Xcode will continue when My iPhone is connected. (code -13)

• No issues found!

Additional context The error appears to be happening in this function:

  Future start() async {
    _server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0);
    _server.listen((request) async {
      if (request.method == 'GET') {
        final uriPath = _requestKey(request.uri);
        final handler = _handlerMap[uriPath]!;
        handler(request);
      }
    });
  }

_requestKey(request.uri) returns a String ending in a '?', making it different from the key in the map.

ryanheise commented 3 years ago

[VERBOSE-2:ui_dart_state.cc(199)] Unhandled Exception: Null check operator used on a null value

0 _ProxyHttpServer.start. (package:just_audio/just_audio.dart:1454:45)

Hmm, I looked at that line number and didn't find anything. Was that error message captured from the same version that you've used in your minimal reproduction project?

djsjr commented 3 years ago

Whoops. Had an old version. Updated to 0.9.5. Edited my initial post to reflect correct the lines. (Same error occurring)

[VERBOSE-2:ui_dart_state.cc(199)] Unhandled Exception: Null check operator used on a null value
#0      _ProxyHttpServer.start.<anonymous closure> (package:just_audio/just_audio.dart:1901:45)
ryanheise commented 3 years ago

The documentation for HlsAudioSource states:

Currently headers are not applied recursively.

It's not the highest priority feature to implement now, but pull requests are always welcome if you or anyone else would like to help the project.

djsjr commented 3 years ago

Thanks for the quick reply. Good to know why it’s not working at least

djsjr commented 2 years ago

@ryanheise I suggest removing "bug" label and adding "enhancement". It would be great to see this implemented! Right now I can't play protected streamed audio from AWS Cloudfront - necessary to build a robust streaming app. Flutter video_player already seems to accomplish this if that helps in any way.

ryanheise commented 2 years ago

Are you sure you want me to do that? I treat bugs as higher priority than features ;-)

That said, I have a tonne of issues that are personally at a higher priority than this one on my todo list, so I would appreciate if those who want this feature themselves would consider themselves motivated to become a pull request contributor and help make that happen faster.

My own plans with respect to this are to actually find a way of implementing headers without the proxy which in turn will help to solve this issue in another way. That's not going to happen quickly either, but since I don't have time to do everything just as one person, my focus will be on this longer term vision. In the meantime, I welcome pull requests for a quicker solution that improves the header application of recursive requests in the proxy as long as it doesn't break existing apps.

djsjr commented 2 years ago

I have very little experience with native iOS, Android, and flutter plugin dev, but I may look into taking a look into it in the future! Thanks

ryanheise commented 2 years ago

One convenient thing is that this aspect of the plugin is written entirely in Dart (i.e. the proxy), so no knowledge of native programming is necessary.

djsjr commented 2 years ago

I didn’t know that - maybe I’ll take a crack at it 👍🏻

djsjr commented 2 years ago

I created this PR that is hopefully a suitable quick fix. Let me know if you see any problems

ryanheise commented 2 years ago

Thanks, @djsjr ! Apologies for introducing a merge conflict since I just bumped the version number for another fix. I'm just working through another bug fix first and then will review this. The fact that the diff is simple is promising, although I'd just want to be sure it doesn't break any other use cases for the proxy.

djsjr commented 2 years ago

No problem at all. And if it does break any other use cases, perhaps an if statement to check AudioSource type would suffice.

mt633 commented 2 years ago

I did some investigation on the Android side and to pass headers to ExoPlayer, you only need to add a line like this

DataSource.Factory httpDataSourceFactory = new DefaultHttpDataSource.Factory()
    .setUserAgent(userAgent)
    .setDefaultRequestProperties(headers) // <- This line, where [headers] is a Map<String, String>
    .setAllowCrossProtocolRedirects(true);

here: https://github.com/ryanheise/just_audio/blob/3db0316ed0cae71e0041e983f19eaa58e3eec8b0/just_audio/android/src/main/java/com/ryanheise/just_audio/AudioPlayer.java#L687-L693

I suppose there should be something similar for iOS.

Shouldn't it be a better option to use that instead of passing headers through a local HTTP proxy?

ryanheise commented 2 years ago

I have known about that since the beginning it but iOS doesn't have an equivalent aside from an undocumented key that is not supposed to be used. So the proxy is a common technique on iOS, and this implementation also gives consistency across platforms. That's not to say I won't use a native implementation in the future, but the iOS side is just a bit difficult if I want to implement that on the native side.

mt633 commented 2 years ago

I see. However, the current approach does not work for me. I've been attempting to use PR #568 without success. It works on a example file like this, but for some reason not on my own files.

ryanheise commented 2 years ago

Are you able to share some test media to reproduce this?

mt633 commented 2 years ago

Not really, I'm reluctant to exposing our API now that we are unable to work with token headers.

But I just discovered that it works with the PR if the files are served as is on our website. However, we have a PHP/Laravel API that serves the files upon request and when I use that method it fails. The return statement looks like this:

return response(file_get_contents($file_path), 200);

The ExoPlayer states that it is an EOFException, so I suspect it has something to do with how the data is returned or how it's processed after it is returned.

That said, just_audio works without any issues with our API setup if we don't use headers.

ryanheise commented 2 years ago

Are you able to create fake HLS content with the same structure as your internally used HLS content?

mt633 commented 2 years ago

Sure, here is a test audio file that uses the same FFMPEG conversion as our other files.

test.zip

ryanheise commented 2 years ago

Thanks, @mt633 , I have been able to create a test environment based on that file. I will start working on an implementation that attempts to parse the m3u8 file recursively.

I notice there is an HLS parser on pub.dev, although I'd prefer not to introduce another dependency, and for what we need, a full blown parser is not necessary.

If I understand the m3u8 format correctly, if I filter out everything after the #, and I then filter out blank lines, I'll be left with just the nested URLs. Is that correct? Then for each URL hosted on the same server, e.g. a relative path, I should propagate the headers.

mt633 commented 2 years ago

Great! Yes, that sounds right to me. Ideally, though, it should also be able to select a stream based on bandwidth from the content of the master.m3u8 file.

ryanheise commented 2 years ago

I think the platform player should take care of that since it would implement a full parser in and extract everything it needs to actually "play" the media. What I'm doing here is merely loading up the proxy with the right headers in advance for any URL that might need headers, should it get queried, but it's then up to the platform player to actually issue those queries.

mt633 commented 2 years ago

Oh I see, sounds like a good plan!

ryanheise commented 2 years ago

I have written up an implementation on the fix/hls_headers branch if you would like to test it.

ryanheise commented 2 years ago

The fix has been merged into minor.

mt633 commented 2 years ago

Just took a look and this fix works well for us. Thanks!

ryanheise commented 2 years ago

This fix has now been published (0.9.28).

mt633 commented 2 years ago

Great! The only thing is that I cannot get it to work in our live setting. It works well with the same audio files and the same code if I run it with a local web server using XAMPP, but not when I test it against our hosting service.

The debug log for Android says: ``` E/ExoPlayerImplInternal( 4374): Playback error E/ExoPlayerImplInternal( 4374): com.google.android.exoplayer2.ExoPlaybackException: Source error E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.ExoPlayerImplInternal.handleIoException(ExoPlayerImplInternal.java:641) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:613) E/ExoPlayerImplInternal( 4374): at android.os.Handler.dispatchMessage(Handler.java:102) E/ExoPlayerImplInternal( 4374): at android.os.Looper.loopOnce(Looper.java:201) E/ExoPlayerImplInternal( 4374): at android.os.Looper.loop(Looper.java:288) E/ExoPlayerImplInternal( 4374): at android.os.HandlerThread.run(HandlerThread.java:67) E/ExoPlayerImplInternal( 4374): Caused by: com.google.android.exoplayer2.upstream.HttpDataSource$HttpDataSourceException: java.io.IOException: unexpected end of stream on com.android.okhttp.Address@d413389 E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.open(DefaultHttpDataSource.java:365) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.DefaultDataSource.open(DefaultDataSource.java:258) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.StatsDataSource.open(StatsDataSource.java:84) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.DataSourceInputStream.checkOpened(DataSourceInputStream.java:99) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.DataSourceInputStream.open(DataSourceInputStream.java:62) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.ParsingLoadable.load(ParsingLoadable.java:174) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:412) E/ExoPlayerImplInternal( 4374): at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) E/ExoPlayerImplInternal( 4374): at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) E/ExoPlayerImplInternal( 4374): at java.lang.Thread.run(Thread.java:920) E/ExoPlayerImplInternal( 4374): Caused by: java.io.IOException: unexpected end of stream on com.android.okhttp.Address@d413389 E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.http.Http1xStream.readResponse(Http1xStream.java:203) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.http.Http1xStream.readResponseHeaders(Http1xStream.java:129) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.http.HttpEngine.readNetworkResponse(HttpEngine.java:750) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.http.HttpEngine.readResponse(HttpEngine.java:622) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.huc.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:475) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:411) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getResponseCode(HttpURLConnectionImpl.java:542) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.makeConnection(DefaultHttpDataSource.java:542) E/ExoPlayerImplInternal( 4374): at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.open(DefaultHttpDataSource.java:359) E/ExoPlayerImplInternal( 4374): ... 9 more E/ExoPlayerImplInternal( 4374): Caused by: java.io.EOFException: \n not found: size=0 content=... E/ExoPlayerImplInternal( 4374): at com.android.okhttp.okio.RealBufferedSource.readUtf8LineStrict(RealBufferedSource.java:202) E/ExoPlayerImplInternal( 4374): at com.android.okhttp.internal.http.Http1xStream.readResponse(Http1xStream.java:188) E/ExoPlayerImplInternal( 4374): ... 17 more E/AudioPlayer( 4374): TYPE_SOURCE: java.io.IOException: unexpected end of stream on com.android.okhttp.Address@d413389 I/ExoPlayerImpl( 4374): Release f2e343d [ExoPlayerLib/2.17.1] [emulator64_arm64, sdk_gphone64_arm64, Google, 32] [goog.exo.core, goog.exo.exoplayer, goog.exo.decoder, goog.exo.hls, goog.exo.datasource, goog.exo.extractor] ```

In other words, it says there is an unexpected end of stream. I've logged the file data that is returned and it looks normal to me:

The actual data returned ``` #EXTM3U #EXT-X-VERSION:6 #EXT-X-STREAM-INF:BANDWIDTH=105600,CODECS="mp4a.40.2" stream_0.m3u8 #EXT-X-STREAM-INF:BANDWIDTH=70400,CODECS="mp4a.40.2" stream_1.m3u8 ```
The file content ``` #EXTM3U #EXT-X-VERSION:6 #EXT-X-STREAM-INF:BANDWIDTH=105600,CODECS="mp4a.40.2" stream_0.m3u8 #EXT-X-STREAM-INF:BANDWIDTH=70400,CODECS="mp4a.40.2" stream_1.m3u8 ```

The only difference I've noticed is that there are some extra space characters in the data that is returned, but given that it works locally I feel like that can't be the issue here.

I really can't figure out what could be the issue here, so if you have any ideas I'd appreciate it!

ryanheise commented 2 years ago

I'll need a way to reproduce it in order to investigate it myself. But if you'd like to investigate it, you can try adding some logging in just_audio's proxy server. Since it's written in Dart, you might find it easy enough to tinker with, or at least add debug output to.

mt633 commented 2 years ago

I guess I could send you a test URL that you could use if there's any way to send a PM, but I'll start by trying to add some logging and see if I can find anything.

Speaking of logging, it would be nice to get more information by default about the server response in case of an error. Currently I can't see the HTTP status code or the error message that the server returns and handle errors differently based on that.

It might also be nice to have the option to get a more verbose output from the native players if that is possible.

mt633 commented 2 years ago

It fails on this line: https://github.com/ryanheise/just_audio/blob/0e8376f87be64bee016e919f90d2caf0c0f6490d/just_audio/lib/just_audio.dart#L3186

And here's the error:

HttpException (HttpException: Content size exceeds specified contentLength. 162 bytes written while expected 116. [#EXTM3U
#EXT-X-VERSION:6
#EXT-X-STREAM-INF:BANDWIDTH=105600,CODECS="mp4a.40.2"
stream_0.m3u8
#EXT-X-STREAM-INF:BANDWIDTH=70400,CODECS="mp4a.40.2"
stream_1.m3u8
]

Turns out that the content is 162 bytes and that my local server adds the Content-Length header correctly, but that the server on our hosting service returns 116 for some reason. I've tried different methods of setting the header and reading the content, but I always get the same issue with the incorrect length in the header.

I'll try to see if I can get help from the support at our hosting service to resolve this, but if you have any ideas I would of course appreciate it.

I suppose we don't want to ignore that header either in just_audio to ensure that everything is written correctly, right?

ryanheise commented 2 years ago

I see, yes the error reporting is a bit tricky there because we simply never expect anything to go wrong, except when HTTP 0.9 is being used.

Regarding the content length mismatch itself, I wonder if perhaps the server is auto-compressing the response, and perhaps Dart's HttpClient is by default decompressing the response. If that's the case, then the proxy should be rewritten to either not auto-decompress the response, or to rewrite the content-length header appropriately on the way out.

mt633 commented 2 years ago

You're right. I just found this comment which talks about the same type of issue. Adding the header Accept-Encoding: identity resolves the issue. Might perhaps be worth adding it as a default header?

ryanheise commented 2 years ago

I think the proxy should be rewritten to actually support compression, since I believe your original server configuration should be supported.

mt633 commented 2 years ago

Yes, that's probably better. Meanwhile I'll use the workaround I found.