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

Fix crash on RESTAPI script invocation - Closes #4001 #4046

Merged
merged 5 commits into from
Dec 12, 2022
Merged

Conversation

JavierJF
Copy link
Collaborator

@JavierJF JavierJF commented Dec 8, 2022

This PR addresses several issues:

  • Crash reported in issue Crash on REST API script invocation #4001 due to invalid double call to close on file descritor.
  • Replace of the use of select in favor of poll to address the FD_SETSIZE limitation.
  • Enforce proper JSON formatting for each response of the RESTAPI endpoints, and for the formatting of input parameters in GET requests.
  • General improvement in testing of the RESTAPI.

This commit addresses several issues with 'wexecvp':
  - Fix invalid double call to 'close' in case fd was higher than
    'FD_SETSIZE'. This could lead to the invalidation of an unrelated
    fd resulting in asserts, as in issue #4001 and other instabilities.
  - Fix previous limitations of the legacy 'select 'impl that rendered
    the RESTAPI unusable when ProxySQL had more then 'FD_SETSIZE' fds
    opened.
  - Other minor improvements in function logic and interface.
This commit pack a couple of fixes and improvements for the RESTAPI:
  - Homogenization of GET/POST endpoint responses by ProxySQL. All
    responses should now be guaranteed to be valid JSON.
  - Fix JSON construction from parameters supplied to GET endpoint.
  - Add two new fields 'script_stdout' and 'script_stderr' to the
    JSON response when the target script fails to be executed. This is,
    when it exists with an error code other than zero. This makes the
    response homogeneous to when the scripts fails to produce a valid
    JSON output, and add extra information for debugging on client side.
  - Add a new debugging module 'PROXY_DEBUG_RESTAPI', to help tracing in
    debugging builds the requests to the endpoints.
- Fix several utility functions and add new ones.
- Improve current tests targeting RESTAPI.
- Add new regression test for issue #4001.
- Add several scripts used for RESTAPI tests rework.
- Fix compilation of several tests after 'wexcevp' interface change.
Sleeps have been increased for tests 'test_clickhouse_server-t' and
'test_sqlite3_server-t' in order to prevent potentials 'EADDRINUSE',
that could lead to client stalls when connecting to the interfaces.
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