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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Include Fastly-specific status codes #138

Open
miketheman opened this issue Apr 3, 2023 · 3 comments
Open

[Feature] Include Fastly-specific status codes #138

miketheman opened this issue Apr 3, 2023 · 3 comments
Assignees

Comments

@miketheman
Copy link

Describe the feature you'd like
When using Fastly-specific status codes, an INFO level message is raised:

馃攬 [INFO] code 801 in error statemnt should use between 600-699 (error-statement/code)

Happy to send a PR if that's desired!

Put example VCL after the feature has been implemented

            if (req.request == "GET" || req.request == "HEAD") {
                error 801 "Force SSL";
            }

This VCL should not change, nor emit any messages.

Additional context

See: https://developer.fastly.com/reference/http/http-statuses/#700-899-reserved-for-fastly

@ysugimoto
Copy link
Owner

Hi @miketheman

Thank you for reaching out to the project!
Regarding your suggestion, I would like to discuss the spec:

  1. I'm guessing typically error 801 Force SSL presents if Force TLS feature is enabled, and then that snippet will be embedded by extracting #FASTLY RECV macro. Do you intend to enforce SSL redirection on your custom VCL? falco will only do linting the extracted VCL snippets, do not lint the VCL codes which is embedded by fastly runtime.
  2. We could suppress INFO error by excluding 801 error code. But then should we also check the error message is exactly Force SSL?

I'm glad to hear your thoughts and opinions. Of course, we welcome your PR 馃槃

@miketheman
Copy link
Author

Hi @ysugimoto ! Thanks for the detailed response.

  1. Yes, I believe the Force TLS setting is in place (I'm not an admin on the service yet so cannot confirm, rather a contributor to the VCL). A more complete snippet example would be:
sub vcl_recv {
  ...
#FASTLY recv
  ...
            if (req.request == "GET" || req.request == "HEAD") {
                error 801 "Force SSL";
            }
...
}

The intent is that in the conditions that are hit, to have the Force TLS feature automatically redirect the client in some conditions, but in other conditions we set a 603 status code, which we then handle in sub vcl_error/#FASTLY error to turn it into a client-facing 403 with extra metadata.
Is it possible that we're misusing status code 801?

  1. This one is a little more ambiguous, I guess - since if the answer to 1 is to change our behavior, then it is unnecessary. Since there's no standard I could find that the structure of an 801 message would be "Force SSL" beyond our implementation, maybe we shouldn't check the error message.

Thinking about this more, I'm even thinking that we should probably update our VCL to conform to a 6xx range response code, like https://developer.fastly.com/solutions/examples/force-tls-ssl-https

@ysugimoto
Copy link
Owner

ysugimoto commented Apr 4, 2023

@miketheman Thanks for giving your thoughts.

I understand that we should lint error 801 would present via Fastly HTTPS redirection feature so the user should not write error 801 manually and suggest Fastly redirection feature so,

but in other conditions we set a 603 status code, which we then handle in sub vcl_error/#FASTLY error to turn it into a client-facing 403 with extra metadata.

This is correct way to HTTPS redirection on custom VCL.

However, falco could not understand VCL context so could not recognize what the error code means, so it depends on the user whether 801 error code is correct or not so currently we raise INFO message the 8xx error is reserved on Fastly. It means you should not use 8xx code in your custom VCL.

On that note, we could do the following ways I think:

  1. Keep it as it is, raise INFO message for 8xx code is using
  2. Add more linter rule that suggests Fastly's HTTPS redirection feature, should not be placed in custom VCL
  3. suppress raising message for 801 error code using

Any other thoughts? or If I misunderstand your opinion, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants