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.

Sign up for GitHub

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

Jump to bottom

Chrome tries to load .../gitea/gitea/img/logo.png, Firefox loads .../gitea/img/logo.png #14422

Closed
2 of 6 tasks
uli-heller opened this issue Jan 22, 2021 · 7 comments · Fixed by #14827
Closed
2 of 6 tasks

Chrome tries to load .../gitea/gitea/img/logo.png, Firefox loads .../gitea/img/logo.png #14422

uli-heller opened this issue Jan 22, 2021 · 7 comments · Fixed by #14827
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug

Comments

@uli-heller
Copy link
Contributor

  • Gitea version (or commit ref): 1.14.0+dev-575-g1c230f69d
  • Git version: 2.25.1
  • Operating system: ubuntu-20.04
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
  • I'm using the context /gitea for my instance

Description

After clicking on the upper left logo, chrome tries to load several urls. There are two logo urls:

For firefox the error doesn't happen. Firefox does not try to load the PNG logo.

Screenshots

Chrome:

image

Firefox:

image
@uli-heller uli-heller mentioned this issue Jan 22, 2021
Closed
6 tasks
@lunny lunny added the type/bug label Jan 22, 2021
@silverwind
Copy link
Member

silverwind commented Jan 22, 2021 •
edited
Loading

The only place where we still use the PNG should be the meta tag <meta property="og:image" content="/img/logo.png" />. What does that tag's URL look like in your case?

Edit: Actually PNG is also used in manifest. Probably coming from there if you could post the base64 content in the manifest tag in <head>.

@uli-heller
Copy link
Contributor Author

I see these two pngs

$ curl -s https://hetzner-de.daemons-point.com/gitea|grep png
	<link rel="alternate icon" href="/gitea/img/favicon.png" type="image/png">
	<meta property="og:image" content="/gitea/img/logo.png" />

The SVG looks similar:

       <link rel="icon" href="/gitea/img/logo.svg" type="image/svg+xml">

Maybe for the PNG, og:url and og:image are contatenated somewhere somehow?

        <meta property="og:image" content="/gitea/img/logo.png" />
        <meta property="og:url" content="https://hetzner-de.daemons-point.com/gitea/" />

@uli-heller
Copy link
Contributor Author

The manifest looks like this:

       <link rel="manifest" href="data:application/json;base64,eyJuYW1lIjoiR2l0ZWE6IEdpdCB3aXRoIGEgY3VwIG9mIHRlYSIsInNob3J0X25hbWUiOiJHaXRlYTogR2l0IHdpdGggYSBjdXAgb2YgdGVhIiwic3RhcnRfdXJsIjoiaHR0cHM6Ly9oZXR6b
mVyLWRlLmRhZW1vbnMtcG9pbnQuY29tL2dpdGVhLyIsImljb25zIjpbeyJzcmMiOiJodHRwczovL2hldHpuZXItZGUuZGFlbW9ucy1wb2ludC5jb20vZ2l0ZWEvZ2l0ZWEvaW1nL2xvZ28ucG5nIiwidHlwZSI6ImltYWdlL3BuZyIsInNpemVzIjoiNTEyeDUxMiJ9LHsic3JjIj
oiaHR0cHM6Ly9oZXR6bmVyLWRlLmRhZW1vbnMtcG9pbnQuY29tL2dpdGVhL2dpdGVhL2ltZy9sb2dvLnN2ZyIsInR5cGUiOiJpbWFnZS9zdmcreG1sIiwic2l6ZXMiOiI1MTJ4NTEyIn1dfQ=="/>

or:

{
    "icons": [
        {
            "sizes": "512x512",
            "src": "https://hetzner-de.daemons-point.com/gitea/gitea/img/logo.png",
            "type": "image/png"
        },
        {
            "sizes": "512x512",
            "src": "https://hetzner-de.daemons-point.com/gitea/gitea/img/logo.svg",
            "type": "image/svg+xml"
        }
    ],
    "name": "Gitea: Git with a cup of tea",
    "short_name": "Gitea: Git with a cup of tea",
    "start_url": "https://hetzner-de.daemons-point.com/gitea/"
}

@silverwind
Copy link
Member

silverwind commented Jan 22, 2021 •
edited
Loading

Thanks. Pretty certain it's a bug in the manifest generation.

@silverwind silverwind added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jan 22, 2021
@uli-heller
Copy link
Contributor Author

I looked at modules/setting/setting.go and it seems that MakeAbsoluteAssetURL is called with
appURL: https://hetzner-de.daemons-point.com/gitea/, staticURLPrefix: /gitea
and it returns the concatenation of these...

uli-heller added a commit to uli-heller/gitea that referenced this issue Jan 22, 2021
@uli-heller
Fixes go-gitea#14422: Don't use sub-path twice for manifest
d7b7484
@techknowlogick techknowlogick mentioned this issue Jan 22, 2021
Closed
@lunny
Copy link
Member

lunny commented Feb 18, 2021

@uli-heller Could you try this patch

diff --git a/modules/setting/setting.go b/modules/setting/setting.go
index be7ec16e1..1f05bd7b6 100644
--- a/modules/setting/setting.go
+++ b/modules/setting/setting.go
@@ -1070,12 +1070,7 @@ func MakeAbsoluteAssetURL(appURL string, staticURLPrefix string) string {
        }

        if err == nil && parsedPrefix.Hostname() == "" {
-               if staticURLPrefix == "" {
-                       return strings.TrimSuffix(appURL, "/")
-               }
-
-               // StaticURLPrefix is just a path
-               return strings.TrimSuffix(appURL, "/") + strings.TrimSuffix(staticURLPrefix, "/")
+               return strings.TrimSuffix(appURL, "/")
        }

        return strings.TrimSuffix(staticURLPrefix, "/")

@uli-heller
Copy link
Contributor Author

@lunny Your patch looks good!

image

zeripath added a commit to zeripath/gitea that referenced this issue Feb 27, 2021
@zeripath
Prevent use of double sub-path and incorrect asset path in manifest
5c4342b 
MakeAbsoluteAssetURL should just url join the static url prefix on to appurl
if it is not an absolute path - this is because StaticURLPrefix is an absolute
prefix not a relative prefix to the app sub url.

Fix go-gitea#14422

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Feb 27, 2021
Merged
zeripath added a commit that referenced this issue Feb 28, 2021
@zeripath
Prevent use of double sub-path and incorrect asset path in manifest (#…
cf29cb3 
…14827)

MakeAbsoluteAssetURL should just url join the static url prefix on to appurl
if it is not an absolute path - this is because StaticURLPrefix is an absolute
prefix not a relative prefix to the app sub url.

Fix #14422

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
Milestone
No milestone
3 participants
@lunny @silverwind @uli-heller