Load video from absolute path

[Trainmaster2]

I wanted to disguise my instance as a file-sharing site, but found that the video wouldn't load if the URL path had two or more segments. I found that the issue was that the video wasn't referenced as an absolute path in the generated HTML.

[modem7]

Heya, thanks for this, and for digging into the cause. Good catch, and the fix itself is spot on.

I can see a couple of issues though:

• CI will go red. Two checks in .github/workflows/test.yml look for the old relative URL:
  • grep -q 'source src="video.mp4"'
  • grep -q 'source src="custom.mp4"' (the container run with VIDEO_FILE=custom.mp4)
• The / gets added to whatever VIDEO_FILE is set to, not just bare filenames. Anyone already setting an absolute path or a full URL would end up with //video.mp4 or /https://example.com/v.mp4, which breaks. The README describes it as a filename relative to the web root, but those values work today, so it'd be a quiet break for anyone relying on them.

Getting it green

For the prefix, only add the / when the value doesn't already start with one and isn't a URL:

case "$VIDEO_FILE" in
    /*|*://*) ;;
    *) VIDEO_FILE="/$VIDEO_FILE" ;;
esac

For CI, swap the two checks to source src="/video.mp4" and source src="/custom.mp4".

It'd also be worth adding an assert for source src="/video.mp4" in the step that loads /some/random/path. That's the exact case you fixed, so it'd stop it quietly breaking again.

Something like:

--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -73,7 +73,7 @@ jobs:
           echo "$html" | grep -q '<title>Loading...</title>'
           echo "$html" | grep -qE '<video id="video"[^>]*\bautoplay\b'
           echo "$html" | grep -qE '<video id="video"[^>]*\bmuted\b'
-          echo "$html" | grep -q 'source src="video.mp4"'
+          echo "$html" | grep -q 'source src="/video.mp4"'
           echo "$html" | grep -q 'id="cookie-banner"'
           echo "$html" | grep -q 'id="site-error"'
           echo "$html" | grep -q 'id="loading-screen"'
@@ -91,6 +91,7 @@ jobs:
           [ "$status" = "404" ]
           html=$(curl -sS http://localhost:8080/some/random/path)
           echo "$html" | grep -q 'id="video"'
+          echo "$html" | grep -q 'source src="/video.mp4"'
 
       - name: Check container runs as non-root
         run: |
@@ -161,7 +162,7 @@ jobs:
           echo "$html" | grep -q 'height: 50vh'
           echo "$html" | grep -q 'width: 50%'
           echo "$html" | grep -q 'object-fit: contain'
-          echo "$html" | grep -q 'source src="custom.mp4"'
+          echo "$html" | grep -q 'source src="/custom.mp4"'
           if echo "$html" | grep -qE '<video id="video"[^>]*\bloop\b'; then
             echo "unexpected loop attribute present with LOOP=false"
             exit 1

Happy to push any of this myself if you'd rather, just shout.

[modem7]

I also noticed the auto-assign task has failed. This is unrelated to your change and I've fixed it in modem7/docker-rickroll#161. You don't need to do anything about it.

[Trainmaster2]

I can get to it later. Here I was thinking it was a simple fix. I'd be willing to bet the auto-assign had something to do with me making the PR from github.dev.

[modem7]

Hah, nothing is simple when CICD is involved!

But it's nothing to do with github.dev, and nothing you did wrong. It'd have happened with any PR from a fork unfortunately!

GitHub gives workflows on fork PRs a read-only token for safety, and the auto-assign job needs write access to assign me, so it got a 403. It was a gap in my workflow, shows how often I get PR's from others!