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

Optimization detective can return non-ascii characters in the Link header, breaking some reverse proxies and HTTP standards. #1775

Open
amitay-elementor opened this issue Jan 6, 2025 · 4 comments · May be fixed by #1802
Assignees
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken

Comments

@amitay-elementor
Copy link

Bug Description

Optimization detective can return non-ascii characters in the Link header, breaking some reverse proxies and HTTP standards.

Steps to reproduce

Example link with Hebrew chars: https://testsite.com/wp-content/uploads/2025/01/חנות-scaled.avif
If this is added to optimization detection, for example via image prioritizer, it will cause the response header of the page to include:

Link: <https://testsite.com/wp-json/>; rel="https://api.w.org/", <https://testsite.com/wp-json/wp/v2/pages/1234>; rel="alternate"; title="JSON"; type="application/json", <https://testsite.com/wp-content/uploads/2025/01/חנות-scaled.avif>; rel="preload"; fetchpriority="high"; as="image"; media="screen and (max-width: 480px)"

HTTP headers should contain only ISO-8859-1 chars and this breaks it, the url path should be encoded.

@amitay-elementor amitay-elementor added the [Type] Bug An existing feature is broken label Jan 6, 2025
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Jan 6, 2025
@westonruter westonruter added the [Plugin] Optimization Detective Issues for the Optimization Detective plugin label Jan 6, 2025
@westonruter westonruter moved this from Not Started/Backlog 📆 to To Do 🔧 in WP Performance 2024 Jan 6, 2025
@AhmarZaidi
Copy link
Contributor

Hey 👋,
I'd like to work on this issue.

@westonruter
Copy link
Member

Here's the relevant code:

public function get_response_header(): ?string {
$link_headers = array();
foreach ( $this->get_prepared_links() as $link ) {
// The about:blank is present since a Link without a reference-uri is invalid so any imagesrcset would otherwise not get downloaded.
$link['href'] = isset( $link['href'] ) ? esc_url_raw( $link['href'] ) : 'about:blank';
$link_header = '<' . $link['href'] . '>';
unset( $link['href'] );
foreach ( $link as $name => $value ) {
/*
* Escape the value being put into an HTTP quoted string. The grammar is:
*
* quoted-string = DQUOTE *( qdtext / quoted-pair ) DQUOTE
* qdtext = HTAB / SP / %x21 / %x23-5B / %x5D-7E / obs-text
* quoted-pair = "\" ( HTAB / SP / VCHAR / obs-text )
* obs-text = %x80-FF
*
* See <https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.4>. So to escape a value we need to add
* a backslash in front of anything character which is not qdtext.
*/
$escaped_value = preg_replace( '/(?=[^\t \x21\x23-\x5B\x5D-\x7E\x80-\xFF])/', '\\\\', $value );
$link_header .= sprintf( '; %s="%s"', $name, $escaped_value );
}
$link_headers[] = $link_header;
}
if ( count( $link_headers ) === 0 ) {
return null;
}
return 'Link: ' . implode( ', ', $link_headers );
}

@westonruter
Copy link
Member

@AhmarZaidi How's it going with this?

@AhmarZaidi
Copy link
Contributor

Hey @westonruter,

I've created a Draft PR for this issue: #1802
I have separately encoded the filename only instead of the whole URL since the rest is standard uploads path in WordPress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken
Projects
Status: To Do 🔧
3 participants