Ignore :443 in ROOT_URL when https:// is specified

[SaswatPadhi]

Description

Hello,

I have some automation in place that generates an initial configuration for Gitea and then spins up the instance. I have a couple of instances running on distinct ports. I noticed a spurious warning in one case.

When the following configuration was used:

ROOT_URL = https://redacted.doma.in:443/gitea/

I see the following warning (after successful installation):

Your ROOT_URL in app.ini is https://redacted.doma.in:443/gitea/ but you are visiting https://redacted.doma.in/gitea/admin
You should set ROOT_URL correctly, otherwise the web may not work correctly.

Most browsers today simply drop the default :443, so on the client side there is no way to suppress this error.

Of course, I edited my "app.ini" and removed :443, but would it make sense to drop :443 from an https ROOT_URL within Gitea? And may be similarly drop :80 from an http ROOT_URL?

Gitea Version

1.17.3

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

I am running 1.17.3-rootless image with Docker.

Database

No response

[KN4CK3R]

I would say that's a configuration "error" and Gitea should not try to fix that for the user.

[SaswatPadhi]

I am not sure if it's an error, because of the fact that the configuration works as expected. The warning issued by Gitea is spurious, IMO.

Just like https://xxx/ and https://xxx are equivalent, so are https://xxx:443 and https://xxx.

But to Gitea's ROOT_URL checking logic they are not, which is not a configuration issue, but a Gitea issue.

[SaswatPadhi]

I would suggest using a more robust URL comparison logic than a simple startsWith as done here:

gitea/web_src/js/features/common-global.js

Lines 358 to 360 in 43aafc5

	// some users visit "https://domain/gitea" while appUrl is "https://domain/gitea/", there should be no warning
	if (curUrl.startsWith(appUrl) || `${curUrl}/` === appUrl) {
	return;

Concretely, I would suggest using a URL normalization routine, perhaps https://github.com/sindresorhus/normalize-url, and do something like:

const normOptions = {
    normalizeProtocol: false,
    removeDirectoryIndex: true,
    removeQueryParameters: true,
    removeWWW: false,
    stripHash: true
};
if (normalizeUrl(curUrl, normOptions) === normalizeUrl(appUrl, normOptions))
    return;

The normalization also takes care of trailing slash.

The README for normalize-url states:

Port 443 is always removed from HTTPS URLs and 80 is always removed from HTTP URLs.

[wxiaoguang]

I have no motivation to make the code more complex (impact user's browser's performance and waste user's CPU), and I do not understand why you must write "https://domain.com:443/" in the config.

[zeripath]

@SaswatPadhi if you would like to create a PR to do this normalisation, the place to do it is here:

gitea/modules/setting/setting.go

Lines 756 to 764 in d7f12af

	AppURL = sec.Key("ROOT_URL").MustString(defaultAppURL + "/")
	// This should be TrimRight to ensure that there is only a single '/' at the end of AppURL.
	AppURL = strings.TrimRight(AppURL, "/") + "/"
	// Check if has app suburl.
	appURL, err := url.Parse(AppURL)
	if err != nil {
	log.Fatal("Invalid ROOT_URL '%s': %s", AppURL, err)
	}

Consider moving lines L758-L764 to a separate function, adjust the L758 to do your cleaning and normalisation as you wish, return the cleaned string AppURL and a parsed url back to the main function.

[SaswatPadhi]

Thanks @zeripath. Normalizing on server side certainly makes more sense.

I can take a stab at implementing the normalization using https://pkg.go.dev/github.com/PuerkitoBio/purell

[SaswatPadhi]

I realized that I don't actually need a 3rd-party library for normalization.

The RFC 3986 scheme-based normalization that I am proposing was actually being done a few lines above, but only on the defaultAppURL. For fixing this particular issue, it suffices to do the normalization on AppURL instead.