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

Relative URIs with absolute paths are not resolved correctly #521

Closed
tedjp opened this issue Apr 21, 2015 · 4 comments · Fixed by #1024
Closed

Relative URIs with absolute paths are not resolved correctly #521

tedjp opened this issue Apr 21, 2015 · 4 comments · Fixed by #1024
Milestone

Comments

@tedjp
Copy link

tedjp commented Apr 21, 2015

A relative URI that starts with a / is not parsed correctly. It should replace the path component of its context, but in dash.js it is appended to the directory component of the context URL.

These relative URIs are specified in RFC 3986 as "path-absolute".

For example serving the MPD below from /MPDs/baseurl.mpd results in dash.js requesting /MPDs//path/at/root/video_900000bps.mp4 when it should be requesting /path/at/root/video_900000bps.mp4.

<?xml version="1.0"?>
<MPD xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xmlns="urn:mpeg:dash:schema:mpd:2011"
  xsi:schemaLocation="urn:mpeg:dash:schema:mpd:2011 DASH-MPD.xsd"
  type="static"
  minBufferTime="PT2S"
  profiles="urn:mpeg:dash:profile:isoff-live:2011"
  mediaPresentationDuration="PT234S">
  <Period>
    <BaseURL>/path/at/root/</BaseURL>
    <AdaptationSet mimeType="video/mp4" segmentAlignment="true" startWithSAP="1">
      <SegmentTemplate duration="2" startNumber="1"
         media="video_$Number$_$Bandwidth$bps.mp4"
         initialization="video_$Bandwidth$bps.mp4"/>
      <Representation id="v0" codecs="avc1.4d401e"
         width="720" height="576" bandwidth="900000"/>
    </AdaptationSet>
  </Period>
</MPD>

A similar test can be constructed with BaseURL elements in nested scope, each with an absolute path.

@TalLevAmi
Copy link

It seems that this issue was re-introduced by 2328620
The support for prepending the origin's host and protocol to the path-absolute urls has been removed.

@davemevans
Copy link
Contributor

Confirmed.

@davemevans davemevans reopened this Jul 6, 2016
@davemevans davemevans added this to the v2.3.0 milestone Jul 6, 2016
@davemevans
Copy link
Contributor

@TalLevAmi I believe davemevans@c7efeb6 should address this. Please could you try applying that and report back? If so, I'll create a PR.

@TalLevAmi
Copy link

@bbcrddave Confirmed it works. Thanks for the quick response!

dsparacio pushed a commit that referenced this issue Jul 6, 2016
Fix #521 - properly support path-absolute BaseURLs
marcoslhc added a commit to veo-televisa/dash.js that referenced this issue Jul 19, 2016
… into DC-7-create-test-cases-for-blim-env

* 'master' of https://github.com/Dash-Industry-Forum/dash.js: (97 commits)
  adding dist files to master
  Adding release notes to Reference player.
  Adding release notes to Reference player.
  Fix Dash-Industry-Forum#521 - properly support path-absolute BaseURLs
  Revert "For SegmentTemplate, only include available segments in DVRInfo"
  fix Dash-Industry-Forum#1463 - update documentation for setScheduleWhilePaused
  use sidx exclusively if there is an indexRange - thanks @bwidtmann
  Enable SegmentTimeline within SegmentList
  ManifestModel setValue should not trigger MANIFEST_LOADED when reset
  Update ISSUE_TEMPLATE.md
  Dash-Industry-Forum#1145 cleanup protection log
  Fix for issue Dash-Industry-Forum#887
  Removed build artifacts
  Reverted grunt changes
  Added support for filtering out key systems when no protection data for that key system is present
  fixed audio only buffer targets for longform etc...
  added missing comma
  fix for failing end test in browserstack
  Dash-Industry-Forum#1445 add a robustness level capabilities configuration for the protection controller. This is optional for Widevine Chrome to prevent robustness level warnings.
  remove check on required quality / check not needed anymore. See Dash-Industry-Forum#1443
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants