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: do not allow to fall into infinite query loop on error #8

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

bart-jaskulski
Copy link
Contributor

Internally, esc_html calls database to find if site actually uses
utf8. This has some serious implications during our new database
installation (after hitting install button) because we land in
undetermined state.

  1. User hits install button.
  2. We are not installing database yet, just checking if it's available.
  3. We found out that db is not present (missing tables points that, but
    we collect errors).
  4. During error collection we call esc_html to sanitize text, which
    again calls get_options, resulting in another error being collected
    and prepared for output, which recursively fails.

The situation doesn't look better if we immediately enter installation
state (navigating directly to wp-admin/install.php). This ensures that
WP_INSTALLING constant is defined and introduces some safety checks,
but those merely limits to suppressing errors, yet those are still
collected in HTML format.

The easiest solution allowing us to properly install database would be
to drop formatting function because of it's dependencies in favour of
native PHP function. Nevertheless, it might be reconsidered whether we
actually need HTML output and such sanitization, especially when
sometimes wpdb error outputs encoded HTML, resulting in illegible wall
of plain HTML (not parsed by browser). Following original wpdb class
implementation, we might opt-in for concise error messages, which
doesn't require complex formatting (especially within database driver
logic).

Signed-off-by: Bart Jaskulski [email protected]

Internally, `esc_html` calls database to find if site actually uses
utf8. This has some serious implications during our new database
installation (after hitting *install* button) because we land in
undetermined state.

1. User hits *install* button.
2. We are not installing database yet, just checking if it's available.
3. We found out that db is not present (missing tables points that, but
   we collect errors).
4. During error collection we call `esc_html` to sanitize text, which
   again calls `get_options`, resulting in another error being collected
   and prepared for output, which recursively fails.

---

The situation doesn't look better if we immediately enter installation
state (navigating directly to `wp-admin/install.php`). This ensures that
`WP_INSTALLING` constant is defined and introduces some safety checks,
but those merely limits to suppressing errors, yet those are still
collected in HTML format.

The easiest solution allowing us to properly install database would be
to drop formatting function because of it's dependencies in favour of
native PHP function. Nevertheless, it might be reconsidered whether we
actually need HTML output and such sanitization, especially when
sometimes wpdb error outputs encoded HTML, resulting in illegible wall
of plain HTML (not parsed by browser). Following original `wpdb` class
implementation, we might opt-in for concise error messages, which
doesn't require complex formatting (especially within database driver
logic).

Signed-off-by: Bart Jaskulski <[email protected]>
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