fix(security): prevent reflected xss #1200
Conversation
| @@ -35,8 +35,6 @@ class WCMP_API extends WCMP_Rest | |||
| */ | |||
| public function __construct($key) | |||
| { | |||
| parent::__construct(); | |||
There was a problem hiding this comment.
Waarom is deze weggehaald?
There was a problem hiding this comment.
De parent heeft geen __construct() meer omdat ie geen dingen meer hoeft te initialiseren voor de instance.
| @@ -559,7 +559,7 @@ public function addShipments(array $order_ids, bool $process): array | |||
| } catch (Exception $ex) { | |||
| $errorMessage = sprintf( | |||
| __('error_export_order_id_failed_because', 'woocommerce-myparcel'), | |||
| $order_id, __($ex->getMessage(), 'woocommerce-myparcel') | |||
There was a problem hiding this comment.
Wordt dit nu niet en nooit niet vertaald? Anders kunnen we deze ook door die esc_html__ halen denk ik om de vertaalfunctie voor de message niet kwijt te raken.
There was a problem hiding this comment.
Dit werd volgens mij nooit vertaald.
| @@ -15,102 +15,6 @@ | |||
| */ | |||
| class WCMP_Rest | |||
There was a problem hiding this comment.
Waarom zijn al deze functionaliteiten hier uit gehaald?
There was a problem hiding this comment.
Als je het curl gedeelte bedoelt: dat werd helemaal niet meer gebruikt, ten gunste van wp_remote_get en wp_remote_post, wat aanbevolen is.
| esc_html($date), | ||
| selected($date, $selected), | ||
| $dateString | ||
| esc_html($dateString) |
There was a problem hiding this comment.
Deze data zou volgens mij niet escaped hoeven worden
There was a problem hiding this comment.
Volgens mij ook niet, en dat geldt voor meer dingen, maar de WP waakhond wil dat wel echt.
| @@ -1375,7 +1374,7 @@ private function getConfirmationData(DeliveryOptions $deliveryOptions): ?array | |||
| ]; | |||
|
|
|||
| if (WCMYPA()->setting_collection->isEnabled(WCMYPA_Settings::SETTING_SHOW_DELIVERY_DAY)) { | |||
| $confirmationData[__('Date:', 'woocommerce')] = wc_format_datetime(new WC_DateTime($deliveryOptions->getDate() ?? '')); | |||
| $confirmationData[__('Date:', 'woocommerce-myparcel')] = wc_format_datetime(new WC_DateTime($deliveryOptions->getDate() ?? '')); | |||
There was a problem hiding this comment.
Klopt het dat deze vertaling nu niet meer uit Wooc zelf gehaald wordt? En daardoor misschien nu ontbreekt?
There was a problem hiding this comment.
Dat is correct en helaas. Aan de andere kant, op andermans vertaalbestanden dependen zonder dat expliciet te maken is ook niet zo netjes.
| exit($e); | ||
| } | ||
| return; | ||
| try { |
There was a problem hiding this comment.
Hier wordt ondersteuning voor Wooc < 2.7 gedropped?
There was a problem hiding this comment.
Ik zal inbouwen dat onze plugin minimaal versie 5.x moet hebben (dat is al zo vanwege een functie die de landen uitleest), dus de plugin werkte sowieso al niet met oudere versies, ook al declarede die dat niet netjes.
| // Remove placeholder values (IE8 & 9) | ||
| add_action('woocommerce_checkout_update_order_meta', [$this, 'remove_placeholders'], 10, 2); | ||
|
|
||
| // Fix weird required field translations |
There was a problem hiding this comment.
Waarom is dit verwijderd?
There was a problem hiding this comment.
Deze translations zijn niet meer van toepassing, heb ik gecontroleerd, WooC doet dat nu netjes. IE ondersteunen we helemaal niet meer.
| * @return string Normalized screen ID. | ||
| * @since 4.6.0-dev | ||
| */ | ||
| public static function normalize_wc_screen_id($slug = 'wc-settings') |
There was a problem hiding this comment.
Moet dit niet worden opgelost met de version check uit het gerefereerde ticket in dit docblock (ipv helemaal weggehaald)
There was a problem hiding this comment.
Hmtja, dit werkt ook gewoon, tegenwoordig.
| @@ -139,21 +139,10 @@ public static function set_props($object, $props, $compat_props = []) | |||
| */ | |||
| public static function set_address_prop(WC_Order $order, $prop, $address = 'billing', $value = null, $save = true) | |||
| { | |||
| if (WC_Core::is_wc_version_gte_3_0()) { | |||
There was a problem hiding this comment.
Hier wordt ook weer compatibiliteit verwijderd?
There was a problem hiding this comment.
Yes ik zal dit netjes doen.
INT-740