fix: prevent service domain FQDN corruption when ports are present#8980
Open
pannous wants to merge 1 commit intocoollabsio:nextfrom
Open
fix: prevent service domain FQDN corruption when ports are present#8980pannous wants to merge 1 commit intocoollabsio:nextfrom
pannous wants to merge 1 commit intocoollabsio:nextfrom
Conversation
Replace fragile string manipulation with existing getFqdnWithoutPort() helper. Fix strict type comparison in port matching. Fixes coollabsio#8798
There was a problem hiding this comment.
Pull request overview
This PR standardizes “FQDN without port” handling by reusing the existing getFqdnWithoutPort() helper in Docker Compose parsers, and prevents unnecessary port rewrites by comparing ports numerically.
Changes:
- Use
getFqdnWithoutPort()when derivingCOOLIFY_URL/ service URLs (instead of inline string slicing). - In service parsing, reuse the computed FQDN-without-port for
$urlto avoid duplicate transformations. - Compare URL ports as integers when enforcing predefined ports (to avoid string/int strict-compare mismatches).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| bootstrap/helpers/shared.php | Casts ports to int before comparing to avoid spurious port updates/saves. |
| bootstrap/helpers/parsers.php | Refactors URL-without-port generation to use getFqdnWithoutPort() across application/service parsing. |
You can also share your feedback on Copilot code review. Take the survey.
| @@ -2539,7 +2539,7 @@ function serviceParser(Service $resource): Collection | |||
| }); | |||
| $coolifyEnvironments->put('COOLIFY_FQDN', $fqdnsWithoutPort->implode(',')); | |||
| $urls = $fqdns->map(function ($fqdn): Stringable { | |||
| } elseif ($isServiceApplication) { | ||
| $fqdn = str($savedService->fqdn)->after('://')->before(':')->prepend(str($savedService->fqdn)->before('://')->append('://'))->value(); | ||
| $url = str($savedService->fqdn)->after('://')->before(':')->prepend(str($savedService->fqdn)->before('://')->append('://'))->value(); | ||
| $fqdn = getFqdnWithoutPort($savedService->fqdn); |
Member
Which version of Coolify has this issue? |
ShadowArcanist
added
the
💤 Waiting for feedback
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
When a one-click service has a predefined port (e.g., GitLab with port 80), saving the domain FQDN corrupts the URL from
http://git.example.com:80into//:80git.example.com.Three places in
parsers.phpuse fragile string manipulation (str()->after('://')->before(':')) to strip ports. This breaks when the URL structure is unexpected. The existinggetFqdnWithoutPort()helper inshared.phpalready handles this correctly usingSpatie\Url, so this PR replaces the inline code with calls to that helper.Also fixes a strict type comparison (
int !== string) in the old parser's port matching that caused unnecessary FQDN rewrites on every parse cycle.Issues
Category
Preview
N/A — backend-only fix, no UI changes.
AI Assistance
If AI was used:
getFqdnWithoutPort()implementation.Testing
getFqdnWithoutPort('http://git.example.com:80')returnshttp://git.example.com(uses Spatie\Url internally)getFqdnWithoutPort('git.example.com')falls back gracefully (returns input unchanged)(int) $env_port !== (int) $predefinedPortcorrectly compares80and"80"as equalContributor Agreement
Important