roots / acorn

Laravel components for WordPress plugins and themes
https://roots.io/acorn/
MIT License
825 stars 94 forks source link

ErrorException: `Undefined array key 1` with `header('x-test:header');` #401

Closed RafaelKr closed 2 months ago

RafaelKr commented 2 months ago

Version

4.3.0

What did you expect to happen?

I expected a response with status code 200.

What actually happens?

I got an ErrorException: Undefined array key 1.

The exception happens on this line: https://github.com/roots/acorn/blob/dce6d72c0f9eb1000cc0bc73e22cc2958cb08271/src/Roots/Acorn/Application/Concerns/Bootable.php#L146

It's triggered by the wp-seopress plugin because it adds the header x-robots-tag:noindex, follow (note the missing whitespace after the colon). Here's the source line from the plugin: https://github.com/wp-seopress/wp-seopress-public/blob/d0d5c0cebc4cf238a96e7a3163fa7260aec43c32/src/Services/Sitemap/Headers.php#L23

MDN specifies

HTTP headers let the client and the server pass additional information with an HTTP request or response. An HTTP header consists of its case-insensitive name followed by a colon (:), then by its value. Whitespace before the value is ignored. See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers

Here's the related RFC section: https://datatracker.ietf.org/doc/html/rfc7230#section-3.2

Each header field consists of a case-insensitive field name followed by a colon (":"), optional leading whitespace, the field value, and optional trailing whitespace.

A previous RFC included more whitespace characters than the current RFC: https://datatracker.ietf.org/doc/html/rfc2616#section-4.2 So I think using PHPs ltrim function should be fine.

I suggest to update the Acorn code from

- [$header, $value] = explode(': ', $header, 2);
+ [$header, $value] = explode(':', $header, 2);
+ // HTTP/1.1 Header specification: https://datatracker.ietf.org/doc/html/rfc7230#section-3.2
+ // remove the optional leading whitespace
+ $value = ltrim($value);

I'm not sure if we even should use trim to also trim the optional trailing whitespace. I think we can ignore it.

Steps to reproduce

Send a custom header without a space after the colon.

I can do this in my project by adding a header-bug.php with the following contents inside my mu-plugins folder:

<?php

header('x-test:header');

System info

No response

Log output

No response

Please confirm this isn't a support request.

Yes

Log1x commented 2 months ago

Hey, thanks for the report! Care to do a PR? I'd say ltrim is fine.

RafaelKr commented 2 months ago

There you go :)

402