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

DASH: Keep using redirected URL after 3xx response code #6907

Closed
jzhoubupt opened this issue Jan 23, 2020 · 10 comments
Closed

DASH: Keep using redirected URL after 3xx response code #6907

jzhoubupt opened this issue Jan 23, 2020 · 10 comments
Assignees

Comments

@jzhoubupt
Copy link

jzhoubupt commented Jan 23, 2020

[REQUIRED] Issue description

When play a DASH Live stream with the MPD file configured as permanently re-direct(http status code 301/308) ExoPlayer still use the original MPD URI to re-fresh, which doesn't follow DASH IF IOP recommendation.

[REQUIRED] Reproduction steps

Use the standard Exo Demo App play a DASH LIVE stream with the MPD file configured as permanently redirect to another URI. The actual ExoPlayer behaves in following sequence:

  1. Make HTTP Request with URI "http://xxx/content/index.mpd"
  2. Server returns http status code 308 with re-directed URI to "http://xxx/content/sessionID/index.mpd"
  3. ExoPlayer network module will use the new address to download mpd which is all correct
    ... after a few seconds to re-fresh the manifest file
  4. Make HTTP Request with URI "http://xxx/content/index.mpd"
    ...

The step 4 i'm afraid not following the HTTP re-direct and not follow the DASH IF IOP 4.3 chapter 3.2.15.3. Client Requirements and Recommendations:

If the HTTP request results in an HTTP redirect using a 3xx response code, the redirected URL replaces the original manifest URL

The source code in DashMediaSource::onManifestLoadCompleted is the place to handle this logic.
BTW, testing with DASH.js and Shaka-Player the re-direct handling is more aligned with DASH IOP.
I'm not quite sure if this is a BUG from your point of view. I open this as a bug mainly referring to DASH IOP spec.

[REQUIRED] Link to test content

The issue is found in production with commercial CDN and dynamic session management. Some CDN open a new session when client first requests MPD file and send back http re-direct to a new URI with sessionID there. When client doesn't follow the re-direct spec it causes CDN opening too many session and failed to server segment request.

[REQUIRED] A full bug report captured from the device

The issue is very clear and i belie you all understand the scenario.

[REQUIRED] Version of ExoPlayer being used

Test with 2.9.3 and 2.11.1 get the same behavior.

[REQUIRED] Device(s) and version(s) of Android being used

Issue can be observed from any devices.

@icbaker
Copy link
Collaborator

icbaker commented Jan 23, 2020

Thanks for the report, I'll let Olly take a look as he knows the most about DASH.

@tonihei
Copy link
Collaborator

tonihei commented Jan 23, 2020

We already have a pending internal feature request for this but didn't consider it as high-priority so far because repeated redirects still work. I'll mark this as an enhancement as well to track it publicly.

@tonihei tonihei changed the title DASH Live http re-direct support issue Keep using redirected URL after 3xx response code Jan 23, 2020
@jzhoubupt
Copy link
Author

Hi @tonihei ,
Thanks for your efficient feedback and confirmation.
Regards,
jzhoubupt

@ojw28
Copy link
Contributor

ojw28 commented Jan 27, 2020

I think we should fix this for DASH given the explicit recommendation to replace the URL in the DASH-IF guidelines. We do already support MPD.Location, in case that's helpful to you in the meantime.

@ojw28 ojw28 changed the title Keep using redirected URL after 3xx response code DASH: Keep using redirected URL after 3xx response code Jan 27, 2020
@jzhoubupt
Copy link
Author

Thanks @ojw28 . The fix is quite straight forward for this case.

@Stonos
Copy link

Stonos commented Mar 3, 2020

As a workaround until this is implemented (adding MPD.Location wasn't feasible in my case), I'm using the following OkHttp interceptor:

class RedirectCacheInterceptor : Interceptor {
    private val redirects = ConcurrentHashMap<String, String>(3)

    override fun intercept(chain: Interceptor.Chain): Response {
        val originalRequest = chain.request()
        val originalUrl = originalRequest.url
        val originalUrlHost = originalUrl.host

        val redirectedHost = redirects[originalUrlHost]
        if (redirectedHost != null) {
            Timber.d("Found cached redirect host for $originalUrlHost: $redirectedHost")
            return chain.proceed(getNewRequest(originalRequest, redirectedHost))
        }

        val response = chain.proceed(originalRequest)
        val newUrl = response.request.url
        val newUrlHost = newUrl.host
        val isRedirect = isRedirect(originalUrl, newUrl)

        if (isRedirect) {
            Timber.d("Redirect detected. Original host: $originalUrlHost New host: $newUrlHost")
            redirects[originalUrlHost] = newUrlHost
        }

        return response
    }

    fun clearCache() {
        redirects.clear()
        Timber.d("Redirect cache cleared")
    }

    private fun isRedirect(originalUrl: HttpUrl, newUrl: HttpUrl): Boolean {
        return originalUrl.host != newUrl.host
                && originalUrl.encodedPath == newUrl.encodedPath
                && originalUrl.query == newUrl.query
    }

    private fun getNewRequest(request: Request, newHost: String): Request {
        val newUrl = request.url.newBuilder()
                .host(newHost)
                .build()

        val newRequestBuilder = request.newBuilder().url(newUrl)
        return newRequestBuilder.build()
    }
}

@davibe
Copy link
Contributor

davibe commented Mar 6, 2020

If there is a 302, and "redirect stickiness" is enabled/expected but the manifest also has its own <Location>, who takes priority in determining the new url of the manifest ?

I assumed the http redirection takes precedence because the goal is for the CDN to avoid manifest rewrites.

I believe #1536 was the same "issue"

@ojw28
Copy link
Contributor

ojw28 commented Mar 9, 2020

If there is a 302, and "redirect stickiness" is enabled/expected but the manifest also has its own , who takes priority in determining the new url of the manifest ?

A server cannot respond with both a 302 and an MPD containing a Location element. It must do one or the other. If the MPD returned by the final server (after 3xx redirection) contains a <Location> element, then I would expect it to be used for the next request.

@ojw28
Copy link
Contributor

ojw28 commented Mar 11, 2020

@davibe - We made some changes when we merged this. Please check the latest dev-v2 still behaves as you expect. Thanks!

@ojw28 ojw28 closed this as completed Mar 11, 2020
@davibe
Copy link
Contributor

davibe commented Mar 11, 2020

I have seen it, thank you

@google google locked and limited conversation to collaborators May 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants