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

Serving images from endpoint routes no longer possible #1438

Closed
drzax opened this issue May 13, 2021 · 2 comments · Fixed by #1687
Closed

Serving images from endpoint routes no longer possible #1438

drzax opened this issue May 13, 2021 · 2 comments · Fixed by #1687
Labels
bug Something isn't working
Milestone

Comments

@drzax
Copy link

drzax commented May 13, 2021

Describe the bug

Prior to 1.0.0-next.82 it was possible to serve binary data from endpoint routes. Etag generation appears to cause an exception (str.charCodeAt is not a function) on subsequent versions (see #1136) and the fix in #1382, while preventing the exception also prevents serving binary data outside the narrow confines of a Content-Type: application/octet-stream http header.

To Reproduce

A minimum example which reproduces this issue (as provided in #1136 by @intrikate) is at https://github.com/intrikate/sveltekit-jpeg-endpoint-issue/blob/master/src/routes/index.jpg.js

Results:

Expected behaviour

The ability to serve binary data from endpoints without an exception or error. Obvious use case (as per the example above) is serving images, but more use cases are likely to exist.

Information about your SvelteKit Installation:

Diagnostics System: OS: macOS 10.15.7 CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz Memory: 3.37 GB / 32.00 GB Shell: 5.0.11 - /usr/local/bin/bash Binaries: Node: 14.15.1 - ~/.nave/installed/default/bin/node Yarn: 1.22.10 - ~/.nave/installed/default/bin/yarn npm: 7.6.3 - ~/.nave/installed/default/bin/npm Browsers: Chrome: 92.0.4503.0 Edge: 90.0.818.56 Firefox: 88.0.1 Firefox Developer Edition: 79.0 Safari: 14.1 npmPackages: @sveltejs/kit: next => 1.0.0-next.104 svelte: ^3.34.0 => 3.38.2

Using Firefox and the node adapter, but that's not relevant to this issue.

Severity
This makes it impossible to serve images from an endpoint route. For my use case (a utility for generating static fallback imagery for @abcnews interactive content) that will make it impossible to sveltekit for the task.

So I'd rate the severity high.

@Rich-Harris Rich-Harris added the bug Something isn't working label May 13, 2021
@benmccann benmccann added this to the 1.0 milestone May 13, 2021
@benmccann
Copy link
Member

There's a related suggestion for improving this in #1639

@coreh
Copy link
Contributor

coreh commented Jun 12, 2021

I'm having the same problem. As a workaround, I'm currently using patch-package to comment out the content-type check:

patches/@sveltejs+kit+1.0.0-next.115.patch

diff --git a/node_modules/@sveltejs/kit/dist/ssr.js b/node_modules/@sveltejs/kit/dist/ssr.js
index 32b08df..5313d2d 100644
--- a/node_modules/@sveltejs/kit/dist/ssr.js
+++ b/node_modules/@sveltejs/kit/dist/ssr.js
@@ -1269,17 +1269,17 @@ async function render_route(request, route) {
 			const type = headers['content-type'];

 			// validation
-			if (type === 'application/octet-stream' && !(body instanceof Uint8Array)) {
-				return error(
-					`Invalid response from route ${request.path}: body must be an instance of Uint8Array if content type is application/octet-stream`
-				);
-			}
-
-			if (body instanceof Uint8Array && type !== 'application/octet-stream') {
-				return error(
-					`Invalid response from route ${request.path}: Uint8Array body must be accompanied by content-type: application/octet-stream header`
-				);
-			}
+			// if (type === 'application/octet-stream' && !(body instanceof Uint8Array)) {
+			// 	return error(
+			// 		`Invalid response from route ${request.path}: body must be an instance of Uint8Array if content type is application/octet-stream`
+			// 	);
+			// }
+
+			// if (body instanceof Uint8Array && type !== 'application/octet-stream') {
+			// 	return error(
+			// 		`Invalid response from route ${request.path}: Uint8Array body must be accompanied by content-type: application/octet-stream header`
+			// 	);
+			// }

 			/** @type {import('types/hooks').StrictBody} */
 			let normalized_body;

I don't see any negative effects from removing the check. Etag generation seems to rely on the JS type of the body so if I understand it correctly that should work even without the header the check is enforcing.

#1639 mentions that header is used to check the response type later, but I couldn't find anywhere in the code that did that check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants