SAMMI-Official icon indicating copy to clipboard operation
SAMMI-Official copied to clipboard

SAMMI does not properly handle Expect headers in webhook requests

Open reversefold opened this issue 11 months ago • 3 comments

When sending a webhook request to SAMMI from a client that implements the "Expect: 100-Continue" header, SAMMI does not send back a "100 Continue" or "417 Expectation Failed", causing some clients to hang or not send their payloads (specifically, .NET's HTTP client never sends the payload, causing the webhook to never be received).

Sending "100 Continue" should fix the problem as the payload would then be received. Sending "417 Expectation Failed" should also solve the problem as the client is supposed to resend without the header at that point, but it's unclear to me if this is something built into most client libraries to automatically do.

reversefold avatar Jan 16 '25 04:01 reversefold

This is known, and fully documented. We have an alert about it in our docs that tell you how to solve this issue here in your client (specifically c#!): https://sammi.solutions/docs/api/webhooks#sendingwebhookstosammi

Using C#? System.Net.ServicePointManager.Expect100Continue must be set to false to stop the header “Expect: 100-Continue” from being sent with the request as SAMMI will not handle it properly.

SAMMI cannot handle this header.

Landiie avatar Jan 16 '25 06:01 Landiie

Understood, but not all clients necessarily have control over this. For instance, when trying to send a webhook from a LUA script in the BizHawk emulator. The LUA environment doesn't have access to set the client's properties. I've submitted some MRs there but technically if you're exposing an endpoint as HTTP/1.1 then it's a bug in SAMMI that should be fixed as it's not complying with the HTTP spec. Understandably the BizHawk devs are reluctant to directly expose setting that property as it's a niche use case of a niche use case and script writers shouldn't have to be digging deep into the implementation of the LUA environment and the webhook endpoint. Additionally there are valid use cases for programs to use this 25 year old feature so it would also be wrong to simply turn it off for everything.

Can you please explain why SAMMI "cannot" handle this part of the HTTP/1.1 spec properly?

On Wed, Jan 15, 2025, 10:26 PM Landie @.***> wrote:

This is known, and fully documented. We have an alert about it in our docs that tell you how to solve this issue here in your client (specifically c#!): https://sammi.solutions/docs/api/webhooks#sendingwebhookstosammi

Using C#? System.Net.ServicePointManager.Expect100Continue must be set to false to stop the header “Expect: 100-Continue” from being sent with the request as SAMMI will not handle it properly.

SAMMI cannot handle this header.

— Reply to this email directly, view it on GitHub https://github.com/SAMMISolutions/SAMMI-Official/issues/23#issuecomment-2594632952, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAYOVFBBP4HLDETT2DRORD2K5GLBAVCNFSM6AAAAABVIWVRCGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJUGYZTEOJVGI . You are receiving this because you authored the thread.Message ID: @.***>

reversefold avatar Jan 16 '25 07:01 reversefold

I should clarify: SAMMI in it's current state, cannot handle the header.

The issue is known, and it will be fixed. Resources are low at the moment so the best we could do was give a disclaimer in the documentation.

The engine we use, GameMaker, does not provide HTTP server capabilities. We have to recreate the HTTP spec manually with a TCP server, which has been a daunting task for our team. Thank you for bringing the issue to light again for us! I assumed you were looking for a quick workaround vs actually reporting the issue since it's been reported already as indicated by the existence of the note in the documentation. Rest assured it is not intentional and it is noted as an issue. My apologies!

Landiie avatar Jan 16 '25 08:01 Landiie