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

New: comply with WP Plugin Check standards #899

Open
wants to merge 86 commits into
base: main
Choose a base branch
from

Conversation

alexmigf
Copy link
Member

@alexmigf alexmigf commented Oct 28, 2024

closes #950

Not everything has been resolved, but the plugin is now in a better state to meet WP Plugin Checks compliance.

Remaining Issues and Rationale for Not Fixing:

ERRORS

FILE: /home/alex/www/github-wpo/woocommerce-pdf-invoices-packing-slips/includes/Admin.php

322 21 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'str_replace'.
323 21 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'str_replace'.
666 21 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'str_replace'.
668 21 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'str_replace'.

Although we use wp_kses() with additional arguments, it converts attribute names to lowercase, causing issues with viewbox.

FILE: /home/alex/www/github-wpo/woocommerce-pdf-invoices-packing-slips/includes/Documents/BulkDocument.php

160 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found '$pdf'.

This is the PDF itself, and escaping could introduce unexpected issues.

165 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found '$this'.

This is the PDF HTML, and escaping could introduce unexpected issues.

FILE: /home/alex/www/github-wpo/woocommerce-pdf-invoices-packing-slips/includes/Documents/OrderDocument.php

1266 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found '$this'.

This captures the footer HTML in a buffer, not displayed to the user. It is sanitized later in footer().

1271 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.
1282 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.
1292 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.
1302 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.

We are using a custom sanitization function.

1407 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found '$pdf'.

This is the PDF itself, and escaping could introduce unexpected issues.

1412 14 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found '$this'.

This is the PDF HTML, and escaping could introduce unexpected issues.

1459 18 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks), found '$wp_filesystem'.

This is the UBL content, and escaping could introduce unexpected issues.

FILE: /home/alex/www/github-wpo/woocommerce-pdf-invoices-packing-slips/includes/Documents/OrderDocumentMethods.php

450 32 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.
1159 18 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.
1274 18 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found 'wpo_wcpdf_sanitize_html_content'.

We are using a custom sanitization function.

FILE: /home/alex/www/github-wpo/woocommerce-pdf-invoices-packing-slips/templates/Simple/html-document-wrapper.php

11 12 ERROR WordPress.Security.EscapeOutput.OutputNotEscaped Output should be escaped (see the WordPress Developer Handbook on security), found '$content'.

This is the PDF HTML content, and escaping could introduce unexpected issues.

WARNINGS

Warnings are mainly related to database usage, such as:

751 17 WARNING WordPress.DB.DirectDatabaseQuery.DirectQuery Direct database calls are discouraged.
751 17 WARNING WordPress.DB.DirectDatabaseQuery.NoCaching Direct database call without caching detected. Consider using wp_cache_get() / wp_cache_set() or wp_cache_delete().

To avoid impacting functionality, these will be addressed later in a new pull request.

@alexmigf
Copy link
Member Author

@YordanSoares @dwalkerpriv @dpeyou could you run some functional tests to ensure everything is working as expected? No need to review the code.

woocommerce-pdf-invoices-packingslips.php Outdated Show resolved Hide resolved
woocommerce-pdf-invoices-packingslips.php Outdated Show resolved Hide resolved
woocommerce-pdf-invoices-packingslips.php Outdated Show resolved Hide resolved
includes/Admin.php Outdated Show resolved Hide resolved
includes/Admin.php Outdated Show resolved Hide resolved
includes/Main.php Outdated Show resolved Hide resolved
includes/Main.php Outdated Show resolved Hide resolved
includes/Main.php Outdated Show resolved Hide resolved
includes/Documents/OrderDocument.php Outdated Show resolved Hide resolved
includes/Documents/OrderDocument.php Outdated Show resolved Hide resolved
@alexmigf alexmigf added this to the IPS Free: 3.9.1 milestone Dec 9, 2024
includes/Admin.php Show resolved Hide resolved
includes/Documents/SequentialNumberStore.php Show resolved Hide resolved
views/advanced-tools.php Show resolved Hide resolved
views/advanced-tools.php Show resolved Hide resolved
includes/Documents/OrderDocument.php Show resolved Hide resolved
includes/Documents/OrderDocument.php Show resolved Hide resolved
includes/Documents/OrderDocument.php Outdated Show resolved Hide resolved
includes/Documents/OrderDocument.php Show resolved Hide resolved
includes/Documents/OrderDocumentMethods.php Outdated Show resolved Hide resolved
includes/Tables/NumberStoreListTable.php Show resolved Hide resolved
@alexmigf alexmigf modified the milestones: 3.9.1, 3.9.2 Dec 11, 2024
@alexmigf alexmigf modified the milestones: 3.9.4, 3.9.5 Dec 22, 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.

Comply with WP Plugin Check
7 participants