Ingress blocking duplicate response header names

mijofa commented 1 year ago

Describe the issue you are experiencing

When using the Ingress feature to access an add-on's web server, if that add-on sends multiple "Set-Cookie" headers, HA/Ingress will only send one of those headers to the user's browser. I believe this is happening with every HTTP header, but the only one I've actually run into issues with is Set-Cookie, since that requires repeating the header to set multiple cookies.

What type of installation are you running?

Home Assistant OS

Which operating system are you running on?

Home Assistant Operating System

Steps to reproduce the issue

Add something like this to the nginx.conf of an add-on::

server { 
  listen 8099;
  add_header "X-Frame-Options" "SAMEORIGIN";
  add_header "Set-Cookie" "foo=bar";
  add_header "Set-Cookie" "baz=quux";
  add_header "Set-Cookie" "test=OnlyThisOneWorks";
  location / { 
      proxy_pass http://localhost:8888;
      proxy_cookie_path / $http_X_INGRESS_PATH/;
  } 
}

Browse to that add-on's Ingress endpoint and look at your browser's Network Inspector.
All of the requests that went to the add-on should have all 3 "Set-Cookie" headers (at least) and all 3 cookies should be set in the browser, but "test=OnlyThisOneWorks" makes it through.

Anything in the Supervisor logs that might be useful for us?

Nothing useful, since it's not an actual error ocurring, just the system not quite behaving as one would expect.

System Health information

System Information

version	core-2023.5.2
installation_type	Home Assistant OS
dev	false
hassio	true
docker	true
user	root
virtualenv	false
python_version	3.10.11
os_name	Linux
os_version	6.1.25
arch	x86_64
timezone	Australia/Sydney
config_dir	/config

Home Assistant Community Store

AccuWeather

can_reach_server | ok -- | -- remaining_requests | 42

Home Assistant Cloud

Home Assistant Supervisor

host_os | Home Assistant OS 10.1 -- | -- update_channel | stable supervisor_version | supervisor-2023.04.1 agent_version | 1.5.1 docker_version | 23.0.3 disk_total | 116.7 GB disk_used | 55.3 GB healthy | true supported | true board | generic-x86-64 supervisor_api | ok version_api | ok installed_addons | Mosquitto broker (6.2.1), motionEye (0.19.1), NGINX Home Assistant SSL proxy (3.4.2), Advanced SSH & Web Terminal (14.1.0), Home Assistant Google Drive Backup (0.110.4), Let's Encrypt (4.12.8), Mautrix Facebook bridge (0.0.3), TimescaleDB (2.1.1), Matrix.org Synapse (0.0.14), Mautrix Google Chat bridge (0.0.2), Matrix Steam bridge (0.1.0), Matrix Heisenbridge, with Wireguard (0.5.20), Rsyslog (2022.5.1), snapcast (0.42)

Dashboards

Recorder

Supervisor diagnostics

I don't think this is useful for this issue, and there's too much data for me to reliably check it for privacy/etc. Happy to include if told it is necessary though.

Additional information

No response

mijofa commented 1 year ago

Just (accidently) confirmed it did the same deduping with X-Frame-Options header as well, although it's probably not bad to dedupe that one, it does confirm it's not just Set-Cookie

I believe this is because the headers are being processed as a dict around here but dicts can't have duplicate keys so they just overwrite each other. Might be more valid to use a list of 2-tuples, but I'm not sure how to neatly fit that in here.

github-actions[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

home-assistant / supervisor