~cadence/tube-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH cloudtube] Proxy captions via new /proxy route

~lomanic
Details
Message ID
<161758234479.27477.3786424548421313502-0@git.sr.ht>
DKIM signature
missing
Download raw message
Patch: +36 -25
From: Lomanic <lomanic@hotmail.fr>

We can add more authorized paths to authorizedPaths if we need
more resources to be pulled from the NewLeaf/Invidious backend
on the same domain
---
 api/captions.js | 25 -------------------------
 api/proxy.js    | 31 +++++++++++++++++++++++++++++++
 api/video.js    |  5 +++++
 3 files changed, 36 insertions(+), 25 deletions(-)
 delete mode 100644 api/captions.js
 create mode 100644 api/proxy.js

diff --git a/api/captions.js b/api/captions.js
deleted file mode 100644
index 29611ef..0000000
--- a/api/captions.js
@@ -1,25 +0,0 @@
const fetch = require("node-fetch")
const {getUser} = require("../utils/getuser")
const constants = require("../utils/constants.js")

module.exports = [
	{
		route: `/api/v1/captions/(${constants.regex.video_id})`, methods: ["GET"], code: async ({req, fill, url}) => {
			const instanceOrigin = getUser(req).getSettingsOrDefaults().instance
			const fetchURL = new URL(`${url.pathname}${url.search}`, instanceOrigin)
			return fetch(fetchURL.toString()).then(res => {
				return res.text().then(text => {
					if (res.status === 200) {
						// Remove the position annotations that youtube unhelpfully provides
						text = text.replace(/(--> \S+).*/g, "$1")
					}
					return {
						statusCode: res.status,
						contentType: res.headers.get("content-type"),
						content: text
					}
				})
			})
		}
	}
]
diff --git a/api/proxy.js b/api/proxy.js
new file mode 100644
index 0000000..a0c8246
--- /dev/null
+++ b/api/proxy.js
@@ -0,0 +1,31 @@
const fetch = require("node-fetch")
const {getUser} = require("../utils/getuser")
const constants = require("../utils/constants.js")

// list of paths relative to the backend this route is authorized to serve
const authorizedPaths = [`/api/v1/captions/(${constants.regex.video_id})`]

module.exports = [
	{
		route: `/proxy`, methods: ["GET"], code: async ({req, fill, url}) => {
			const instanceOrigin = getUser(req).getSettingsOrDefaults().instance
			let remoteURL = url.searchParams.get("url")
			const fetchURL = new URL(remoteURL, instanceOrigin)
			if (!authorizedPaths.some(element => fetchURL.pathname.match(new RegExp(`^${element}$`)))) {
				return {
					statusCode: 401,
					content: "Unauthorized"
				}
			}
			return fetch(fetchURL.toString()).then(res => {
				return res.text().then(text => {
					return {
						statusCode: res.status,
						contentType: res.headers.get("content-type"),
						content: text
					}
				})
			})
		}
	}
]
diff --git a/api/video.js b/api/video.js
index 9ca4633..12a03dc 100644
--- a/api/video.js
+++ b/api/video.js
@@ -151,6 +151,11 @@ async function renderVideo(video, {user, settings, id, instanceOrigin}, locals =
		// rewrite description
		video.descriptionHtml = rewriteVideoDescription(video.descriptionHtml, id)

		// rewrite captions urls so they are served on the same domain via the /proxy route
		for (const caption of video.captions) {
			caption.url = "/proxy?url=" + encodeURIComponent(caption.url)
		}

		return render(200, "pug/video.pug", Object.assign(locals, {video, formats, subscribed, instanceOrigin}))
	} catch (e) {
		// show an appropriate error message
-- 
2.30.2
Details
Message ID
<20210410005810.740dce1b6c5ce6239869004e@disroot.org>
In-Reply-To
<161758234479.27477.3786424548421313502-0@git.sr.ht> (view parent)
DKIM signature
pass
Download raw message
> +			let remoteURL = url.searchParams.get("url")

To aid readability, let's use `const`. Naming the variables, e.g.,
`path` instead of `URL` would also help, when it is a path.

> +					statusCode: 401,
> +					content: "Unauthorized"

You need a `contentType`. Would `application/json` be appropriate?
`text/plain` would be acceptable too. Might be useful to make the
message something like `CloudTube: Unauthorised` to indicate to humans
that this response didn't hit the proxy. Otherwise it would be
ambiguous whether CT or NL gave this response.

> +			caption.url = "/proxy?url=" + encodeURIComponent(caption.url)

encodeURIComponent is not the same as a key/value pair in the URL.
Please use `x = new URL(); x.searchParams.set(...)`, or similar, to
build query strings.

> +			return fetch(fetchURL.toString()).then(res => {
> +				return res.text().then(text => {
> +					return {
> +						statusCode: res.status,
> +						contentType: res.headers.get("content-type"),
> +						content: text
> +					}
> +				})
> +			})

Please use `proxy` from `require("pinski/plugins")`. I may be
mistaken, but I think it should fit your needs. It also has the
advantage of streaming the response instead of buffering the whole
thing into memory. You can check here to see how it's used:
https://git.sr.ht/~cadence/cloudtube/commit/500aa820bce5d3439c57391e93e2fcfa29e318c1

Note that if this were to be used for video streams, you also need to
forward some headers like `Range` / `Accept-Ranges` / `Content-Range`.
Reply to thread Export thread (mbox)