Skip to content

Fix SSL certificate check errors by using the system's certificate authorities, if it's possible.#93

Merged
msyk merged 8 commits intomsyk:masterfrom
patacra:master
Sep 3, 2024
Merged

Fix SSL certificate check errors by using the system's certificate authorities, if it's possible.#93
msyk merged 8 commits intomsyk:masterfrom
patacra:master

Conversation

@patacra
Copy link
Copy Markdown
Contributor

@patacra patacra commented Sep 3, 2024

On the project we are working on, another developer received null to some $layout->query() calls to fetch data from our database. After some investigations with XDebug, I finally found out that his php.ini didn't have the cURL curl.cainfo set. It was blank. This was causing cURL to not validate the SSL certificate of our FileMaker server.

I downloaded a PEM file and set it with curl.cainfo = "C:\dev\php8.3\ssl\cacert.pem" and this solved the problem.

But this solution has some problems:

  • The cacert.pem file should ideally be updated from time to time.
  • Other developers might not know that this setting should be done to your PHP installation, and if it's not done, you get this kind of silent bug and don't know why it's happening.

Later on, I saw that cURL added a new way to use the system's certificate authorities files, which are updated by system updates, which is far safer and easier to keep up to date.

I added the code with backward compatibility in the callRestAPI() method of the communication provider.
In the same time, I simplified the if () {} else if () {} ... part for the different request methods by using a single if.

The other changes are just to improve a bit some type declarations and PHPDoc comments for the auto-complete and code linters. Hope I didn't do any mistake.

@patacra
Copy link
Copy Markdown
Contributor Author

patacra commented Sep 3, 2024

Oups, it seems that I made a mistake on the $data private member of FileMakerRelation. It's not an array but an object.
Sorry! I'll see if we can find a correct type for the @var declaration.

@msyk msyk merged commit 11d5585 into msyk:master Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants