fix(headers): add support for consistent * wildcard

[ctjlewis]

Summary

The CLI's current behavior is to only allow the asterisk wildcard * at the end of a rule. This PR aims to add support for wildcard which matches Netlify's deploy logic, which does not seem to have the same restriction.

1. See Header config causes CLI to throw "invalid rule: cannot contain anything after *" next-on-netlify#151
2. Closes Error: invalid rule (A path rule cannot contain anything after * token) #1148

Test plan

Tests were updated in headers.test.js. More robust testing should likely be added before merging.

Changelog

Previously, (*) wildcards had to appear at the end of a rule path, as in /path/to/dir/*. The CLI now supports the asterisk wildcard at any part of the rule, i.e. /test/*/test/*/test.

[ctjlewis]

If anyone has any suggestions regarding the PR label check, please let me know! I'm not sure what to do to clear that up - I thought the feat: tag was enough 😭

(also cc @lindsaylevine)

[lindsaylevine]

this looks amazing @ctjlewis !!! the label thing has to be one of the type: ${something} labels 😺 . i added both bug and feat cause i think it qualifies as both, lol. but thanks for being so thorough!! looks like the windows 14.x tests timed out..? 🤔 i'll leave it to @ehmicky and @erezrokah from here :)

[ctjlewis]

This will currently match /path/to/dir/:placeholder with /path/to/dir, which we might not want as I understand it (that would effectively be :placeholder matching an empty dir /, wouldn't it?).

This "spec" is not well-worded nor detailed (no offense - I plan to send a PR to clarify the language once this is merged), and it only says:

Paths can contain * or :placeholders. A :placeholder matches anything except /, while a * matches anything.

I definitely knew we should take this to mean / does not match /:placeholder but does match /*, but does the / in the quote provided above refer also to nonexistent directory, i.e., should /path/to/dir match /path/to/dir/:placeholder where it definitely matches /path/to/dir/*?

Without adjusting, /path/to/dir/:placeholder and /path/to/dir/* will both match /path/to/dir, but only the latter will match /path/to/dir/one/two/three.

Once we get clarification on this, this will be ready to finalize.

[ctjlewis]

Just tested: This version of netlify dev correctly handles the next-on-netlify rule /*/_next/static/chunks/*, from netlify/next-on-netlify#151, which inspired this PR.

The dev view finally loads and displays the content!

[erezrokah]

Hi @ctjlewis, this is great work! Thank for the fix and those comments & tests look awesome 🚀

I'm going to look into our production logic to verify we're not missing anything here before I merge this.

[ctjlewis]

Absolutely @erezrokah - if you can share any clarification you find on the production logic, I'd appreciate it. The Custom Headers documentation could use a teeny bit more context IMO (mostly just regarding the /path/to/dir/:placeholder vs /path/to/dir case).

[erezrokah]

Hi @ctjlewis sorry for the delay. I added another commit for the headers parsing to make it more consistent with our production logic.
I think it should also make it easier to understand the expected structure.

I'll follow up on the headers matching as well. After finishing the latter I'll open an issue for our docs team to clarify https://docs.netlify.com/routing/headers/#syntax-for-the-headers-file.

[erezrokah]

Once we get clarification on this, this will be ready to finalize.

/path/to/dir/:placeholder should not match /path/to/dir
/path/to/dir/* should match /path/to/dir

I also added some tests for /*test.

The last changes use a regular expression instead of the previous approach - that's much closer to the production logic (it's not pretty though).

[erezrokah]

@ctjlewis thanks again for helping us to clarify this.

I've opened a follow up issue to our docs team

[eduardoboucas]

This looks great — thanks @ctjlewis! 🙌🏻

[ctjlewis]

/path/to/dir/:placeholder should not match /path/to/dir
/path/to/dir/* should match /path/to/dir

Super sorry for the delay in response! One note:

cli/src/utils/headers.test.js

Line 192 in 5166b97

t.assert(matchesPath('/path/to/dir/*/:placeholder', '/path/to/dir/test'))

I wonder if /path/to/dir/*/:placeholder would match /path/to/dir, when it technically... should not?

No idea, not just trying to make more work, just couldn't not notice it lol.

[erezrokah]

I wonder if /path/to/dir/*/:placeholder would match /path/to/dir, when it technically... should not?
Another interesting one :)

Well it is matched by our prod logic:
https://github.com/erezrokah/netlify-build-reproductions/blob/7408e60fdaa2d83780c3664474ab813127a6475e/public/_headers#L1
https://6013edd04e27200008943118--erez-build-reproductions.netlify.app/path/to/dir/test/

I'll add that use case to our docs issue, thanks!

Update

I missed read your comment.

• /path/to/dir/*/:placeholder matches /path/to/dir/test
• /path/to/dir/*/:placeholder doesn't match /path/to/dir