git / git-scm.com

The git-scm.com website. Note that this repository is only for the website; issues with git itself should go to https://git-scm.com/community.
https://git-scm.com/
MIT License
2.17k stars 1.22k forks source link

404 on `/assets/errors.css` #1316

Closed peff closed 51 minutes ago

peff commented 5 years ago

If you visit a bogus page like https://git-scm.com/doesnotexist, you'll get a 404. But it refers to /assets/errors.css as a stylesheet. And we don't actually serve that. I'm not sure if that's intentional (and we need to upset the 404 page not to refer to it), or if something is broken in our asset pipeline. There is an assets/stylesheets/errors.scss in the repo, but that path does not work either.

AFAICT this isn't really hurting anything, but I'd like to clean it up and I'm not sure how. Anyone with rails expertise want to weigh in?

prasadgujar commented 5 years ago

@peffI @pedrorijo91 anyone working on this issue to resolve? if not i can take this issue.

pedrorijo91 commented 5 years ago

I think the issue is that we are referring to a css file that does not exist:

image

adsingh14 commented 2 years ago

Strange ! It's working on local server. Same with 500 error screen. 404

@peff I want to fix this issue but don't know how to provide relative path in an .html file ?

peff commented 2 years ago

I think the issue is that in the production site, we end up with asset files tagged with their hash, like:

~ $ ls -l public/assets/errors-*
-rw------- 1 u24297 dyno 4317 Dec  1 17:13 public/assets/errors-66c6a1ddb6367adb3d9600e2110cbb75c00453b5385854fedfd31926752e39ca.css
-rw------- 1 u24297 dyno 1504 Dec  1 17:13 public/assets/errors-66c6a1ddb6367adb3d9600e2110cbb75c00453b5385854fedfd31926752e39ca.css.gz
-rw------- 1 u24297 dyno 4114 May 19  2021 public/assets/errors-8310492f9e1ebc1fb483ac671943cd79ab2a81362e348c3bc6b5e4a6d910ec1b.css
-rw------- 1 u24297 dyno 1429 May 19  2021 public/assets/errors-8310492f9e1ebc1fb483ac671943cd79ab2a81362e348c3bc6b5e4a6d910ec1b.css.gz
-rw------- 1 u24297 dyno 4145 Nov 11 16:36 public/assets/errors-8c22f5bde2af7bffa6b370f8e5eaf35ecb60051e4ce272a3e4bf4ebb620b66c1.css
-rw------- 1 u24297 dyno 1438 Nov 11 16:36 public/assets/errors-8c22f5bde2af7bffa6b370f8e5eaf35ecb60051e4ce272a3e4bf4ebb620b66c1.css.gz

whereas in a development environment, it will process the scss on the fly as necessary from the source files. (But I'm pretty ignorant of rails asset pipeline stuff in general).

In the error view page we use some rails magic to get the right name: https://github.com/git/git-scm.com/blob/fcf5b8bcb7fad9a7e277739473925b4b26de9ec5/app/views/layouts/errors.html.erb#L15-L17

But the static 404 page just mentions the name directly: https://github.com/git/git-scm.com/blob/fcf5b8bcb7fad9a7e277739473925b4b26de9ec5/public/404.html#L8

Which kind of make sense. We aren't processing those files as templates, so they can't use the rails magic. We could make them into views, but especially for a 500, that feels like a chicken-and-egg problem (depending on what is broken, we may not be able to even render the 500 page properly). I wonder if we should just push all of the content in errors.scss into the actual static error pages themselves.

I also wonder if app/views/layouts/errors.html.erb is ever used at all. I don't see it mentioned anywhere after some grepping around, but I could be missing some place where rails implicitly loads the view.

adsingh14 commented 2 years ago

@peff I tested the bogus page and css didn't render as expected. It works on local space.

I was checking Ruby doc and reached to this section. As mentioned this is the preferred but insecure way for error pages.

Yes, we didn't utilise public/errors.html.erb which is secure way. Therefore, we've two ways to resolve this issue.

To1ne commented 6 hours ago

@peff This no longer seems to be an issue with Hugo, can we close?