response_headers for 4xx and 5xx codes about unit HOT 7 OPEN

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.

from unit.

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?

from unit.

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
        }
    }
}

from unit.

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.

from unit.

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.

from unit.

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.

from unit.

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

from unit.