fix: should SSR render if accept header includes / #5234

SamVerschueren · 2023-09-28T14:02:30Z

Overview

If I start a Qwik app and use curl to make a GET request to the home page, Qwik returns a 404.

❯ curl -vv http://localhost:5173
*   Trying 127.0.0.1:5173...
* connect to 127.0.0.1 port 5173 failed: Connection refused
*   Trying [::1]:5173...
* Connected to localhost (::1) port 5173 (#0)
> GET / HTTP/1.1
> Host: localhost:5173
> User-Agent: curl/8.1.2
> Accept: */*
> 
< HTTP/1.1 404 Not Found
< Access-Control-Allow-Origin: *
< cache-control: max-age=5, stale-while-revalidate=604800
< Content-Security-Policy: default-src 'none'
< X-Content-Type-Options: nosniff
< Content-Type: text/html; charset=utf-8
< Content-Length: 139
< Date: Thu, 28 Sep 2023 13:53:22 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< 
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /</pre>
</body>
</html>

The reason is that curl uses Accept: */* by default, which is not caught by the middleware because shouldSsrRender() returns false in that case.

What is it?

Feature / enhancement
Bug
Docs / tests / types / typos

Description

See Overview

Use cases and why

When running curl -vv http://localhost:5173 to get the home page of your Qwik app, I expect that it returns the html instead of a 404. Currently that only works if you run curl -vv -H "Accept: text/html" http://localhost:5173.

Checklist:

My code follows the developer guidelines of this project
I have performed a self-review of my own code
I have made corresponding changes to the documentation
Added new tests to cover the fix / functionality

netlify · 2023-09-28T14:02:33Z

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	`6534d6c`

mhevery

Thank you for your contribution. ❤️

In #5234 we added support for requesting SSR with `curl` which defaults to `Accept: */*`. The problem with that is that most image requests will trigger SSR attempts. To fix this, special case the `*/*` and only SSR if it is the only one.

In #5234 we added support for requesting SSR with `curl` which defaults to `Accept: */*`. The problem with that is that most image requests will trigger SSR attempts. To fix this, special case the `*/*` and only SSR if it is the only one. Fix #5283

fix: should SSR render if accept header includes */*

6534d6c

mhevery approved these changes Sep 28, 2023

View reviewed changes

mhevery merged commit a79bd06 into QwikDev:main Sep 28, 2023
20 checks passed

mhevery mentioned this pull request Oct 13, 2023

fix(core): prevent SSR rendering on images #5313

Merged

7 tasks

This was referenced Oct 17, 2023

[🐞] library starter throws an error when trying to load @builder.io/qwik/build #5325

Closed

fix: @builder.io/qwik/build should not SSR render #5326

Merged

dmitry-stepanenko mentioned this pull request Nov 26, 2023

chore: upgrade qwik to latest qwikifiers/qwik-ui#525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: should SSR render if accept header includes / #5234

fix: should SSR render if accept header includes / #5234

SamVerschueren commented Sep 28, 2023

netlify bot commented Sep 28, 2023

mhevery left a comment

fix: should SSR render if accept header includes */* #5234

fix: should SSR render if accept header includes */* #5234

Conversation

SamVerschueren commented Sep 28, 2023

Overview

What is it?

Description

Use cases and why

Checklist:

netlify bot commented Sep 28, 2023

👷 Deploy request for qwik-insights pending review.

mhevery left a comment

Choose a reason for hiding this comment

fix: should SSR render if accept header includes / #5234

fix: should SSR render if accept header includes / #5234