nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.27k stars 324 forks source link

response_headers for 4xx and 5xx codes #994

Open AlekseyArh opened 8 months ago

AlekseyArh commented 8 months ago

Моё приложение использует http состояния, например 409 при создании уже существующей записи. My application uses http states, for example 409 when creating an already existing record.

Но я не могу добавить CORS заголовки для ответов отличных от 2xx и 3xx. But I can't add CORS headers for responses other than 2xx and 3xx.

В nginx я бы мог использовать параметр always. In nginx I could use the always parameter.

Как мне быть в таком случае при использовании unit? What should I do in this case when using unit?

hongzhidao commented 8 months ago

Hi, Unit hasn't supported the always option in the latest version. Actually, we thought about it. We also consider supporting it with njs. Btw, what's your configuration about the response_headers option? I wonder if your requirement is only to add always to work with all the response status? Or there is a more complicated logic?

AlekseyArh commented 8 months ago

I wonder if your requirement is only to add always to work with all the response status? Or there is a more complicated logic?

There is no complicated logic.

{
"routes": [
     {
       "match":{
            "method": "GET"
        },
        "action": {
            "response_headers": {
                "Access-Control-Allow-Origin": "${header_origin}",
                "Access-Control-Allow-Credentials": "true",
                "Access-Control-Allow-Methods": "GET, POST, OPTIONS, PUT, DELETE",
                "Access-Control-Allow-Headers": "Accept, Authorization, Content-Type"
            }
        }
    }
]
}

Inclusion at the global level would be appropriate.

{
    "settings": {
        "http": {
            "server_version": false,
            "response_headers_always": true
        }
    }
}
hongzhidao commented 8 months ago

Thanks for the feedback.

Inclusion at the global level would be appropriate.

Yep, it makes sense to apply some of the options in the action object into the http object. Compared to nginx configuration. The settings/http is similar to the http main context, and the action is similar to the location context. So we need to introduce always into the response_headers to make each action able to control the status scope. The configuration would look like:

{
    "settings": {
        "http": {
            "server_version": false,
            "response_headers_always": true
        }
    },
   "routes": [
     {
        "action": {
            "response_headers": {
                 "always": true,
                 "Access-Control-Allow-Origin": "${header_origin}",
                  "Access-Control-Allow-Credentials": "true",
                  "Access-Control-Allow-Methods": "GET, POST, OPTIONS, PUT, DELETE",
                  "Access-Control-Allow-Headers": "Accept, Authorization, Content-Type"
              }
          }
      }
  ]
}

request_header for 4xx and 5xx codes

"response_headers for 4xx and 5xx codes" Btw, we are discussing the response_headers? If yes, feel free to change the issue title.

tippexs commented 7 months ago

Thanks for the feedback.

Inclusion at the global level would be appropriate.

Yep, it makes sense to apply some of the options in the action object into the http object. Compared to nginx configuration. The settings/http is similar to the http main context, and the action is similar to the location context. So we need to introduce always into the response_headers to make each action able to control the status scope.

if this is the case, whats the global configuration dooing? Setting response_headers_always true would turn it on for all response header actions? And off would turn if off which is the default until somebody turnes it to on on the action level?

The configuration would look like:

{
    "settings": {
        "http": {
            "server_version": false,
            "response_headers_always": true
        }
    },
   "routes": [
     {
        "action": {
            "response_headers": {
                 "always": true,
                 "Access-Control-Allow-Origin": "${header_origin}",
                  "Access-Control-Allow-Credentials": "true",
                  "Access-Control-Allow-Methods": "GET, POST, OPTIONS, PUT, DELETE",
                  "Access-Control-Allow-Headers": "Accept, Authorization, Content-Type"
              }
          }
      }
  ]
}

request_header for 4xx and 5xx codes

"response_headers for 4xx and 5xx codes" Btw, we are discussing the response_headers? If yes, feel free to change the issue title.

I think we are good at the action level for now as dealing with the global settings sounds like a little bit to overkill to me. We would need to write a good amount of documentation to let users know when to use what settings. Let's start with the on-action basis. We have to discuss whats the effort of this change and if it fits well into one of the next releases.

AlekseyArh commented 7 months ago

Thanks for the feedback.

Inclusion at the global level would be appropriate.

Yep, it makes sense to apply some of the options in the action object into the http object. Compared to nginx configuration. The settings/http is similar to the http main context, and the action is similar to the location context. So we need to introduce always into the response_headers to make each action able to control the status scope.

if this is the case, whats the global configuration dooing? Setting response_headers_always true would turn it on for all response header actions? And off would turn if off which is the default until somebody turnes it to on on the action level?

The configuration would look like:

{
    "settings": {
        "http": {
            "server_version": false,
            "response_headers_always": true
        }
    },
   "routes": [
     {
        "action": {
            "response_headers": {
                 "always": true,
                 "Access-Control-Allow-Origin": "${header_origin}",
                  "Access-Control-Allow-Credentials": "true",
                  "Access-Control-Allow-Methods": "GET, POST, OPTIONS, PUT, DELETE",
                  "Access-Control-Allow-Headers": "Accept, Authorization, Content-Type"
              }
          }
      }
  ]
}

request_header for 4xx and 5xx codes

"response_headers for 4xx and 5xx codes" Btw, we are discussing the response_headers? If yes, feel free to change the issue title.

I think we are good at the action level for now as dealing with the global settings sounds like a little bit to overkill to me. We would need to write a good amount of documentation to let users know when to use what settings. Let's start with the on-action basis. We have to discuss whats the effort of this change and if it fits well into one of the next releases.

I thought it was easier to do it on a global level. "response_headers_always": true - better than nothing. But "always": true is certainly more correct and flexible.

tippexs commented 7 months ago

I have just looked into our internal documentation (Which I will move to Github - as there is no reason keeping the discussion about the way we want to work with response headers in the private).

Always sending all response headers are not a good idea. For example Cache-Control Headers should not be send if the HTTP-Status of the response is 404.

I think the best solution is to introduce a more detailed response_header object in the Unit configuration making it easier for you to define what Header should be sent with what HTTP-Status code. The rule should be: If not defined differently, the HTTP-Header will only be sent on positive HTTP-Status Codes (2xx,3xx). Not for 4xx and 5xx. If defined differently like

[
  {
    "action": {
      "share": "/var/www$uri",
      "response_headers": {
        "Set-Cookie": {
          "value": "returnee=true",
          "status": [200, 404, "5xx"]
        }
      }
    }
  }
]

headers can be included in 5xx and 4xx messages. I would support BOTH way to configure headers

[
  {
    "action": {
      "share": "/var/www$uri",
      "response_headers": {
        "X-Test-Header-Short": "Value-for-short",
        "Set-Cookie": {
          "value": "returnee=true",
          "status": [200, 404, "5xx"]
        }
      }
    }
  }
]

@hongzhidao what do you think? I will move this into a discussion thread to have the function requirements about response headers in Unit in a new thread.

hongzhidao commented 7 months ago

I'd like to ask @AlekseyArh if the suggestion meets your requirements. @tippexs, what does your configuration look like for @AlekseyArh example?

nathanOreilly commented 1 month ago

I have a similar requirement to @AlekseyArh and it looks like the suggestion by @tippexs would be perfect as long as there was some way to say all statuses should be covered. The config would look something like this:

{
"routes": [
     {
       "match":{
            "method": "GET"
        },
        "action": {
            "response_headers": {
                "Access-Control-Allow-Origin": {
                    "value": "${header_origin}",
                    "status": ["all"]
                } ,
                "Access-Control-Allow-Credentials": {
                    "value": "true",
                    "status": ["all"]
                },
                "Access-Control-Allow-Methods": {
                    "value": "GET, POST, OPTIONS, PUT, DELETE",
                    "status": ["all"]
                },
                "Access-Control-Allow-Headers": {
                    "value": "Accept, Authorization, Content-Type",
                    "status": ["all"]
                }
            }
        }
    }
]
}