fix: preserve :80 in FQDN when explicitly defined (fixes #7156) #7162
Conversation
WalkthroughThe FQDN attribute mutator in InstanceSettings has been reworked to explicitly handle URL schemes and ports. The new logic preserves non-standard ports, explicitly retains port 80 if present in the original input, and omits standard ports (80/443) when not explicitly provided, addressing a bug where port 80 was being stripped from generated URLs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Models/InstanceSettings.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
app/Models/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Keep Eloquent domain models in app/Models; models represent core entities (e.g., Application, Server, Service, Team, Project)
app/Models/**/*.php: Define $fillable for mass assignment protection in Eloquent models
Use $casts for attribute casting (e.g., arrays, enums, datetimes) in Eloquent models
Define relationships using Eloquent relation return types (e.g., BelongsTo, HasMany)
app/Models/**/*.php: Define Eloquent relationships using proper relationship methods with return type hints
Prefer Eloquent ORM and relationships over raw queries or DB:: calls
Prevent N+1 queries by using eager loading when appropriate
When creating new models, also create useful factories and seeders
Files:
app/Models/InstanceSettings.php
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions
**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies
**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)
Files:
app/Models/InstanceSettings.php
app/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Use database transactions to group related write operations for consistency in services/controllers/jobs
Files:
app/Models/InstanceSettings.php
app/Models/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Place domain models under app/Models
Files:
app/Models/InstanceSettings.php
app/Models/*.php
📄 CodeRabbit inference engine (.cursor/rules/database-patterns.mdc)
app/Models/*.php: When adding new database columns, update the model's $fillable array to allow mass assignment (e.g., for Model::create() and $model->update())
Use UUID primary keys via HasUuids for distributed systems
Enable soft deletes on models that require audit trails using SoftDeletes
Log model activity using the Spatie LogsActivity trait where auditing is required
Define explicit Eloquent relationships (belongsTo/hasMany/etc.) for navigable associations used by queries and eager loading
Files:
app/Models/InstanceSettings.php
🧬 Code graph analysis (1)
app/Models/InstanceSettings.php (2)
app/Models/Server.php (1)
port(822-829)docker/coolify-realtime/terminal-server.js (1)
host(209-209)
| if ($port && !in_array($port, [80, 443])) { | ||
| return "{$scheme}://{$host}:{$port}"; | ||
| } | ||
|
|
||
| return $url->getScheme().'://'.$host; | ||
| // explicitly keep :80 if user/template provided it | ||
| if ($port === 80 && str_contains($value, ':80')) { | ||
| return "{$scheme}://{$host}:{$port}"; | ||
| } | ||
|
|
||
| return "{$scheme}://{$host}"; | ||
| } | ||
| ); | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
Don't drop :443 for HTTP URLs.
As written, http://example.test:443 is rewritten to http://example.test, which breaks any template relying on that non-standard port. We only want to hide :443 for HTTPS defaults; for any other scheme (including HTTP) the explicit port must be kept.
- // keep :80 or :443 only if explicitly defined
- if ($port && !in_array($port, [80, 443])) {
- return "{$scheme}://{$host}:{$port}";
- }
-
- // explicitly keep :80 if user/template provided it
- if ($port === 80 && str_contains($value, ':80')) {
- return "{$scheme}://{$host}:{$port}";
- }
-
- return "{$scheme}://{$host}";
+ if ($port === null) {
+ return "{$scheme}://{$host}";
+ }
+
+ if ($scheme === 'https' && $port === 443) {
+ return "{$scheme}://{$host}";
+ }
+
+ if ($scheme === 'http' && $port === 80 && ! str_contains($value, ':80')) {
+ return "{$scheme}://{$host}";
+ }
+
+ return "{$scheme}://{$host}:{$port}";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Models/InstanceSettings.php around lines 63 to 75, the current logic
strips :443 for any scheme; change it to only omit default ports per scheme
(omit :80 only for http and omit :443 only for https). Update the initial
port-check so you include the port in the returned URL unless it is the scheme's
default (http+80 or https+443), and preserve explicit user/template-specified
ports (e.g., keep :80 if the original value contained ':80' and keep :443 for
http). Ensure the final return builds the URL without a port only when the port
equals the scheme's default and wasn’t explicitly present in the original
string.
|
This does not fix the issue. It is totally unrelated so I am closing it. |
2 participants
Fixes #7156
Problem:
Templates using port 80 (like Paymenter, GitLab, and Pterodactyl) were losing the :80 part in their URLs.
Example: http://example.com:80 became http://example.com.
Reason:
The fqdn() function in InstanceSettings was removing ports, keeping only the scheme and host.
Fix:
Updated fqdn() to:
Tested:
✅ Paymenter on port 80 → shows example.com:80
✅ GitLab on port 80 → shows example.com:80
✅ HTTPS and custom ports still work correctly
Summary by CodeRabbit