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

Closed
opened 2025-11-02 08:51:31 -06:00 by GiteaMirror · 7 comments
Owner

Originally created by @SaswatPadhi on GitHub (Nov 19, 2022).

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

Originally created by @SaswatPadhi on GitHub (Nov 19, 2022). ### 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_
Author
Owner

@KN4CK3R commented on GitHub (Nov 19, 2022):

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

@KN4CK3R commented on GitHub (Nov 19, 2022): I would say that's a configuration "error" and Gitea should not try to fix that for the user.
Author
Owner

@SaswatPadhi commented on GitHub (Nov 19, 2022):

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 commented on GitHub (Nov 19, 2022): 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.
Author
Owner

@SaswatPadhi commented on GitHub (Nov 20, 2022):

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

43aafc5ba1/web_src/js/features/common-global.js (L358-L360)

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.

@SaswatPadhi commented on GitHub (Nov 20, 2022): I would suggest using a more robust URL comparison logic than a simple `startsWith` as done here: https://github.com/go-gitea/gitea/blob/43aafc5ba189efe9750326b21ee5a7c827929b75/web_src/js/features/common-global.js#L358-L360 Concretely, I would suggest using a URL normalization routine, perhaps https://github.com/sindresorhus/normalize-url, and do something like: ```js 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.
Author
Owner

@wxiaoguang commented on GitHub (Nov 26, 2022):

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.

@wxiaoguang commented on GitHub (Nov 26, 2022): 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.
Author
Owner

@zeripath commented on GitHub (Nov 26, 2022):

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

d7f12af805/modules/setting/setting.go (L756-L764)

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.

@zeripath commented on GitHub (Nov 26, 2022): @SaswatPadhi if you would like to create a PR to do this normalisation, the place to do it is here: https://github.com/go-gitea/gitea/blob/d7f12af805e33e327ec0c7a250ad8ed09a88e8fd/modules/setting/setting.go#L756-L764 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.
Author
Owner

@SaswatPadhi commented on GitHub (Nov 26, 2022):

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 commented on GitHub (Nov 26, 2022): 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
Author
Owner

@SaswatPadhi commented on GitHub (Nov 27, 2022):

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.

@SaswatPadhi commented on GitHub (Nov 27, 2022): 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.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#9857