Ensure the discovery endpoint is validated and all required fields ar… #750
Conversation
| @@ -63,12 +63,59 @@ public function __construct( | |||
| $this->crypto = $crypto; | |||
| } | |||
|
|
|||
| public function isDiscoveryEndpointValid($url) { | |||
| // Initialize cURL session | |||
There was a problem hiding this comment.
This should use https://docs.nextcloud.com/server/latest/developer_manual/digging_deeper/http_client.html, not plain curl
There was a problem hiding this comment.
Thanks Julius, refactored it
| } else { | ||
| // Set isReachable to false if http code wasn't 200 | ||
| $result['isReachable'] = false; | ||
| } |
There was a problem hiding this comment.
| } else { | |
| // Set isReachable to false if http code wasn't 200 | |
| $result['isReachable'] = false; | |
| } | |
| } |
No need for this else branch as isReachable is only set to true if the status code is 200 in line 86
| $result['isReachable'] = false; | ||
| } | ||
| } catch (Exception $e) { | ||
| $result['isReachable'] = false; |
There was a problem hiding this comment.
Same here, but still worth to catch and best log the exception here
nc-fkl
force-pushed
the
fix/validate-discovery-endpoint
branch
from
eb085e8
to
50e9a29
Compare
| $body = $response->getBody(); | ||
|
|
||
| // Check if the request was successful | ||
| if ($httpCode == 200 && !empty($body)) { |
There was a problem hiding this comment.
| if ($httpCode == 200 && !empty($body)) { | |
| if ($httpCode === Http::STATUS_OK && !empty($body)) { |
Use triple equal and use OCP\AppFramework\Http constants for HTTP response codes.
| } | ||
| } | ||
| } catch (Exception $e) { | ||
| $this->logger->error('Error - discovery endpoint validation: ' . $e); |
There was a problem hiding this comment.
| $this->logger->error('Error - discovery endpoint validation: ' . $e); | |
| $this->logger->error('Discovery endpoint validation error', ['exception' => $e]); |
So we get more info on the exception.
…e verified Signed-off-by: Florian Klinger <[email protected]>
Signed-off-by: Florian Klinger <[email protected]>
Signed-off-by: Florian Klinger <[email protected]>
julien-nc
force-pushed
the
fix/validate-discovery-endpoint
branch
from
50e9a29
to
ee13ccd
Compare
Signed-off-by: Julien Veyssier <[email protected]>
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
Ensure the discovery endpoint is validated and all required fields are verified