Ad_Layers_DFP::sanitize_key() is stripping valid Ad Unit code characters

[onpubcom]

Ad_Layers_DFP::sanitize_key() strips valid Ad Unit code characters such as: '-', '_', '*", etc. Per the DFP docs here is the full list of supported characters:

Ad unit codes
Ad unit codes can be up to 100 characters in length and must be unique. Only letters, numbers, underscores, hyphens, periods, asterisks, forward slashes, backslashes, exclamation marks, left angle brackets, colons and parentheses are allowed.

More info: https://support.google.com/dfp_premium/answer/1628457?hl=en

This is leading to arrays that have different keys than the actual Ad Unit name. For example, within Ad_Layers_DFP::ad_unit_js():

                printf(
                    "dfpAdUnits[%s] = googletag.%s(%s%s,%s)%s%s.addService(googletag.pubads());\n",
                    wp_json_encode( $ad_unit ),
                    $is_oop ? 'defineOutOfPageSlot' : 'defineSlot',
                    wp_json_encode( $this->get_path( $page_type, $ad_unit ) ),
                    // if this is not oop, this is an additional arg to the
                    // method call, and is prefixed with a comma:
                    $is_oop ? '' : ',' . wp_json_encode( $this->default_by_unit[ $ad_unit ] ),
                    wp_json_encode( $this->get_ad_unit_id( $ad_unit ) ),
                    ( ! empty( $this->mapping_by_unit[ $ad_unit ] ) && ! in_array( $ad_unit, $this->oop_units ) ) ? '.defineSizeMapping(mapping' . $this->sanitize_key( $ad_unit ) . ')' : '',
                    ( ! empty( $this->targeting_by_unit[ $ad_unit ] ) ) ? $this->targeting_by_unit[ $ad_unit ] : '' // This is escaped above as it is built
                );

In the above code, if the $ad_unit is named something like 'ad_bigbox', then the (non)matching key in the $this->targeting_by_unit array is actually 'adbigbox' due to sanitize_key() stripping out the '_' character. This is currently preventing the above JS generation logic from working for size mapping and ad-unit level targeting key/values.

[bcampeau]

The intent here was to create an internal reference to the ad unit, not to follow Google's ad unit name specification. If custom code is expecting this index to exactly match the ad unit name, then that could certainly be an issue. We'll have to review what's possible to change here.