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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ha-proxy.js sends two location headers #2227

Closed
mpietruschka opened this issue Feb 20, 2023 · 4 comments · Fixed by #2228
Closed

ha-proxy.js sends two location headers #2227

mpietruschka opened this issue Feb 20, 2023 · 4 comments · Fixed by #2228
Labels
🐛 bug-report Something isn't working platform/ha-addon Home Assistant Add-on platform

Comments

@mpietruschka
Copy link
Contributor

mpietruschka commented Feb 20, 2023

Describe the issue you are experiencing

I do integrate raspberrymatic supervised, with traefik and docker labels. Raspberymatic is supposed to be accessable by the location http://hostname-doesnt-matter/raspberrymatic/.

To make that happen I reused your HomeAssistant integration, added the neccessary 'x-ingress-path' headern and HM_HAPROXY_SRC environment variable. Everything looked very promissing until I accessed the starting page http://hostname-doesnt-matte/raspberrymatic/

My browser stopped porcessing the response by "Beim Laden der Seite http://10.0.0.2/raspberrymatic/pages/index.htm wurde gegen das Netzwerkprotokoll verstoßen. Dieser Fehler kann nicht behoben werden."

After some digging I found out that your ha-proxy.js adds a second location header to the response. Since Traefik does not sanitize headers, my browser was the first one complaining.

A simple res.removeHeader('location'); before res.append("location", redirect); in your ha-proxy.js is doing the trick.

I'm going to send a Pull Request.

Describe the behavior you expected

Here's my test-setup...

version: "3.8"
services:
  reverse-proxy:
    image: traefik:v2.9
    restart: always
    labels:
      - "traefik.http.routers.traefik.rule=HostRegexp(`{host:.*}`) && (PathPrefix(`/dashboard`) || PathPrefix(`/api`))"
      - "traefik.http.services.traefik.loadbalancer.server.port=8080"
    # Enables the web UI and tells Traefik to listen to docker
    command: --api.insecure=true --providers.docker
    ports:
      # The HTTP port
      - "80:80"
    volumes:
      - /etc/localtime:/etc/localtime:ro
      - /var/run/docker.sock:/var/run/docker.sock

  raspberrymatic:
    image: ghcr.io/jens-maus/raspberrymatic:latest
    labels:
      - "traefik.http.routers.raspberrymatic.rule=HostRegexp(`{host:.*}`) && PathPrefix(`/raspberrymatic`)"
      - "traefik.http.routers.raspberrymatic.middlewares=raspberrymatic-header,raspberrymatic-stripprefix"
      - "traefik.http.middlewares.raspberrymatic-stripprefix.stripprefix.prefixes=/raspberrymatic/,/"
      - "traefik.http.middlewares.raspberrymatic-header.headers.customrequestheaders.x-ingress-path=/raspberrymatic"
      - "traefik.http.services.raspberrymatic.loadbalancer.server.port=8099"
    privileged: true
    restart: unless-stopped
    stop_grace_period: 30s
    environment:
      - HM_HAPROXY_SRC=0.0.0.0/0
    volumes:
      - ccu_data:/usr/local:rw
      - /lib/modules:/lib/modules:ro
      - /run/udev/control:/run/udev/control
    ports:
      - "8099:8099"
#    cpu_rt_runtime: 950000
    ulimits:
      rtprio: 99
volumes:
  ccu_data:

Steps to reproduce the issue

Setup traefik

What is the version this bug report is based on?

3.67.10.20230114

Which base platform are you running?

rpi4 (RaspberryPi4)

Which HomeMatic/homematicIP radio module are you using?

RPI-RF-MOD

Anything in the logs that might be useful for us?

$ curl --verbose --header "x-ingress-path: /bla"  --header "Cookie: _pk_id.1.cafc=ca8adf6e40890c91.1676830552."  -data "tbUsernameShow=admin&tbUsername=admin&tbPassword=mypassword"  http://myip:8099/login.htm

* Closing connection -1
curl: (3) URL using bad/illegal format or missing URL
*   Trying 10.0.0.2:8099...
* Connected to 10.0.0.2 (10.0.0.2) port 8099 (#0)
> POST /login.htm HTTP/1.1
> Host: 10.0.0.2:8099
> User-Agent: curl/7.87.0
> Accept: */*
> x-ingress-path: /bla
> Cookie: _pk_id.1.cafc=ca8adf6e40890c91.1676830552.
> Content-Length: 3
> Content-Type: application/x-www-form-urlencoded
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 302 Found
< X-Powered-By: Express
< accept-ranges: bytes
< cache-control: private, no-cache, must-revalidate, no-transform, max-age=0
< content-type: text/html; charset=iso-8859-1
< location: /login.htm?error=1
< location: /bla/login.htm?error=1
< x-frame-options: SAMEORIGIN
< x-content-type-options: nosniff
< x-xss-protection: 1; mode=block
< x-robots-tag: none
< x-download-options: noopen
< x-permitted-cross-domain-policies: none
< referrer-policy: no-referrer
< connection: close
< date: Mon, 20 Feb 2023 12:39:10 GMT
< content-length: 0
<
* Closing connection 0

Additional information

No response

@mpietruschka mpietruschka added the 🐛 bug-report Something isn't working label Feb 20, 2023
@mpietruschka mpietruschka mentioned this issue Feb 20, 2023
7 tasks
@jens-maus jens-maus added the platform/ha-addon Home Assistant Add-on platform label Feb 20, 2023
@jens-maus jens-maus linked a pull request Feb 21, 2023 that will close this issue
7 tasks
jens-maus added a commit that referenced this issue Feb 21, 2023
…dd multiple location headers by using setHeader instead (#2228, #2227, @mpietruschka)

Co-authored-by: Jens Maus <mail@jens-maus.de>
@mpietruschka
Copy link
Contributor Author

mpietruschka commented Feb 21, 2023

@jens-maus So you don't want to set the location header, when it didn't exist in the response header, right?

from the readme doesn't seem to work. It just uses the header from the req (or leaves it out if the request header was empty).
http-party/node-http-proxy#889

@jens-maus
Copy link
Owner

@jens-maus So you don't want to set the location header, when it didn't exist in the response header, right?

Of course I always want to set the location header. AFAIK the setHeader method should take care of this (cf. https://www.geeksforgeeks.org/node-js-response-setheader-method/). However, feel free to test and report back.

@mpietruschka
Copy link
Contributor Author

@jens-maus Please excuse my odd way of asking my question. Of course I assumed that you want it always to be set. Somehow I had been confused by the referenced bugreport. I'll test it later! Thanks again :)

https://nodejs.org/api/http.html#requestsetheadername-value

@mpietruschka
Copy link
Contributor Author

@jens-maus Your solution works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug-report Something isn't working platform/ha-addon Home Assistant Add-on platform
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants