fix(DnsPinning): Ensure to always lookup based on FQDN#59147
Open
DerDreschner wants to merge 1 commit intomasterfrom
Open
fix(DnsPinning): Ensure to always lookup based on FQDN#59147DerDreschner wants to merge 1 commit intomasterfrom
DerDreschner wants to merge 1 commit intomasterfrom
Conversation
DerDreschner
requested review from
ArtificialOwl,
icewind1991,
provokateurin and
salmart-dev
and removed request for
a team
Contributor
Author
|
/backport to stable32 |
DerDreschner
force-pushed
the
fix/use-fqdn-for-dns-pinning
branch
from
31e56ed to
d413f74
Compare
Contributor
Author
|
/backport to stable33 |
DerDreschner
force-pushed
the
fix/use-fqdn-for-dns-pinning
branch
from
d413f74 to
b6f77cb
Compare
Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
DerDreschner
force-pushed
the
fix/use-fqdn-for-dns-pinning
branch
from
b6f77cb to
aa45c07
Compare
provokateurin
requested changes
Mar 21, 2026
Member
provokateurin
left a comment
provokateurin
left a comment
There was a problem hiding this comment.
Uh that is quite something 😅
|
|
||
| // Before looking up any DNS record, we need to make sure the | ||
| // provided target is an FQDN by adding a dot to the end. | ||
| $target = str_ends_with($target, '.') ? $target : "$target."; |
Member
There was a problem hiding this comment.
Suggested change
| $target = str_ends_with($target, '.') ? $target : "$target."; | |
| if (!str_ends_with($target, '.')) { | |
| $target .= '.'; | |
| } |
kesselb
reviewed
Mar 21, 2026
|
|
||
| // Before looking up any DNS record, we need to make sure the | ||
| // provided target is an FQDN by adding a dot to the end. | ||
| $target = str_ends_with($target, '.') ? $target : "$target."; |
Contributor
There was a problem hiding this comment.
Would prefer rtrim($target, '.') . '.' ;)
Contributor
There was a problem hiding this comment.
Maybe through a small helper function enforceFdqn or such.
kesselb
approved these changes
Mar 21, 2026
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
3 participants
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.
Summary
After upgrading my private server from Debian 12 to Debian 13, I've experienced the same issue mentioned in #56489 which resulted in the app store to be totally unusable. Upgrading the Nextcloud version worked without any issues tho.
The investigation got the following results:
github.com, it tried to resolve A, AAAA and CNAME recordsgithub.com.dreschner.netFor reproducing the issue on command line level, you can use the following command on a server with IPv6 connectivity:
The reason is that we don't use FQDNs for resolving the DNS records and GitHub doesn't provide an AAAA record. Therefore, the program behind
dns_get_records()looks for the host as subdomain of the local domain name. This works as I have a wildcard AAAA record set-up.The fix is easy: just add a
.at the end to make the requested domain name a FQDN and prevent the lookup under the local domain name when no record is being found in the first lookup.Checklist
3. to review, feature component)stable32)AI (if applicable)