I think we can have the browser do this for us simply by creating a Blob. I may be wrong, but it could be worth exploring.

function createTextDataURL( rawText: string, mimeType: string ): Promise<string> {
	const blob = new Blob( [ ( new TextEncoder() ).encode( rawText ) ], { type: mimeType } );
	const file = new FileReader();
	file.readAsDataURL( blob );
	return file.result;
}

const attributeValue = await createTextDataURL( css, 'text/css' );

there’s a reverse transformation from this as well. it’s not like it’s considerably less code than what you have proposed, but it removes some of the internal details about converting text and producing the data URL, so might be simpler to maintain.

what’s the purpose of checking for the null byte? I am imagining a couple of ideas but it’s a guess.

by the way, I think the comment might be a little confused, because the decoded string absolutely should include characters from outside the base64 alphabet.

perhaps there is a valid case to be made to check for wp_is_valid_utf8(), but HTML and CSS are both capable of handling a null byte, and removing one would increase the risk of accidentally introducing an exploit.

what you’ve done by rejecting the CSS entirely is the far-preferable alternative to stripping out null bytes, but I would be curious to learn more the reasoning behind the check in the first place.

would there be a reason to return $value here instead of '' on a failure to decode? what do you think the tradeoffs would be?

Good catch, and thanks for questioning. The comment is wrong.

This was definitely hacked together on a Friday eve. 😇

The strict = true flag on base64_decode is what handles characters outside the base64 alphabet.

The way I understand things, rejecting characters from outside the base64 alphabet is okay when decoding. Because encoding only produces A-Za-z0-9+/= anyway.

Can you help me understand the case for not rejecting? Happy to update this in any way you think is correct.

what’s the purpose of checking for the null byte?

The null byte check is probably an echo now that I pause to think about it. I have to research it again, but some brain cell kept a memory that it might be treated as the end of the string.

color: red;\0</style><script>alert(1)</script>

Shows how old I am: https://pentesterlab.com/glossary/null-byte-injection

"Most modern languages and frameworks now properly handle or reject null bytes. PHP 5.3.4+ throws an error on null bytes in paths. However, legacy systems may still be vulnerable."

I think I'm fine to relax and touch grass 😄

would there be a reason to return $value here instead of '' on a failure to decode? what do you think the tradeoffs would be?

I like that question.

I suppose neither is super duper for UX, but I went with '' to be conservative.

My reasoning was thus - this is inside a render operation function ultimately, so returning nothing is preferable to returning wrong output. Maybe the function name needs to be updated, e..g, to something like gutenberg_decode_custom_css_attribute_for_display. 🤷🏻

Now that we're talking about it, I guess a storage decoder on failure should return $value (preserve the encoded string to avoid data loss). I can't think of a valid use case right now, maybe import/export tools (??) or if we ever wanted the block editor to always receive plain CSS (and not have to call decodeCSSAttribute).

Anyway, I appreciate the help and discussion again. This was a POC to push @glendaviesnz's case forward, I hope we get there!

I like this approach here. while it does leave open some confusion by blurring raw content from data-URI-encoded content, I think the likelihood of finding CSS starting with a data URI prefix is low.

this approach runs the CSS through the existing flow, which is a nice touch, because ultimately, regardless of how we store or source the CSS, we want it running through WordPress’ broader policies.

I’m saddened on your behalf that you have to pollute the code with noise in order to prevent the linter from rejecting your valid and legitimate work.

on the other hand, maybe we should add rules rejecting every builtin PHP function 🤔, or reject all contributions without a sworn disclaimer…