Skip to content

fix: frontend malformed uri crash#31921

Closed
yo3gnd wants to merge 1 commit into
Koenkk:devfrom
yo3gnd:yo3gnd-fix-frontend-malformed-uri-crash
Closed

fix: frontend malformed uri crash#31921
yo3gnd wants to merge 1 commit into
Koenkk:devfrom
yo3gnd:yo3gnd-fix-frontend-malformed-uri-crash

Conversation

@yo3gnd

@yo3gnd yo3gnd commented May 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #31920

Summary

This change stops Zigbee2MQTT from crashing when the FE receives a malformed percent-encoded request path.

The symptom is:

TypeError: res.status is not a function
    at expressStaticGzip (.../express-static-gzip/index.js:37:11)
    at Server.onRequest (/app/lib/extension/frontend.ts:109:21)

The correct behaviour here is rather boring: send 400 and move on, and, more importantly, don't crash.

Root cause

Frontend.onRequest() rewrites request.url / request.path and then passes raw Node IncomingMessage / ServerResponse objects into express-static-gzip.

That works fine for normal requests. It goes sideways when req.path contains invalid percent-encoding.

Inside express-static-gzip, the middleware does roughly this:

  1. decodeURIComponent(req.path)
  2. if that throws, res.status(400).send(e.message)

The snag is that res here is not an Express response. It is a plain Node ServerResponse, so res.status does not exist. Instead of returning a 400, the process crashes.

How this turned up

The field report that led me here came from an elderly but previously functional setup:

  • Zigbee2MQTT 2.10.0
  • Sonoff ZBBridge (not pro, old one)
  • Tasmota TCP serial bridge
  • EZSP v8 6.7.9.0 build 405

That context sent me off chasing adapter gremlins for rather longer than I care to admit. The bridge had already been opened, the LEDs had been civilised with resistor surgery, a new USB socket had been soldered on, the PSU and cables had been swapped, and various other acts of honest suspicion had been committed. It still happened, like clockwork. After all that, the culprit turned out to be a malformed FE request path - and this took down all my zigbee devices for half a minute, every 5 minutes, in the HA endpoint.

To be clear, the crash itself is not an ezsp bug. The adapter and hardware were simply the scenery around the crime.

In the environment that exposed this, an unrelated local service lobbed malformed HTTP requests at the frontend port due to a shared host/network layout. Daft, yes, but a malformed URL should still earn a 400, not a dead bridge.

What this patch does

Before handing the request to express-static-gzip, validate request.path with decodeURIComponent().

If decoding fails:

  • return 400
  • set Content-Type: text/plain; charset=utf-8
  • end the response directly with the Node HTTP response object

If decoding succeeds:

  • continue exactly as before
  • normal frontend requests are unchanged
  • device icon serving is unchanged

This keeps the fix small and local. One bad URL no longer gets to kill the whole process.

Validation

Added a regression test for a malformed path:

/%E0%A4%A

The new test verifies that Zigbee2MQTT:

  • responds with 400
  • does not call the static file server
  • does not crash

I also ran the frontend test suite on this branch.

Beyond that, I validated the built patch mildly. This wasn't a full coordinator-backed integration test, but it does exercise the real HTTP and WebSocket paths in-container.

Observed results:

  • default frontend:
    • GET / returns 200
    • real JS and CSS assets return 200
    • existing device_icons file returns 200
    • missing device_icons file returns normal 404
    • GET /%E0%A4%A returns 400 and crashes
    • GET /device_icons/%E0%A4%A returns 400
    • WebSocket handshake on /api succeeds
  • non-default base_url: /z2m:
    • GET /z2m redirects to /z2m/
    • GET /z2m/ returns 200
    • real asset under /z2m/assets/... returns 200
    • existing icon under /z2m/device_icons/... returns 200
    • missing icon under /z2m/device_icons/... returns normal 404
    • GET /z2m/%E0%A4%A returns 400
    • WebSocket handshake on /z2m/api succeeds

Most importantly, after the malformed requests the patched containers stayed up. For comparison, stock 2.10.0 under the same harness still crashes on /%E0%A4%A with the original res.status is not a function stack.

Notes

This is in Zigbee2MQTT's frontend server wrapper rather than in the frontend asset package itself. The middleware's assumption is understandable in an Express app; it is merely wrong in this particular bit of plumbing.

@yo3gnd yo3gnd changed the base branch from master to dev May 6, 2026 17:11
@Nerivec

Nerivec commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Didn't dig into this, but this seems like a bug that should be fixed in the library, not as a workaround in Z2M?

@yo3gnd

yo3gnd commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

I think this is more of a contract mismatch than a purely library-side bug. Z2M is using express-static-gzip successfully on the normal path with raw Node HTTP objects plus a few express-like fields, but the malformed-URI branch calls res.status(...).send(...), which is not available there. So the failing part is specifically the error path under Z2M’s current integration. Since this took a fair bit of digging to pin down, I thought it worth contributing the small, tested fix here rather than keeping it as a local patch.

That said, an upstream safeguard in the library would be better indeed.

@Nerivec

Nerivec commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Can you take a look at https://github.com/tkoenig89/express-static-gzip see if you can find the offending location(s)?
CC: @bdolgov

@Nerivec Nerivec changed the title Yo3gnd fix frontend malformed uri crash fix: frontend malformed uri crash May 6, 2026
@yo3gnd

yo3gnd commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Already on it. Probably.

I opened the issue upstream and got a patch ready.

@yo3gnd

yo3gnd commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

That was pleasantly quick: the library-side fix is now merged upstream and released as 3.0.1. Z2M will still only benefit once that lands in a release and is bumped here, so I’m happy with whichever route you prefer: keep this local guard, or rely on the upstream fix once it is consumable.

@Koenkk

Koenkk commented May 6, 2026

Copy link
Copy Markdown
Owner

Bumped in #31924, thanks!

@Koenkk Koenkk closed this May 6, 2026
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

Successfully merging this pull request may close these issues.

FE crash on malformed percent-encoded request path (res.status is not a function)

3 participants