Add full URLs in exercises to ensure they always load

[rtibbles]

Summary

Previously we were just replacing a relative path to origin for image URLs inside perseus json blobs. On some browsers these were not then properly resolved, resulting in images being missing.

This fixes that by always creating a full URL for replacing in the json blob. It attempts to construct this with a URL derived from the HTTP_REFERER header, which appears to be widely supported, as this will give the host that the frontend request this from, not the host that Kolibri is running on (in case Kolibri is behind a proxy).

It also uses this to more accurately set our Content Security Policy header, so that it still works even behind a poorly configured proxy.

Reviewer guidance

Check that Perseus images still work.
Check that they now work on older iOS.
See my tests, do they cover everything?

References

Fixes #1049

Contributor Checklist

PR process:

• PR has the correct target branch and milestone
• PR has 'needs review' or 'work-in-progress' label
• If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
• If this is an important user-facing change, PR or related issue has a 'changelog' label
• If this includes an internal dependency change, a link to the diff is provided

Testing:

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Gherkin stories have been updated
• Unit tests have been updated

Reviewer Checklist

• Automated test coverage is satisfactory
• PR is fully functional
• PR has been tested for accessibility regressions
• External dependency files were updated if necessary (yarn and pip)
• Documentation is updated
• Contributor is in AUTHORS.rst

[rtibbles]

Looks like my code is not Python3 compatible!

[codecov]

Codecov Report

Merging #4589 into release-v0.11.x will decrease coverage by 0.13%.
The diff coverage is 95%.

@@                 Coverage Diff                 @@
##           release-v0.11.x    #4589      +/-   ##
===================================================
- Coverage            51.92%   51.79%   -0.14%     
===================================================
  Files                  739      739              
  Lines                24385    24402      +17     
  Branches              3304     3230      -74     
===================================================
- Hits                 12661    12638      -23     
- Misses               10959    10983      +24     
- Partials               765      781      +16

Impacted Files	Coverage Δ
kolibri/core/content/views.py	92.85% <95%> (+10.76%)	⬆️
kolibri/utils/compat.py	82.85% <0%> (-17.15%)	⬇️
kolibri/core/analytics/pskolibri/common.py	66.17% <0%> (-10.3%)	⬇️
kolibri/core/apps.py	91.3% <0%> (-8.7%)	⬇️
kolibri/core/content/test/sqlalchemytesting.py	84% <0%> (-8%)	⬇️
kolibri/core/deviceadmin/utils.py	92.3% <0%> (-6.42%)	⬇️
kolibri/utils/env.py	87.5% <0%> (-6.25%)	⬇️
kolibri/core/analytics/utils.py	96.49% <0%> (-3.51%)	⬇️
kolibri/core/discovery/utils/filesystem/posix.py	63.82% <0%> (-3.2%)	⬇️
kolibri/plugins/coach/api.py	86.36% <0%> (-2.28%)	⬇️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 469d2ca...66dc942. Read the comment docs.

[codecov]

Codecov Report

Merging #4589 into release-v0.11.x will increase coverage by 0.07%.
The diff coverage is 95.23%.

@@                 Coverage Diff                 @@
##           release-v0.11.x    #4589      +/-   ##
===================================================
+ Coverage             51.9%   51.98%   +0.07%     
===================================================
  Files                  739      739              
  Lines                24385    24403      +18     
  Branches              3305     3308       +3     
===================================================
+ Hits                 12657    12685      +28     
+ Misses               10963    10953      -10     
  Partials               765      765

Impacted Files	Coverage Δ
kolibri/core/content/views.py	92.94% <95.23%> (+10.85%)	⬆️
...s/demo_server/assets/src/views/DemoServerIndex.vue	0% <0%> (ø)	⬆️
kolibri/core/assets/src/views/KModal.vue	73.21% <0%> (+7.14%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 469d2ca...2c19049. Read the comment docs.

[jamalex]

Nice! A couple small questions/tweaks, but otherwise looks good to go!

[benjaoming]

@rtibbles @indirectlylit

In #1049 it was mentioned:

we currently block access to Kolibri via iOS safari

Is this still active and should it then be unblocked?

[benjaoming]

@rtibbles Re: PR description

It also uses this to more accurately set our Content Security Policy header, so that it still works even behind a poorly configured proxy.

That is great, and it might become relevant pretty soon that we have an Nginx proxy in front of Kolibri, meaning that both localhost:80 and localhost:8080 might be active at the same time. We'll want of course to figure out all this such that eventually Kolibri only exists behind one port or that this is somehow configurable. Btw. not so related to the CSP... but do not assume the host name is ever fixed. Kolibri can be accessed from both http://localhost, http://1.2.3.4 and http://my.own.dns. We can probably use Django's ALLOWED_HOSTS setting in this relation, since it's solving the same type of issues.

[rtibbles]

Yeah, I have no intention of assuming the host name is fixed! Is there a better way than what I have done for systematically determining the host name via which the request was made?

[rtibbles]

Is this still active and should it then be unblocked?

I think there are a few other issues that have iOS Safari tags on them, we might want to double check those first.

[benjaoming]

Is there a better way than what I have done for systematically determining the host name via which the request was made?

I think that the HTTP_REFERER isn't necessary set in all cases. If someone just straight-up types an address. Or if applications like RACHEL are deep-linking to resources. Or in cases where the browser has blocked it.

Could request.META.get("HTTP_HOST") together with request.META.get("HTTPS") be fine?

We could also add a utility function in case that someone at some point desires more functionality? Like get_absolute_content_path(request, path), which could in some unknown scenario be pointing to a different location for content.

[benjaoming]

I think there are a few other issues that have iOS Safari tags on them, we might want to double check those first.

Hmm, but can the iOS reporter then actually test the builds in this PR?

[kollivier]

I was unable to reproduce #1046 and #2158 when I was testing on iOS, and both of those tickets are more than a year old so it's possible that changes to Kolibri fixed those issues. I haven't tested #2279 but I definitely don't feel like that's a blocker, and aside from this issue those are the only three that are listed for iOS / Safari.

I would vote for removing the block, and re-considering if we start seeing a lot of new error reports from iOS / Safari users.

[rtibbles]

Thanks for testing them, @kollivier! When I've fixed up this PR and merged, can remove Safari from the blacklist in a follow up PR.

[indirectlylit]

sounds good to me!

[indirectlylit]

@benjaoming

Hmm, but can the iOS reporter then actually test the builds in this PR?

yes we've determined that under the hood, chrome and safari are using the same webkit renderer

[benjaoming]

@rtibbles something has gone wrong, and absolute URLs are appended to their relative paths:

One last additional bit of info. Looking at the failed links found in the debugger interface, I see the resources that load correctly have a correctly formed URL. Whereas, those that don’t load have this duplicated frontend of the URL as shown here.

http://10.0.2.100:8080/learn/http://10.0.2.100:8080/zipcontent/eca85362c5a5c8370b08cae21edb69be.perseus/images/0041a87d6104cbbdb130f3b63baf4b57523e5acd.svg

Would you be able to add a regression test for this case, too?

[rtibbles]

Yeah, looks like this issue here is that Perseus does not expect the scheme to be included in the URL, will fix this up.

[rtibbles]

Could request.META.get("HTTP_HOST") together with request.META.get("HTTPS") be fine?

@benjaoming the issue with using HTTP_HOST is that it is not sent on the request, but added by Django, so this may differ from the external route that the frontend made the request to (so this would bypass any nginx or other proxy).

[rtibbles]

Have replicated the base issue again on Browserstack, using iPhone 6S, and have now confirmed a fix (needed to remove the http scheme from the generated URL).