Fix bug with session id name#414
Conversation
This reverts commit 6ef3c03.
There was a problem hiding this comment.
Pull request overview
This PR fixes generation of the “session id” (used as part of the per-connection SQS FIFO queue name) by stripping all trailing Base64 padding characters (=) from the API Gateway connectionId, preventing invalid SQS queue names when the ID ends with multiple =.
Changes:
- Extracted
get_session_id(event)into a dedicatedSessionIdHelpermodule and updated logic to remove one-or-more trailing=characters. - Updated
api_gateway_proxy_function.rbto use the new helper module instead of an inline implementation. - Added Minitest coverage validating behavior for single padding, multiple padding, and no padding cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| api-gateway-routes/test/session_id_helper_test.rb | Adds unit tests ensuring session IDs strip trailing = padding correctly. |
| api-gateway-routes/session_id_helper.rb | Introduces shared helper to derive a safe session ID for SQS queue naming. |
| api-gateway-routes/api_gateway_proxy_function.rb | Switches to the new helper and removes the previous inline get_session_id implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bencodeorg
left a comment
bencodeorg
left a comment
There was a problem hiding this comment.
Whoa, nice find! So AWS generates the connectionId? And we think that changed somehow?
Yep! |
| # The connection ID is base64-encoded, so it can end with one or more '=' | ||
| # padding characters. We remove all of them here. | ||
| def get_session_id(event) | ||
| event["requestContext"]["connectionId"].sub(/=+$/, "") |
There was a problem hiding this comment.
You may want to consider 1-way hashing the connection id with something like sha256 to ensure alphanumeric to future proof this.
I noticed today that Javabuilder was frequently failing to connect, with the websocket request returning a 502. I (with help from Claude) tracked the error down to
StartSessionsAndRelayMessagesFunction, which was logging"errorMessage": "The name of a FIFO queue can only include alphanumeric characters, hyphens, or underscores, must end with .fifo suffix and be 1 to 80 in length.",. This error pointed to how we were naming our SQS queues. This hasn't changed on our end, but we make an assumption that the websocket connection ids we use to generate the queue names end with a single=. In the case of the failures I found, the ids ended with 2 equals signs. The equals signs aren't valid in FIFO queue names, and therefore we were failing to make the queues.It seems like this is a recent change in how websocket connection ids are generated. I've been using Javabuilder a lot lately and only noticed this happening frequently today. The last week of logs confirm this:
To fix this, instead of only stripping the last
=from the connection id, we instead will strip all=.Testing
I tried to deploy a dev javabuilder instance, but the permissions are broken right now. This change seems pretty safe and I added unit tests for it, so I feel ok about pushing it forward without full testing.