yireo / Yireo_LinkPreload

Magento 2 extension to set HTTP Link headers for primary resources to allow for HTTP/2 Server Push
Open Software License 3.0
74 stars 18 forks source link

Error parsing link header (Unexpected character "/" at offset 0) #45

Closed Morgy93 closed 7 months ago

Morgy93 commented 8 months ago

Hello,

the Google lighthouse reports an error with the header link data:

There were issues affecting this run of Lighthouse:

Error parsing link header (Unexpected character "/" at offset 0): /static/version[....]/css/styles.min.css, /static/version…

The current format is like: Link: /static/version/.../css/styles.min.css, /static/version/.../js/whatever.min.js

My research says that it should look like: Link: <url>; rel="relationship-type"

So in my example: Link: </static/version/.../css/styles.min.css>; rel="stylesheet", </static/version/.../js/whatever.min.js>; rel="script"

jissereitsma commented 8 months ago

Did you make a patch of this and re-run the Lighthouse scan?

Currently, the only place where a header is added by this extension is here: https://github.com/yireo/Yireo_LinkPreload/blob/master/Link/Link.php#L64 And that's according to the specs...

Morgy93 commented 8 months ago

Did you make a patch of this and re-run the Lighthouse scan?

Currently, the only place where a header is added by this extension is here: https://github.com/yireo/Yireo_LinkPreload/blob/master/Link/Link.php#L64 And that's according to the specs...

Interesting, that code you linked is never executed.

It's the processHeaders(): https://github.com/yireo/Yireo_LinkPreload/blob/1.4.21/Link/LinkParser.php#L81

There it just collects the links as array and adds them with implode() using , as seperator. Just like my initial description.

jissereitsma commented 8 months ago

This is a big fail of the code. I'm not sure where this went wrong, but definitely a sidetracking of the original HTTP specs is part of this. A fix is underway, including also the right integration tests to make sure this doesn't break anymore.

jissereitsma commented 8 months ago

Version 1.4.22 should fix this.

Morgy93 commented 7 months ago

Version 1.4.22 should fix this.

Yes, it fixes it. Thanks!

Closed~