-
Notifications
You must be signed in to change notification settings - Fork 6.3k
nginx config minor bug fixed #4312
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
Conversation
without ^~, resources in folders like /static cannot be accessed.
@bpmct or @code-asher could you review this? I haven't used nginx so not sure I can help much here. |
Codecov Report
@@ Coverage Diff @@
## main #4312 +/- ##
==========================================
+ Coverage 68.09% 68.22% +0.12%
==========================================
Files 31 31
Lines 1586 1586
Branches 308 308
==========================================
+ Hits 1080 1082 +2
+ Misses 432 430 -2
Partials 74 74
Continue to review full report at Codecov.
|
I am not sure if we should add this. ^~ means that if this is the
longest matching prefix string (meaning not a regular expression match)
then skip checking regular expressions matches.
In the configuration we provide there are no other location blocks so
this is not needed. Is there an example use case for where this is
necessary?
|
without |
It's always better IMO to avoid using regular expressions as they are harder to maintain (harder for developers to read/understand) and can also lead to reDOS vulnerabilities. If we can duplicate a block in the config to achieve the same behavior, I think that's a preferable solution |
okay, I changed by duplicating the block for |
Good call! @code-asher I'll let you give the final approval here |
I still do not understand what this is fixing. The default / location
block will handle /static.
You can verify the configuration works as-is using Docker with
these steps:
```
$ docker run --rm -it -p 8080:80 ubuntu bash
$ apt update && apt install curl nginx
$ curl -fsSL https://code-server.dev/install.sh | sh
$ code-server --auth none &
$ cat > /etc/nginx/sites-available/default << EOF
server {
listen 80 default_server;
listen [::]:80 default_server;
server_name _;
location / {
proxy_pass http://localhost:8080/;
proxy_set_header Host \$host;
proxy_set_header Upgrade \$http_upgrade;
proxy_set_header Connection upgrade;
proxy_set_header Accept-Encoding gzip;
}
}
EOF
$ nginx
$ tail -f /var/log/nginx/access.log # Just to make sure nginx is being used.
```
From here I browsed to localhost:8080 and confirmed everything loads
correctly including /static, /vscode-remote-resource, etc.
Would you be able to provide a reproduction of the problem this is
solving? Thank you!
|
It works in localhost address
Now I use the similar conf on my domain (from
Then:
Now I add
Then:
I tested the duplicated one (too long so I didn't added to this paragraph), it just didn't work. So basically |
Thank you for the additional details.
I tried to replicate using a domain instead of localhost but everything
still works normally for me. Is that your full config? It seems likely
there is something in your config that is interfering with the static
endpoint (maybe in the security.conf or general.conf files).
|
@SnorlaxYum were you able to try @code-asher's suggestion? Hoping we can either close or move forward! |
@SnorlaxYum we haven't heard from you in a couple days so I'm going to assume you fixed your issue. I'm going to close this PR. If you want to discuss getting this in, feel free to open an issue and we'll work with you! |
without
^~
, resources in folders like/static
cannot be accessed.So I do this PR. It's a minor one in the doc. So I think there's no need to create an issue for it.