Conversation
…y-hosts Add HTTP/3 support toggles for hosts
|
|
@toviszsolt Have not tested it yet, which is why it is still in draft mode. Will update you as soon as the PR is ready |
|
|
Hi, I tested this, I seem to be getting: I don't know if this error is printed anywhere reasonable, but it is saved in the database: I think you will have to add |
I've done this on the Just waiting for a successful build before merging to master. |
|
@jc21 |
Bumps the prod-patch-updates group in /frontend with 2 updates: [@tanstack/react-query](https://github.com/TanStack/query/tree/HEAD/packages/react-query) and [country-flag-icons](https://gitlab.com/catamphetamine/country-flag-icons). Updates `@tanstack/react-query` from 5.90.20 to 5.90.21 - [Release notes](https://github.com/TanStack/query/releases) - [Changelog](https://github.com/TanStack/query/blob/main/packages/react-query/CHANGELOG.md) - [Commits](https://github.com/TanStack/query/commits/@tanstack/react-query@5.90.21/packages/react-query) Updates `country-flag-icons` from 1.6.12 to 1.6.13 - [Changelog](https://gitlab.com/catamphetamine/country-flag-icons/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/catamphetamine/country-flag-icons/compare/v1.6.12...v1.6.13) --- updated-dependencies: - dependency-name: "@tanstack/react-query" dependency-version: 5.90.21 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: prod-patch-updates - dependency-name: country-flag-icons dependency-version: 1.6.13 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: prod-patch-updates ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps the dev-minor-updates group in /frontend with 2 updates: [@biomejs/biome](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome) and [happy-dom](https://github.com/capricorn86/happy-dom). Updates `@biomejs/biome` from 2.3.14 to 2.4.0 - [Release notes](https://github.com/biomejs/biome/releases) - [Changelog](https://github.com/biomejs/biome/blob/main/packages/@biomejs/biome/CHANGELOG.md) - [Commits](https://github.com/biomejs/biome/commits/@biomejs/biome@2.4.0/packages/@biomejs/biome) Updates `happy-dom` from 20.5.3 to 20.6.1 - [Release notes](https://github.com/capricorn86/happy-dom/releases) - [Commits](capricorn86/happy-dom@v20.5.3...v20.6.1) --- updated-dependencies: - dependency-name: "@biomejs/biome" dependency-version: 2.4.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: dev-minor-updates - dependency-name: happy-dom dependency-version: 20.6.1 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: dev-minor-updates ... Signed-off-by: dependabot[bot] <support@github.com>
|
Just testing this image, a few things going on.
|
|
Ok so there's a big problem as is. To replicate, enable http3 on any host. Then do it on another host. This doesn't like to be defined more than once across multiple server blocks. AI tells me:
If I'm being honest, this doesn't seem feasible given the current ecosystem of nginx config generation. |
…ertificates Fix uploading of custom certificates
…ndabot/npm_and_yarn/frontend/prod-patch-updates-95db6732c0 Bump the prod-patch-updates group in /frontend with 2 updates
…ndabot/npm_and_yarn/frontend/dev-minor-updates-d71d2fefd7 Bump the dev-minor-updates group in /frontend with 2 updates
|
Could we put |
|
Addressed the |
|
@jc21 Feel free to modify this PR and or move it to a branch on your repo. Depending how big the |
| listen 443 ssl reuseport; | ||
| listen [::]:443 ssl reuseport; |
There was a problem hiding this comment.
I think you should add a new line for each of the reuseport entries, I'm not sure what the implications and behaviour are with tacking it on the end of listen 443 ssl instead of listen 443 quic.
Unless you're sure this is fine.
There was a problem hiding this comment.
I am relying on the documentation of nginx and this says:
Syntax: listen address[:port] [default_server] [ssl] [http2 | quic] [proxy_protocol] [setfib=number] [fastopen=number] [backlog=number] [rcvbuf=size] [sndbuf=size] [accept_filter=filter] [deferred] [bind] [ipv6only=on|off] [reuseport] [so_keepalive=on|off|[keepidle]:[keepintvl]:[keepcnt]];
listen port [default_server] [ssl] [http2 | quic] [proxy_protocol] [setfib=number] [fastopen=number] [backlog=number] [rcvbuf=size] [sndbuf=size] [accept_filter=filter] [deferred] [bind] [ipv6only=on|off] [reuseport] [so_keepalive=on|off|[keepidle]:[keepintvl]:[keepcnt]];
listen unix:path [default_server] [ssl] [http2 | quic] [proxy_protocol] [backlog=number] [rcvbuf=size] [sndbuf=size] [accept_filter=filter] [deferred] [bind] [so_keepalive=on|off|[keepidle]:[keepintvl]:[keepcnt]];
Default:
listen *:80 | *:8000;
Context: server
Source: https://nginx.org/en/docs/http/ngx_http_core_module.html#listen
According to that, listen 443 ssl reuseport; should be correct. But I have not tested it
There was a problem hiding this comment.
Hi guys, I've written a more detailed comment here, I hope it will help: #5296 (comment)
There was a problem hiding this comment.
Yes it's syntactically correct, however SSL is TCP, while QUIC is UDP. So I don't think it'll be passed to the correct bind call.
There was a problem hiding this comment.
Yes it's syntactically correct, however SSL is TCP, while QUIC is UDP. So I don't think it'll be passed to the correct bind call.
Ah! Now I see what you are saying, and you are probably correct. The answer of @toviszsolt also supports your argument.
|
I have also done some research. Snippets would be needed for http3:
And it should be placed in the It is important that Stream Hosts do not allow the option to enable http3
Additional fine-tuning parameters for the http3 protocol: |
|
Thanks @toviszsolt this was very helpful. Tried to implement a logic, so that we have a different config for the first http server. I did not test the new image yet, so I am not sure if my change affects also the Stream servers. The SSL buttons get rendered in |
|
Docker Image for build 5 is available on DockerHub: Note Ensure you backup your NPM instance before testing this image! Especially if there are database changes. Warning Changes and additions to DNS Providers require verification by at least 2 members of the community! |
| */ | ||
| checkPrivateKey: async (privateKey) => { | ||
| const filepath = await tempWrite(privateKey, "/tmp"); | ||
| const filepath = await tempWrite(privateKey); |
There was a problem hiding this comment.
This change seems like a fair bug fix. The second parameter is optional, although lib does not specify where the temp file will be created. Reference: https://www.npmjs.com/package/temp-write
There was a problem hiding this comment.
The logic seems correct.
toviszsolt
left a comment
There was a problem hiding this comment.
I have reviewed all the changes and everything looks correct. I have left some comments on the modified files, which are just for information to help with the review process.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@7heMech Update: Please note that this PR does not affect Stream Hosts! |
This comment has been minimized.
This comment has been minimized.
|
@7heMech Related part of This PR omits Related part of Modified files related to frontend modals:
NOT modified files related to frontend modals:
|
I didn't say they have something in common... I'm saying we should keep reuseport here like you originally intended just in another manner of activation. |
|
@7heMech |
This comment has been minimized.
This comment has been minimized.
|
@7heMech, Like @toviszsolt said this doesn't effect Streams as that uses dedicated ports, whereas http3/quic is all port 443, and the Edit: Although I'm not sure what would happen if the |
Right, somehow I missed that... |
Added support for HTTP/3 since the OpenResty version supports it, so it was only a matter of adding it to the Nginx config templates and adding it to the frontend.
I am still testing, which is why this is a draft.