Skip to content
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

Transport\Curl/Fsockopen: add input validation #629

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 16, 2021

Transport\Curl/Fsockopen::request(): add input validation

This commit adds input validation to the Curl::request() and the Fsockopen::request() methods along the same lines as added to Requests::request().

  • The $url parameter is validated to allow for a string or Stringable (Iri) object.
  • The $headers parameter is validated to allow only an array for consistency between both transport classes.
  • The $data parameter already had (looser) input validation. This has been tightened up to only allow for the declared supported types, array and string (and null).
  • The $options parameter is validated to allow only an array - same as in Requests::request().

Includes:

  • Adding perfunctory tests for the new exceptions.
  • Adjusting one existing test to ensure that passing the $url as a stringable object is safeguarded.
  • Adjusting the existing tests for the $data parameter validation to be in line with the new validation.

Transport\Curl/Fsockopen::request_multiple(): add input validation

This commit adds input validation to the Curl::request_multiple() and the Fsockopen::request_multiple() methods along the same lines as added to Requests::request_multiple().

  • For $requests, arrays and iterable objects with array access should be accepted based on how the parameter is used within this class.
  • The $options parameter is validated to allow only an array - same as in Requests::request().

Includes aligning the behaviour when receiving an empty $requests array between the Curl and the Fsockopen classes and returning an empty array in that case.

Includes adding perfunctory tests for the new exceptions.


Regarding the other public methods in Curl:

  • While public, the get_subrequest_handle() and process_response() methods look to be intended for internal use only.
  • Same goes for stream_headers() and stream_body(). Note: these methods both take a $handle parameter which is subsequently unused. I imagine this was to allow for overloaded methods to receive the $handle, but the class has become final now (after investigation and finding no extended classes), so these methods can no be overloaded.
    I think it may be a good idea to deprecate the public methods in a future minor in favour of protected/private replacements which only receive the parameters which are actually use.
  • Lastly, there is the test() method for which the $capabilities parameter is only used after an isset() on the keys used, so this is safe as-is.
  • Other public methods do not take parameters.

Regarding the other public methods in Fsockopen:

  • The connect_error_handler() method is an error handler and should not be called directly.
  • While public, the verify_certificate_from_context() method looks to be intended for internal use only.
  • Lastly, there is the test() method for which the $capabilities parameter is only used after an isset() on the keys used, so this is safe as-is.

tests/Transport/BaseTestCase.php Outdated Show resolved Hide resolved
tests/Transport/BaseTestCase.php Outdated Show resolved Hide resolved
This commit adds input validation to the `Curl::request()` and the `Fsockopen::request()` methods along the same lines as added to `Requests::request()`.

* The `$url` parameter is validated to allow for a string or Stringable (Iri) object.
* The `$headers` parameter is validated to allow only an array for consistency between both transport classes.
* The `$data` parameter already had (looser) input validation. This has been tightened up to only allow for the declared supported types, array and string (and null).
* The `$options` parameter is validated to allow only an array - same as in `Requests::request()`.

Includes:
* Adding perfunctory tests for the new exceptions.
* Adjusting one existing test to ensure that passing the `$url` as a stringable object is safeguarded.
* Adjusting the existing tests for the `$data` parameter validation to be in line with the new validation.
This commit adds input validation to the `Curl::request_multiple()` and the `Fsockopen::request_multiple()` methods along the same lines as added to `Requests::request_multiple()`.

* For `$requests`, arrays and iterable objects with array access should be accepted based on how the parameter is used within this class.
* The `$options` parameter is validated to allow only an array - same as in `Requests::request()`.

Includes aligning the behaviour when receiving an empty `$requests` array between the `Curl` and the `Fsockopen` classes and returning an empty array in that case.

Includes adding perfunctory tests for the new exceptions.
@jrfnl jrfnl force-pushed the feature/transport-add-input-validation branch from 8190960 to f703215 Compare November 16, 2021 13:08
@schlessera schlessera merged commit 6ceca84 into develop Nov 16, 2021
@schlessera schlessera deleted the feature/transport-add-input-validation branch November 16, 2021 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants