Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Header field / subfield rework #267

Merged
merged 4 commits into from
Mar 4, 2025

Conversation

richardmarshall
Copy link
Collaborator

@richardmarshall richardmarshall commented Feb 28, 2024

Using the current approach, and in the proposed changes in #248, we have of storing fields in a structured format is challenging to achieve compatibility with unusual behavior of Fastly's RFC-8941 header dictionary implementation.

This PRs implementation takes the approach of extracting the field being operated on at the time of access using the observed behavior of Fastly's implementation.

With this approach edge cases in how fastly handles things can be correctly supported for example dealing with fields that contain the field separator.

set req.http.foo:a = "1,b=2";
log req.http.foo:a; // -> 1,b=2
log req.http.foo:b; // -> 2"

The regular expression used in this is admittedly quite opaque and was derived through trial and error based on observing Fastly behavior in as many cases as I could come up with. This same general approach could be achieved with an iterative parser which would be easier to follow what is happening but would be considerably more code. If there is maintainability concern with the regex approach I'm happy to rework this.

Additionally the subfield built-in function shares the field access behavior of the : operator. Updated that function to share the same implementation.

This implementation was validated against Fastly's behavior using this suite of tests:
https://github.com/richardmarshall/falco-validation-tests/blob/main/header_fields/main.test.vcl

@richardmarshall richardmarshall force-pushed the header_field_experiment branch from b19a509 to 58da188 Compare April 3, 2024 04:31
@richardmarshall richardmarshall changed the title Header field / subfield rework experiment Header field / subfield rework Apr 3, 2024
@richardmarshall richardmarshall marked this pull request as ready for review April 3, 2024 04:33
Copy link
Owner

@ysugimoto ysugimoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests are fails but it's okay to merge, thanks and sorry for the late.

@ysugimoto ysugimoto merged commit 8f7f112 into ysugimoto:main Mar 4, 2025
3 of 4 checks passed
@ysugimoto ysugimoto mentioned this pull request Mar 4, 2025
ysugimoto added a commit that referenced this pull request Mar 4, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants