Fix/plugin checks #2654
Fix/plugin checks #2654
Conversation
…eneration vs. getting when not necessary. Removing commented code.
There was a problem hiding this comment.
Mostly minor stuff and some cases where I'd like to double check that we're sure there isn't supposed to be HTML in things we're using esc_html on. A few cases of CSS/style code that I believe breaks when escaped sometimes.
Some places where you used esc_attr__, where I would have used esc_html__. These functions are literally the same, except for the filter applied. But good to only use the attribute one for html attributes as far as I know.
includes/admin/reporting/tabs/class.llms.admin.reporting.tab.courses.php
Outdated
Show resolved
Hide resolved
includes/admin/reporting/tabs/class.llms.admin.reporting.tab.memberships.php
Outdated
Show resolved
Hide resolved
includes/admin/reporting/tabs/class.llms.admin.reporting.tab.students.php
Outdated
Show resolved
Hide resolved
includes/admin/reporting/tabs/class.llms.admin.reporting.tab.students.php
Outdated
Show resolved
Hide resolved
| @@ -327,18 +324,16 @@ public function get_table_html( $rows ) { | |||
|
|
|||
| ob_start(); | |||
| ?> | |||
| <table style="<?php echo $table_style; ?>"> | |||
| <table style="<?php echo esc_attr( $table_style ); ?>"> | |||
There was a problem hiding this comment.
How can we test this. I feel these styles and content may include things broken by the escaping functions.
| data-handler="<?php echo $this->get_handler(); ?>" | ||
| id="llms-gb-table-<?php echo $this->id; ?>" | ||
| class="<?php echo esc_attr( implode( ' ', $classes ) ); ?>" | ||
| data-args='<?php echo esc_attr( wp_json_encode( $this->get_args() ) ); ?>' |
There was a problem hiding this comment.
We should be good to esc_attr this even though wp_json_encode() should be safe
There was a problem hiding this comment.
Agreed. esc_attr and esc_html don't seem to interfere with the json as used here and similar places.
Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
…tudents.php Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
…tudents.php Co-authored-by: Jason Coleman <33220397+ideadude@users.noreply.github.com>
| <?php if ( $selected_gateway->get_icon() ) : ?> | ||
| <span class="llms-gateway-icon"><?php echo $selected_gateway->get_icon(); ?></span> | ||
| <span class="llms-gateway-icon"><?php echo wp_kses_post( $selected_gateway->get_icon() ); ?></span> |
There was a problem hiding this comment.
These changes or something up the chain are breaking some of the checkout functionality. The icons beside gateways don't appear (ie. credit card images, "powered by stripe" for credit card, and the first gateway isn't checked by default (but is bolded)
There was a problem hiding this comment.
Adding additional tags and attributes to the LLMS_Form_Field::render() array for wp_kses() seems to have solved the output. It's part of the icon property on the form field which gets combined into the description, and it was being stripped out (img and an a tag)
… rather than trusting all callees to the method to have things escaped already.
Description
Various security warning/error fixes.
How has this been tested?
Manually
Checklist: