-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: make federation address book sync work with allow_local_remote_servers = false #48451
fix: make federation address book sync work with allow_local_remote_servers = false #48451
Conversation
f4d1897
to
c7f8a9f
Compare
|
||
$options = [ | ||
'auth' => [$userName, $sharedSecret], | ||
'base_uri' => $url, | ||
'body' => $this->buildSyncCollectionRequestBody($syncToken), | ||
'headers' => ['Content-Type' => 'application/xml'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also add 'nextcloud' => ['allow_local_address' => true],
to always allow private IP ranges for address book federation. But we should also enable it for the initial handshake then, and therefore that's something for a follow-up.
c7f8a9f
to
a16eb6c
Compare
ClientTest is failing:
The above URL is invalid, and guzzle/curl will reject it with:
If we keep a16eb6c, then the notable change is that the request is just rejected a bit earlier. Though, it might be nicer to use a different/new exception for it. Opinions? |
a16eb6c
to
5576f05
Compare
…ervers = false Client.preventLocalAddress expects an absolute URL, which means the base_uri option cannot be used. Signed-off-by: Daniel Kesselberg <[email protected]>
This change should make it easier to spot wrong uses of the HTTP client on development setups where allow_local_remote_servers is usually true. Signed-off-by: Daniel Kesselberg <[email protected]>
5576f05
to
6be0043
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! 👍
/backport 8708164 to stable30 |
/backport 8708164 to stable29 |
Summary
Client.preventLocalAddress expects an absolute URL, which means the base_uri option cannot be used.
Regression from #46002
TODO
Checklist